You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/10/26 04:08:43 UTC
[GitHub] [incubator-nuttx] yamt opened a new pull request #2114: Make CONFIG_HAVE_CXX14 an explicit config
yamt opened a new pull request #2114:
URL: https://github.com/apache/incubator-nuttx/pull/2114
## Summary
Replace automatic detection in compiler.h with an explicit config.
While the following two are related, they are not always same.
- if C++14 is enabled for the compiler to build the OS library (libxx)
(this is what's detected in include/nuttx/compiler.h)
- if the OS library should provide C++14 support
(this is what we need to know here)
## Impact
c++14
## Testing
tested with a local app
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d commented on a change in pull request #2114: Make CONFIG_HAVE_CXX14 an explicit config
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2114:
URL: https://github.com/apache/incubator-nuttx/pull/2114#discussion_r512702734
##########
File path: libs/libxx/Kconfig
##########
@@ -22,7 +22,7 @@ config HAVE_CXX
config HAVE_CXX14
bool "C++14 support"
- default n
+ default y
Review comment:
> @v01d
> libxx_*.cxx has operator delete variants for C++14, which is conditional with CONFIG_HAVE_CXX14.
> and, as far as i know, it's the only use of CONFIG_HAVE_CXX14 in the tree.
> ie. neither LIBCXX or UCLIBCXX is relevant.
> so, i'm puzzled by your suggestion to make LIBCXX select it.
Then according to this, HAVE_CXX14 would refer to compiler support, not STD library implementation of given standard version. In that case, the code being removed is the right approach: it will define HAVE_CXX14 to indicate compiler support for C++14. You are right that this will only be used by NuttX's libxx. If you look at LLVM libcxx, they also look into this macro to disable/enablef features.
So, I don't think the change in this PR is needed nor makes sense to have.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2114: Make CONFIG_HAVE_CXX14 an explicit config
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2114:
URL: https://github.com/apache/incubator-nuttx/pull/2114#discussion_r515756009
##########
File path: libs/libxx/Kconfig
##########
@@ -22,7 +22,7 @@ config HAVE_CXX
config HAVE_CXX14
bool "C++14 support"
- default n
+ default y
Review comment:
> > > things to consider:
> > > a. c++ versions the compiler supports
> > > b1. c++ versions the c++ runtime supports (runtime X provides necessary functionalities to run apps written in c++Y)
> > > b2. c++ versions the c++ runtime needs (runtime X itself needs to be built with -std=Y)
> >
> >
> > Why b1 and b2 need difference? if you want the version v of c++ runtime, the compiler version should >= v at least.
>
> i guess it depends on how the library is implemented.
> at least the CONFIG_HAVE_CXX14 parts of NUTTX_LIBXX doesn't seem to require C++14 to build.
>
> > What do you mean "c++ runtime" here? the fundmental c++ library(e.g. libsupc++, libcxxabi) provided by toolchain, or the standard c++ library(e.g. libc++, uClibc++) implementation. The first catagory is coupled with toolchain.
>
> mainly the latter.
>
> > > c1. c++ version to use to build c++ portions of nuttx, including c++ runtime
> > > c2. c++ version to use to build an app
> >
> >
> > c1 and c2 must use the same compiler version with the same option flag.
> > > d. the compiler flag for c1/c2. (eg. -std=c++14 in case of gcc/clang)
> > > random thoughts:
> > >
> > > * c1 can be a Kconfig "choice"
> > > * c1. is restricted by b1 and b2.
> > > i hope the restrictions here can be represented solely with Kconfig directives.
> > > * right now c1 == c2. it's nicer to let users to specify c2. probably within the app's Makefile.
> >
> >
> > c2 must be same as c1, there isn't guarantee that the object with the different compiler option can be safely linked together.
>
> really?
> while i'm not a C++ expert in any sense, i believe it's common to use a single library for multiple versions.
> or, are you suggesting that compiling your app with -std=X does require c++ libraries built with -std=X?
> maybe some flags can alter the abi incompatible ways. but i thought it works in many cases.
>
Yes, it' nightmare to compile app with one version library and link with the same library with another version. C++ standard never define any binary interface. The implementation is free to add/remove the private field or even reorder the field and function sequence, which mean that the object/vtable layout can be changed as needed.
> > > * a. and d. belong to Make.defs/Toolchain.defs
> > > probably a set of make vars like:
> > > CXXSTDFLAGS_cxx14=-std=c++14
> > > CXXSTDFLAGS_cxx17=-std=c++17
> >
> >
> > Two method, I can come up:
> > ```
> > 1. Let's user hardcode c++ compiler version in Make.defs/Toolchain.defs
> >
> > 2. Add Kconfig option, and let's user select which version he want in defconfig, then Make.def/Toolchain.defs add std=c++14 or std=c++17 as needed. On the other hand, if you pass version w to compiler, c++ library will adapter the implementation to confirm to w version standard automatically.
> > ```
> >
> >
> > But all other c++ related code(NUTTX_LIBXX, UCLIBXX, LIBCXX) should check __cplusplus not Kconfig's option.
> > > * the current CONFIG_HAVE_CXX14 has no good use. it's simpler to check __cplusplus directly if necessary.
> > > * right now, in case of NUTTX_LIBXX, b1. is controlled by c1. (via CONFIG_HAVE_CXX14)
> >
> >
> > yes, NUTTX_LIBXX could directly check __cplusplus.
> i remembered a use case which drove me to this PR.
> i wanted to have C (not C++) code with #ifdef CONFIG_HAVE_CXX14.
> it wanted to know if symbols like "_ZdaPvm" was available in the system or not, to build a symbol table.
> as it's in C, i can't check __cplusplus.
> i don't want to make the code to C++ either, as i don't want to introduce C++ dependency only for this.
why not compile the symbol table with c++ compiler? only the global symbol table need be decorated by extern "C". then:
1. You can check __cplusplus to know which function exist
2. Reference the function directly instead _ZdaPvm(which isn't portable between compiler)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d commented on a change in pull request #2114: Make CONFIG_HAVE_CXX14 an explicit config
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2114:
URL: https://github.com/apache/incubator-nuttx/pull/2114#discussion_r512349741
##########
File path: libs/libxx/Kconfig
##########
@@ -22,7 +22,7 @@ config HAVE_CXX
config HAVE_CXX14
bool "C++14 support"
- default n
+ default y
Review comment:
HAVE definitions are used to describe that a certain feature is supported, so by default you define the hidden symbol as default `n` and then `select` it from some other configuration. HAVE_CXX14 would be selected only by libcxx since the library implements the C++14 standard, which is not the case for uClibC. In any case, you are saying that it isn't used, so I understand the purpose of the PR then.
##########
File path: libs/libxx/Kconfig
##########
@@ -22,7 +22,7 @@ config HAVE_CXX
config HAVE_CXX14
bool "C++14 support"
- default n
+ default y
Review comment:
HAVE definitions are used to describe that a certain feature is supported, so by default you define the hidden symbol as default `n` and then `select` it from some other configuration. HAVE_CXX14 would be selected only by libcxx since the library implements the C++14 standard, which is not the case for uClibC. In any case, you are saying that it isn't used, so I don't understand the purpose of the PR then.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2114: Make CONFIG_HAVE_CXX14 an explicit config
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2114:
URL: https://github.com/apache/incubator-nuttx/pull/2114#discussion_r511848637
##########
File path: libs/libxx/Kconfig
##########
@@ -22,7 +22,7 @@ config HAVE_CXX
config HAVE_CXX14
bool "C++14 support"
- default n
+ default y
Review comment:
But uClibc++ doesn't support CXX14
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #2114: Make CONFIG_HAVE_CXX14 an explicit config
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #2114:
URL: https://github.com/apache/incubator-nuttx/pull/2114#discussion_r512334868
##########
File path: libs/libxx/Kconfig
##########
@@ -22,7 +22,7 @@ config HAVE_CXX
config HAVE_CXX14
bool "C++14 support"
- default n
+ default y
Review comment:
> The HAVE should be default `n` and selected from libcxx to be `y`
can you explain why?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #2114: Make CONFIG_HAVE_CXX14 an explicit config
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #2114:
URL: https://github.com/apache/incubator-nuttx/pull/2114#discussion_r514755503
##########
File path: libs/libxx/Kconfig
##########
@@ -22,7 +22,7 @@ config HAVE_CXX
config HAVE_CXX14
bool "C++14 support"
- default n
+ default y
Review comment:
things to consider:
a. c++ versions the compiler supports
b1. c++ versions the c++ runtime supports (runtime X provides necessary functionalities to run apps written in c++Y)
b2. c++ versions the c++ runtime needs (runtime X itself needs to be built with -std=Y)
c1. c++ version to use to build c++ portions of nuttx, including c++ runtime
c2. c++ version to use to build an app
d. the compiler flag for c1/c2. (eg. -std=c++14 in case of gcc/clang)
random thoughts:
- c1 can be a Kconfig "choice"
- c1. is restricted by b1 and b2.
i hope the restrictions here can be represented solely with Kconfig directives.
- right now c1 == c2. it's nicer to let users to specify c2. probably within the app's Makefile.
- a. and d. belong to Make.defs/Toolchain.defs
probably a set of make vars like:
CXXSTDFLAGS_cxx14=-std=c++14
CXXSTDFLAGS_cxx17=-std=c++17
- the current CONFIG_HAVE_CXX14 has no good use. it's simpler to check __cplusplus directly if necessary.
- right now, in case of NUTTX_LIBXX, b1. is controlled by c1. (via CONFIG_HAVE_CXX14)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2114: Make CONFIG_HAVE_CXX14 an explicit config
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2114:
URL: https://github.com/apache/incubator-nuttx/pull/2114#discussion_r511910681
##########
File path: libs/libxx/Kconfig
##########
@@ -22,7 +22,7 @@ config HAVE_CXX
config HAVE_CXX14
bool "C++14 support"
- default n
+ default y
Review comment:
No, uClibc++ just use c++98 feature.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2114: Make CONFIG_HAVE_CXX14 an explicit config
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2114:
URL: https://github.com/apache/incubator-nuttx/pull/2114#discussion_r512753166
##########
File path: libs/libxx/Kconfig
##########
@@ -22,7 +22,7 @@ config HAVE_CXX
config HAVE_CXX14
bool "C++14 support"
- default n
+ default y
Review comment:
But, let user/c++ library select HAVE_CXX14 is useful to guide Make.defs/Toolchain.defs add the appropriate flag instead hardcoding now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d commented on a change in pull request #2114: Make CONFIG_HAVE_CXX14 an explicit config
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2114:
URL: https://github.com/apache/incubator-nuttx/pull/2114#discussion_r511930195
##########
File path: libs/libxx/Kconfig
##########
@@ -22,7 +22,7 @@ config HAVE_CXX
config HAVE_CXX14
bool "C++14 support"
- default n
+ default y
Review comment:
The HAVE should be default `n` and selected from libcxx to be `y`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2114: Make CONFIG_HAVE_CXX14 an explicit config
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2114:
URL: https://github.com/apache/incubator-nuttx/pull/2114#discussion_r512481252
##########
File path: libs/libxx/Kconfig
##########
@@ -22,7 +22,7 @@ config HAVE_CXX
config HAVE_CXX14
bool "C++14 support"
- default n
+ default y
Review comment:
No, if you enable LIBCXX c++14 support, you must pass --c++14 to gcc compiler too, because there are many core language feature is added in c++14, LIBCXX can't pass the compiler phase without it. You can see the new feature here:
https://gcc.gnu.org/projects/cxx-status.html
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d commented on a change in pull request #2114: Make CONFIG_HAVE_CXX14 an explicit config
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2114:
URL: https://github.com/apache/incubator-nuttx/pull/2114#discussion_r512369411
##########
File path: libs/libxx/Kconfig
##########
@@ -22,7 +22,7 @@ config HAVE_CXX
config HAVE_CXX14
bool "C++14 support"
- default n
+ default y
Review comment:
I believe only libcxx (LLVM's) only implements C++14 support, the other two are only C++98 (or C++03 at most).
But I'm not really sure what you are trying to accomplish with this. Compiler support for a given standard is obtained from macros defined by the compiler (gcc defines `__cplusplus` to the version number supported, or the one indicated during build). I understand you where trying to distinguish whether the library being built implements or not C++14. I answered according to that: make HAVE_CXX14 be default `n` and have LIBCXX Kconfig select to `y`. I don't know what else to answer as I'm not sure what we're trying to fix.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 closed pull request #2114: Make CONFIG_HAVE_CXX14 an explicit config
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 closed pull request #2114:
URL: https://github.com/apache/incubator-nuttx/pull/2114
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #2114: Make CONFIG_HAVE_CXX14 an explicit config
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #2114:
URL: https://github.com/apache/incubator-nuttx/pull/2114#discussion_r511870379
##########
File path: libs/libxx/Kconfig
##########
@@ -22,7 +22,7 @@ config HAVE_CXX
config HAVE_CXX14
bool "C++14 support"
- default n
+ default y
Review comment:
does uclibc++ use HAVE_CXX14? i thought it was only used by libs/libxx/libxx_*.cxx files.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #2114: Make CONFIG_HAVE_CXX14 an explicit config
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #2114:
URL: https://github.com/apache/incubator-nuttx/pull/2114#discussion_r512311544
##########
File path: libs/libxx/Kconfig
##########
@@ -22,7 +22,7 @@ config HAVE_CXX
config HAVE_CXX14
bool "C++14 support"
- default n
+ default y
Review comment:
my question was, "does uclibc++ use this CONFIG_HAVE_CXX14 macro"?
my understanding is that this is specific to libs/libxx/libxx_*.cxx files. (btw, it's better to name this variant of c++ library)
neither libcxx or uclibc++ uses it.
am i missing something?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2114: Make CONFIG_HAVE_CXX14 an explicit config
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2114:
URL: https://github.com/apache/incubator-nuttx/pull/2114#discussion_r512436826
##########
File path: libs/libxx/Kconfig
##########
@@ -22,7 +22,7 @@ config HAVE_CXX
config HAVE_CXX14
bool "C++14 support"
- default n
+ default y
Review comment:
CONFIG_HAVE_CXX14 should be used by Make.defs/Toolchain.defs to pass --c++14 to compiler.
So I think @v01d suggestion is correct that:
1. LIBCXX select CONFIG_HAVE_CXX14 and then let compiler enable c++14
2. UCLIBCXX don't select CONFIG_HAVE_CXX14 and then keep the old compiler behaviour
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #2114: Make CONFIG_HAVE_CXX14 an explicit config
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #2114:
URL: https://github.com/apache/incubator-nuttx/pull/2114#discussion_r512441350
##########
File path: libs/libxx/Kconfig
##########
@@ -22,7 +22,7 @@ config HAVE_CXX
config HAVE_CXX14
bool "C++14 support"
- default n
+ default y
Review comment:
using LIBCXX and using --std=c++14 for applications are two very different things. we should not do that way.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #2114: Make CONFIG_HAVE_CXX14 an explicit config
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #2114:
URL: https://github.com/apache/incubator-nuttx/pull/2114#discussion_r512354821
##########
File path: libs/libxx/Kconfig
##########
@@ -22,7 +22,7 @@ config HAVE_CXX
config HAVE_CXX14
bool "C++14 support"
- default n
+ default y
Review comment:
my understanding that there are three implementations.
* libxx_*.cxx
* libcxx
* uclibc+
and CONFIG_HAVE_CXX14 is relevant to the only first one.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #2114: Make CONFIG_HAVE_CXX14 an explicit config
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #2114:
URL: https://github.com/apache/incubator-nuttx/pull/2114#discussion_r512419343
##########
File path: libs/libxx/Kconfig
##########
@@ -22,7 +22,7 @@ config HAVE_CXX
config HAVE_CXX14
bool "C++14 support"
- default n
+ default y
Review comment:
@v01d
libxx_*.cxx has operator delete variants for C++14, which is conditional with CONFIG_HAVE_CXX14.
and, as far as i know, it's the only use of CONFIG_HAVE_CXX14 in the tree.
ie. neither LIBCXX or UCLIBCXX is relevant.
so, i'm puzzled by your suggestion to make LIBCXX select it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #2114: Make CONFIG_HAVE_CXX14 an explicit config
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #2114:
URL: https://github.com/apache/incubator-nuttx/pull/2114#discussion_r512355696
##########
File path: libs/libxx/Kconfig
##########
@@ -22,7 +22,7 @@ config HAVE_CXX
config HAVE_CXX14
bool "C++14 support"
- default n
+ default y
Review comment:
@v01d or, are you just suggesting those configs should not have been named HAVE_xxx?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #2114: Make CONFIG_HAVE_CXX14 an explicit config
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #2114:
URL: https://github.com/apache/incubator-nuttx/pull/2114#issuecomment-762374729
@yamt I think this patch don't need anymore?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2114: Make CONFIG_HAVE_CXX14 an explicit config
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2114:
URL: https://github.com/apache/incubator-nuttx/pull/2114#discussion_r513236475
##########
File path: libs/libxx/Kconfig
##########
@@ -22,7 +22,7 @@ config HAVE_CXX
config HAVE_CXX14
bool "C++14 support"
- default n
+ default y
Review comment:
libxx can work with the recent compiler by preprocess and provide the different implementation or remove the implementation as needed(maybe >= c++11). so it's good to let's user select which c++ starndard to be used and adapter the code to confirm it. So I think only the change in libs/libxx/cxx.defs is needed, @yamt do you think so?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2114: Make CONFIG_HAVE_CXX14 an explicit config
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2114:
URL: https://github.com/apache/incubator-nuttx/pull/2114#discussion_r512481252
##########
File path: libs/libxx/Kconfig
##########
@@ -22,7 +22,7 @@ config HAVE_CXX
config HAVE_CXX14
bool "C++14 support"
- default n
+ default y
Review comment:
No, if you enable LIBCXX c++14 support, you must pass --c++14 to gcc compiler too, because there are many core language features are added in c++14, LIBCXX can't pass the compiler phase without it. You can see the new feature here:
https://gcc.gnu.org/projects/cxx-status.html
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #2114: Make CONFIG_HAVE_CXX14 an explicit config
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #2114:
URL: https://github.com/apache/incubator-nuttx/pull/2114#issuecomment-766146907
CONFIG_HAVE_CXX14 should detect autoamtically in compiler.h, not from Kconfig, let's close this PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2114: Make CONFIG_HAVE_CXX14 an explicit config
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2114:
URL: https://github.com/apache/incubator-nuttx/pull/2114#discussion_r512436826
##########
File path: libs/libxx/Kconfig
##########
@@ -22,7 +22,7 @@ config HAVE_CXX
config HAVE_CXX14
bool "C++14 support"
- default n
+ default y
Review comment:
CONFIG_HAVE_CXX14 should be used by Make.defs/Toolchain.defs to pass --c++14 to compiler too.
So I think @v01d suggestion is correct that:
1. LIBCXX select CONFIG_HAVE_CXX14 and then let compiler enable c++14
2. UCLIBCXX don't select CONFIG_HAVE_CXX14 and then keep the old compiler behaviour
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d commented on a change in pull request #2114: Make CONFIG_HAVE_CXX14 an explicit config
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2114:
URL: https://github.com/apache/incubator-nuttx/pull/2114#discussion_r512757175
##########
File path: libs/libxx/Kconfig
##########
@@ -22,7 +22,7 @@ config HAVE_CXX
config HAVE_CXX14
bool "C++14 support"
- default n
+ default y
Review comment:
Again, if HAVE_CXX14 refers to compiler support, the correct way is to *read* this from what compiler tells us. The user is in control of which standard to select during build by passing -std=c++14 or any other version. But that would be a separate flag than a *have* flag. In any case, I think that each library should pass the appropriate flags to the compiler since uClibC/libxx may probably give problems when the compiler builds using latest version.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org