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 2022/03/28 17:59:02 UTC
[GitHub] [incubator-nuttx] gustavonihei opened a new pull request #5897: ESP32{S2/S3/C3}: Enable support for C++ applications
gustavonihei opened a new pull request #5897:
URL: https://github.com/apache/incubator-nuttx/pull/5897
## Summary
This PR intends to ease the development of C++ applications on the following chips:
- **ESP32**
- **ESP32-S2**
- **ESP32-C3**
- **ESP32-S3**
## Impact
In order to achieve the integration of `libc++` library, the following bug fixes were applied:
- [xtensa: Move XCHAL_SWINT_CALL definition into syscall header](https://github.com/apache/incubator-nuttx/commit/d948b61f60ed29cecf4ecf670f47d89a76dc2c11)
- [xtensa: Build OS-assisted atomic operations on ESP32-S2](https://github.com/apache/incubator-nuttx/commit/fe1f5e2d81f3bc91265091fdeb1cb4bd6e6c0303)
- [risc-v/esp32c3: Fix inclusion of C++ constructors/destructors table](https://github.com/apache/incubator-nuttx/commit/c2aa550ebde95d39519656079aaf9e4101ab93a9)
Furthermore, the following patches were applied on `libc++`:
- [libcxx: Add patch for preventing redefinition of PS macro on Xtensa](https://github.com/apache/incubator-nuttx/commit/88efa5e646179f7896eba877a4105ec1387a1cee): Submitted on https://reviews.llvm.org/D122479
- [libcxx: Fix libc++ for archs without conditional load/store support](https://github.com/apache/incubator-nuttx/commit/8b325773cf0e9044e27f039d64ea727dffbab56c): Backported to 12.0.0 from https://reviews.llvm.org/D118391, already merged upstream.
## Testing
- `esp32-devkitc:cxx`
- `esp32s2-saola-1:cxx`
- `esp32c3-devkit:cxx`
- `esp32s3-devkit:cxx`
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #5897: ESP32{S2/S3/C3}: Enable support for C++ applications
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5897:
URL: https://github.com/apache/incubator-nuttx/pull/5897#discussion_r837461032
##########
File path: arch/xtensa/src/lx6/Toolchain.defs
##########
@@ -55,8 +59,15 @@ else
MAXOPTIMIZATION += -fomit-frame-pointer
endif
-ifeq ($(CONFIG_MM_KASAN),y)
- ARCHCPUFLAGS += -fsanitize=kernel-address
+ARCHCFLAGS = -mlongcalls
+ARCHCXXFLAGS = -mlongcalls
Review comment:
It was already required, it was declared in `ARCHCFLAGS` of each Xtensa-based board.
And `ARCHCFLAGS` was expanded into `ARCHCXXFLAGS`.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5897: ESP32{S2/S3/C3}: Enable support for C++ applications
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5897:
URL: https://github.com/apache/incubator-nuttx/pull/5897#discussion_r837625638
##########
File path: boards/risc-v/esp32c3/esp32c3-devkit/scripts/Make.defs
##########
@@ -41,8 +41,8 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
ARCHOPTIMIZATION += $(MAXOPTIMIZATION) -fno-strict-aliasing
endif
-ARCHCFLAGS = -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
-ARCHCXXFLAGS = $(ARCHCFLAGS) -fno-exceptions -fcheck-new -fno-rtti
+ARCHCFLAGS += -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
+ARCHCXXFLAGS += -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0 -std=c++17
Review comment:
Ok. Let's keep those separate and introduce a common option later
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #5897: ESP32{S2/S3/C3}: Enable support for C++ applications
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5897:
URL: https://github.com/apache/incubator-nuttx/pull/5897#discussion_r837482498
##########
File path: boards/risc-v/esp32c3/esp32c3-devkit/scripts/Make.defs
##########
@@ -41,8 +41,8 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
ARCHOPTIMIZATION += $(MAXOPTIMIZATION) -fno-strict-aliasing
endif
-ARCHCFLAGS = -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
-ARCHCXXFLAGS = $(ARCHCFLAGS) -fno-exceptions -fcheck-new -fno-rtti
+ARCHCFLAGS += -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
+ARCHCXXFLAGS += -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0 -std=c++17
Review comment:
Well, this example is somewhat absurd, but it still is an example:
```bash
$ g++ -std=c11 ~/hello_main.c
cc1plus: warning: command-line option ‘-std=c11’ is valid for C/ObjC but not for C++
```
Still searching for a `-f` option that `g++` may complain.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #5897: ESP32{S2/S3/C3}: Enable support for C++ applications
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5897:
URL: https://github.com/apache/incubator-nuttx/pull/5897#discussion_r837450190
##########
File path: boards/risc-v/esp32c3/esp32c3-devkit/scripts/Make.defs
##########
@@ -41,8 +41,8 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
ARCHOPTIMIZATION += $(MAXOPTIMIZATION) -fno-strict-aliasing
endif
-ARCHCFLAGS = -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
-ARCHCXXFLAGS = $(ARCHCFLAGS) -fno-exceptions -fcheck-new -fno-rtti
+ARCHCFLAGS += -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
+ARCHCXXFLAGS += -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0 -std=c++17
Review comment:
But now `ARCHCFLAGS` may contain C-specific flags defined in `Toolchain.defs` file, therefore we cannot simply expand it here as part of `ARCHCXXFLAGS`.
This refactor would then require another variable for holding flags common to both C and C++, which needs to evaluate if it is indeed worth it.
I think another variable (e.g. `COMMONFLAGS`) will just contribute to hiding the information, which is bad for human comprehension.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #5897: ESP32{S2/S3/C3}: Enable support for C++ applications
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5897:
URL: https://github.com/apache/incubator-nuttx/pull/5897#discussion_r836721180
##########
File path: boards/risc-v/esp32c3/esp32c3-devkit/scripts/Make.defs
##########
@@ -42,7 +42,13 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
endif
ARCHCFLAGS = -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
-ARCHCXXFLAGS = $(ARCHCFLAGS) -fno-exceptions -fcheck-new -fno-rtti
+ARCHCXXFLAGS = $(ARCHCFLAGS) -std=c++17
+ifneq ($(CONFIG_CXX_EXCEPTION),y)
+ ARCHCXXFLAGS += -fno-exceptions -fcheck-new
+endif
+ifneq ($(CONFIG_CXX_RTTI),y)
+ ARCHCXXFLAGS += -fno-rtti
+endif
Review comment:
Good idea, I'll try to apply this change to Xtensa and RISC-V chips
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5897: ESP32{S2/S3/C3}: Enable support for C++ applications
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5897:
URL: https://github.com/apache/incubator-nuttx/pull/5897#discussion_r837243901
##########
File path: arch/xtensa/src/lx6/Toolchain.defs
##########
@@ -55,8 +59,15 @@ else
MAXOPTIMIZATION += -fomit-frame-pointer
endif
-ifeq ($(CONFIG_MM_KASAN),y)
- ARCHCPUFLAGS += -fsanitize=kernel-address
+ARCHCFLAGS = -mlongcalls
+ARCHCXXFLAGS = -mlongcalls
Review comment:
So why `-mlongcalls` was not needed before?
##########
File path: boards/risc-v/esp32c3/esp32c3-devkit/scripts/Make.defs
##########
@@ -41,8 +41,8 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
ARCHOPTIMIZATION += $(MAXOPTIMIZATION) -fno-strict-aliasing
endif
-ARCHCFLAGS = -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
-ARCHCXXFLAGS = $(ARCHCFLAGS) -fno-exceptions -fcheck-new -fno-rtti
+ARCHCFLAGS += -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
+ARCHCXXFLAGS += -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0 -std=c++17
Review comment:
I was almost starting to do the opposite change and do `ARCHCXXFLAGS += $(ARCHCFLAGS) ....` for all the platforms as sometimes `-fno-common -ffunction-sections -fdata-sections` are not added to `ARCHCXXFLAGS`
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5897: ESP32{S2/S3/C3}: Enable support for C++ applications
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5897:
URL: https://github.com/apache/incubator-nuttx/pull/5897#discussion_r837486269
##########
File path: boards/risc-v/esp32c3/esp32c3-devkit/scripts/Make.defs
##########
@@ -41,8 +41,8 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
ARCHOPTIMIZATION += $(MAXOPTIMIZATION) -fno-strict-aliasing
endif
-ARCHCFLAGS = -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
-ARCHCXXFLAGS = $(ARCHCFLAGS) -fno-exceptions -fcheck-new -fno-rtti
+ARCHCFLAGS += -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
+ARCHCXXFLAGS += -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0 -std=c++17
Review comment:
yeah. this might be the case
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5897: ESP32{S2/S3/C3}: Enable support for C++ applications
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5897:
URL: https://github.com/apache/incubator-nuttx/pull/5897#discussion_r837459641
##########
File path: boards/risc-v/esp32c3/esp32c3-devkit/scripts/Make.defs
##########
@@ -41,8 +41,8 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
ARCHOPTIMIZATION += $(MAXOPTIMIZATION) -fno-strict-aliasing
endif
-ARCHCFLAGS = -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
-ARCHCXXFLAGS = $(ARCHCFLAGS) -fno-exceptions -fcheck-new -fno-rtti
+ARCHCFLAGS += -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
+ARCHCXXFLAGS += -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0 -std=c++17
Review comment:
Do you have an example of "C-specific flags" that will not fit to C++ compiler input?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #5897: ESP32{S2/S3/C3}: Enable support for C++ applications
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5897:
URL: https://github.com/apache/incubator-nuttx/pull/5897#discussion_r837844744
##########
File path: arch/xtensa/src/lx6/Toolchain.defs
##########
@@ -55,8 +59,15 @@ else
MAXOPTIMIZATION += -fomit-frame-pointer
endif
-ifeq ($(CONFIG_MM_KASAN),y)
- ARCHCPUFLAGS += -fsanitize=kernel-address
+ARCHCFLAGS = -mlongcalls
+ARCHCXXFLAGS = -mlongcalls
Review comment:
Done.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5897: ESP32{S2/S3/C3}: Enable support for C++ applications
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5897:
URL: https://github.com/apache/incubator-nuttx/pull/5897#discussion_r836802141
##########
File path: arch/xtensa/src/lx6/Toolchain.defs
##########
@@ -55,8 +59,14 @@ else
MAXOPTIMIZATION += -fomit-frame-pointer
endif
-ifeq ($(CONFIG_MM_KASAN),y)
- ARCHCPUFLAGS += -fsanitize=kernel-address
+CXXFLAGS =
+
+ifneq ($(CONFIG_CXX_EXCEPTION),y)
+ CXXFLAGS += -fno-exceptions -fcheck-new
Review comment:
Why not to reuse `ARCHCXXFLAGS` and leave `CXXFLAGS :=` as is? I mean `ARCHCXXFLAGS += -fno-exceptions -fcheck-new`? The same as it is done few lines above for `ARCHCPUFLAGS += -fsanitize=kernel-address`
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #5897: ESP32{S2/S3/C3}: Enable support for C++ applications
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5897:
URL: https://github.com/apache/incubator-nuttx/pull/5897#discussion_r837465003
##########
File path: boards/risc-v/esp32c3/esp32c3-devkit/scripts/Make.defs
##########
@@ -41,8 +41,8 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
ARCHOPTIMIZATION += $(MAXOPTIMIZATION) -fno-strict-aliasing
endif
-ARCHCFLAGS = -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
-ARCHCXXFLAGS = $(ARCHCFLAGS) -fno-exceptions -fcheck-new -fno-rtti
+ARCHCFLAGS += -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
+ARCHCXXFLAGS += -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0 -std=c++17
Review comment:
I know for sure that the opposite is true, e.g. passing `-fno-rtti`to the C frontend results in a compiler warning.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #5897: ESP32{S2/S3/C3}: Enable support for C++ applications
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #5897:
URL: https://github.com/apache/incubator-nuttx/pull/5897
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #5897: ESP32{S2/S3/C3}: Enable support for C++ applications
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5897:
URL: https://github.com/apache/incubator-nuttx/pull/5897#discussion_r837445358
##########
File path: arch/xtensa/src/lx6/Toolchain.defs
##########
@@ -55,8 +59,15 @@ else
MAXOPTIMIZATION += -fomit-frame-pointer
endif
-ifeq ($(CONFIG_MM_KASAN),y)
- ARCHCPUFLAGS += -fsanitize=kernel-address
Review comment:
Just to move the usage of `ARCHCPUFLAGS` closer to its declaration.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5897: ESP32{S2/S3/C3}: Enable support for C++ applications
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5897:
URL: https://github.com/apache/incubator-nuttx/pull/5897#discussion_r837836506
##########
File path: arch/xtensa/src/lx6/Toolchain.defs
##########
@@ -55,8 +59,15 @@ else
MAXOPTIMIZATION += -fomit-frame-pointer
endif
-ifeq ($(CONFIG_MM_KASAN),y)
- ARCHCPUFLAGS += -fsanitize=kernel-address
+ARCHCFLAGS = -mlongcalls
+ARCHCXXFLAGS = -mlongcalls
Review comment:
@gustavonihei look like this more like ARCHCPUFLAGS flag?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #5897: ESP32{S2/S3/C3}: Enable support for C++ applications
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5897:
URL: https://github.com/apache/incubator-nuttx/pull/5897#discussion_r837490157
##########
File path: boards/risc-v/esp32c3/esp32c3-devkit/scripts/Make.defs
##########
@@ -41,8 +41,8 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
ARCHOPTIMIZATION += $(MAXOPTIMIZATION) -fno-strict-aliasing
endif
-ARCHCFLAGS = -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
-ARCHCXXFLAGS = $(ARCHCFLAGS) -fno-exceptions -fcheck-new -fno-rtti
+ARCHCFLAGS += -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
+ARCHCXXFLAGS += -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0 -std=c++17
Review comment:
```bash
$ g++ -fallow-parameterless-variadic-functions ~/hello_main.c
cc1plus: warning: command-line option ‘-fallow-parameterless-variadic-functions’ is valid for C/ObjC but not for C++
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #5897: ESP32{S2/S3/C3}: Enable support for C++ applications
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5897:
URL: https://github.com/apache/incubator-nuttx/pull/5897#discussion_r837465003
##########
File path: boards/risc-v/esp32c3/esp32c3-devkit/scripts/Make.defs
##########
@@ -41,8 +41,8 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
ARCHOPTIMIZATION += $(MAXOPTIMIZATION) -fno-strict-aliasing
endif
-ARCHCFLAGS = -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
-ARCHCXXFLAGS = $(ARCHCFLAGS) -fno-exceptions -fcheck-new -fno-rtti
+ARCHCFLAGS += -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
+ARCHCXXFLAGS += -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0 -std=c++17
Review comment:
I know for sure that the opposite is true, e.g. passing `-fno-rtti`to the GCC C frontend results in a compiler warning.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #5897: ESP32{S2/S3/C3}: Enable support for C++ applications
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5897:
URL: https://github.com/apache/incubator-nuttx/pull/5897#discussion_r836812661
##########
File path: arch/xtensa/src/lx6/Toolchain.defs
##########
@@ -55,8 +59,14 @@ else
MAXOPTIMIZATION += -fomit-frame-pointer
endif
-ifeq ($(CONFIG_MM_KASAN),y)
- ARCHCPUFLAGS += -fsanitize=kernel-address
+CXXFLAGS =
+
+ifneq ($(CONFIG_CXX_EXCEPTION),y)
+ CXXFLAGS += -fno-exceptions -fcheck-new
Review comment:
It is also an option. At first I preferred to leave `ARCHCFLAGS` and `ARCHCXXFLAGS` restricted to the `board` definition.
But after your comment, I believe it makes sense for the `Toolchain.defs` file to define arch-common C and C++ flags, and each board will append application-specific flags.
I'll submit another patch with this approach.
Thanks for the feedback.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5897: ESP32{S2/S3/C3}: Enable support for C++ applications
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5897:
URL: https://github.com/apache/incubator-nuttx/pull/5897#discussion_r837250371
##########
File path: boards/risc-v/esp32c3/esp32c3-devkit/scripts/Make.defs
##########
@@ -41,8 +41,8 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
ARCHOPTIMIZATION += $(MAXOPTIMIZATION) -fno-strict-aliasing
endif
-ARCHCFLAGS = -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
-ARCHCXXFLAGS = $(ARCHCFLAGS) -fno-exceptions -fcheck-new -fno-rtti
+ARCHCFLAGS += -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
+ARCHCXXFLAGS += -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0 -std=c++17
Review comment:
yes, it's better to add C/C++ shared option to ARCHCFLAGS and C++ specifical option to ARCHCXXFLAGS, so we can remove the unnecessary duplication.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #5897: ESP32{S2/S3/C3}: Enable support for C++ applications
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5897:
URL: https://github.com/apache/incubator-nuttx/pull/5897#discussion_r837450190
##########
File path: boards/risc-v/esp32c3/esp32c3-devkit/scripts/Make.defs
##########
@@ -41,8 +41,8 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
ARCHOPTIMIZATION += $(MAXOPTIMIZATION) -fno-strict-aliasing
endif
-ARCHCFLAGS = -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
-ARCHCXXFLAGS = $(ARCHCFLAGS) -fno-exceptions -fcheck-new -fno-rtti
+ARCHCFLAGS += -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
+ARCHCXXFLAGS += -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0 -std=c++17
Review comment:
But now `ARCHCFLAGS` may contain C-specific flags defined in `Toolchain.defs` file, therefore we cannot simply expand it here as part of `ARCHCXXFLAGS`.
This refactor would then require another variable for holding flags common to both C and C++, which needs to evaluate if it is indeed worth 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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #5897: ESP32{S2/S3/C3}: Enable support for C++ applications
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5897:
URL: https://github.com/apache/incubator-nuttx/pull/5897#discussion_r837463708
##########
File path: boards/risc-v/esp32c3/esp32c3-devkit/scripts/Make.defs
##########
@@ -41,8 +41,8 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
ARCHOPTIMIZATION += $(MAXOPTIMIZATION) -fno-strict-aliasing
endif
-ARCHCFLAGS = -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
-ARCHCXXFLAGS = $(ARCHCFLAGS) -fno-exceptions -fcheck-new -fno-rtti
+ARCHCFLAGS += -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
+ARCHCXXFLAGS += -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0 -std=c++17
Review comment:
Not from my mind, but I may be convinced I can find any documentation stating that the C++ frontend accepts every flag also accepted by the C frontend.
##########
File path: boards/risc-v/esp32c3/esp32c3-devkit/scripts/Make.defs
##########
@@ -41,8 +41,8 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
ARCHOPTIMIZATION += $(MAXOPTIMIZATION) -fno-strict-aliasing
endif
-ARCHCFLAGS = -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
-ARCHCXXFLAGS = $(ARCHCFLAGS) -fno-exceptions -fcheck-new -fno-rtti
+ARCHCFLAGS += -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
+ARCHCXXFLAGS += -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0 -std=c++17
Review comment:
Not from my mind, but I may be convinced if I can find any documentation stating that the C++ frontend accepts every flag also accepted by the C frontend.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5897: ESP32{S2/S3/C3}: Enable support for C++ applications
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5897:
URL: https://github.com/apache/incubator-nuttx/pull/5897#discussion_r837468519
##########
File path: boards/risc-v/esp32c3/esp32c3-devkit/scripts/Make.defs
##########
@@ -41,8 +41,8 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
ARCHOPTIMIZATION += $(MAXOPTIMIZATION) -fno-strict-aliasing
endif
-ARCHCFLAGS = -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
-ARCHCXXFLAGS = $(ARCHCFLAGS) -fno-exceptions -fcheck-new -fno-rtti
+ARCHCFLAGS += -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
+ARCHCXXFLAGS += -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0 -std=c++17
Review comment:
> I know for sure that the opposite is true, e.g. passing `-fno-rtti` to the GCC C frontend results in a compiler warning.
That is true for sure. I will try to find documentation to proof it, but since C is a subset of C++ (actually C++ is forced to maintain compatibility with C) I do not see any reason why C options should generate any warnings or be rejected by C++ frontend
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #5897: ESP32{S2/S3/C3}: Enable support for C++ applications
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5897:
URL: https://github.com/apache/incubator-nuttx/pull/5897#discussion_r837557318
##########
File path: arch/risc-v/src/common/Toolchain.defs
##########
@@ -59,6 +59,17 @@ else
MAXOPTIMIZATION += -fomit-frame-pointer
endif
+ARCHCFLAGS =
Review comment:
Done.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #5897: ESP32{S2/S3/C3}: Enable support for C++ applications
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5897:
URL: https://github.com/apache/incubator-nuttx/pull/5897#discussion_r837450190
##########
File path: boards/risc-v/esp32c3/esp32c3-devkit/scripts/Make.defs
##########
@@ -41,8 +41,8 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
ARCHOPTIMIZATION += $(MAXOPTIMIZATION) -fno-strict-aliasing
endif
-ARCHCFLAGS = -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
-ARCHCXXFLAGS = $(ARCHCFLAGS) -fno-exceptions -fcheck-new -fno-rtti
+ARCHCFLAGS += -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
+ARCHCXXFLAGS += -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0 -std=c++17
Review comment:
But now `ARCHCFLAGS` may contain C-specific flags defined in `Toolchain.defs` file, therefore we cannot simply expand it here as part of `ARCHCXXFLAGS`. Doing so could result in unintended compiler warnings.
This refactor would then require another variable for holding flags common to both C and C++, which needs to evaluate if it is indeed worth it.
I think another variable (e.g. `COMMONFLAGS`) will just contribute to hiding the information, which is bad for human comprehension.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5897: ESP32{S2/S3/C3}: Enable support for C++ applications
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5897:
URL: https://github.com/apache/incubator-nuttx/pull/5897#discussion_r836706804
##########
File path: boards/risc-v/esp32c3/esp32c3-devkit/scripts/Make.defs
##########
@@ -42,7 +42,13 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
endif
ARCHCFLAGS = -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
-ARCHCXXFLAGS = $(ARCHCFLAGS) -fno-exceptions -fcheck-new -fno-rtti
+ARCHCXXFLAGS = $(ARCHCFLAGS) -std=c++17
+ifneq ($(CONFIG_CXX_EXCEPTION),y)
+ ARCHCXXFLAGS += -fno-exceptions -fcheck-new
+endif
+ifneq ($(CONFIG_CXX_RTTI),y)
+ ARCHCXXFLAGS += -fno-rtti
+endif
Review comment:
@gustavonihei this is common option, how about we move them to Toolschain.defs to avoid the duplication?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #5897: ESP32{S2/S3/C3}: Enable support for C++ applications
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5897:
URL: https://github.com/apache/incubator-nuttx/pull/5897#discussion_r837844877
##########
File path: arch/xtensa/src/lx7/Toolchain.defs
##########
@@ -55,8 +59,15 @@ else
MAXOPTIMIZATION += -fomit-frame-pointer
endif
-ifeq ($(CONFIG_MM_KASAN),y)
- ARCHCPUFLAGS += -fsanitize=kernel-address
+ARCHCFLAGS = -mlongcalls
+ARCHCXXFLAGS = -mlongcalls
Review comment:
Done.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #5897: ESP32{S2/S3/C3}: Enable support for C++ applications
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5897:
URL: https://github.com/apache/incubator-nuttx/pull/5897#discussion_r837842145
##########
File path: arch/xtensa/src/lx6/Toolchain.defs
##########
@@ -55,8 +59,15 @@ else
MAXOPTIMIZATION += -fomit-frame-pointer
endif
-ifeq ($(CONFIG_MM_KASAN),y)
- ARCHCPUFLAGS += -fsanitize=kernel-address
+ARCHCFLAGS = -mlongcalls
+ARCHCXXFLAGS = -mlongcalls
Review comment:
Indeed, I'll fix it! Thanks
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5897: ESP32{S2/S3/C3}: Enable support for C++ applications
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5897:
URL: https://github.com/apache/incubator-nuttx/pull/5897#discussion_r837837204
##########
File path: arch/xtensa/src/lx7/Toolchain.defs
##########
@@ -55,8 +59,15 @@ else
MAXOPTIMIZATION += -fomit-frame-pointer
endif
-ifeq ($(CONFIG_MM_KASAN),y)
- ARCHCPUFLAGS += -fsanitize=kernel-address
+ARCHCFLAGS = -mlongcalls
+ARCHCXXFLAGS = -mlongcalls
Review comment:
ditto
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5897: ESP32{S2/S3/C3}: Enable support for C++ applications
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5897:
URL: https://github.com/apache/incubator-nuttx/pull/5897#discussion_r837045562
##########
File path: arch/risc-v/src/common/Toolchain.defs
##########
@@ -59,6 +59,17 @@ else
MAXOPTIMIZATION += -fomit-frame-pointer
endif
+ARCHCFLAGS =
Review comment:
could you update arm/ too?
##########
File path: arch/xtensa/src/lx6/Toolchain.defs
##########
@@ -55,8 +59,15 @@ else
MAXOPTIMIZATION += -fomit-frame-pointer
endif
-ifeq ($(CONFIG_MM_KASAN),y)
- ARCHCPUFLAGS += -fsanitize=kernel-address
Review comment:
why change these lines
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #5897: ESP32{S2/S3/C3}: Enable support for C++ applications
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5897:
URL: https://github.com/apache/incubator-nuttx/pull/5897#discussion_r837500695
##########
File path: boards/risc-v/esp32c3/esp32c3-devkit/scripts/Make.defs
##########
@@ -41,8 +41,8 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
ARCHOPTIMIZATION += $(MAXOPTIMIZATION) -fno-strict-aliasing
endif
-ARCHCFLAGS = -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
-ARCHCXXFLAGS = $(ARCHCFLAGS) -fno-exceptions -fcheck-new -fno-rtti
+ARCHCFLAGS += -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
+ARCHCXXFLAGS += -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0 -std=c++17
Review comment:
> [...], but since C is a subset of C++ (actually C++ is forced to maintain compatibility with C)
I wouldn't bet on this. C and C++ parted ways some years ago. Even according to [ISO CPP](https://isocpp.org/wiki/faq/c#is-c-a-subset), C++ is a superset of C95, but both evolved on their own.
That's why I find hard to believe that the C++ frontend would accept every flag for the C frontend.
But either way, I am usually not in favor of repeating ourselves, but in this case I would prefer to repeat this small amount of flags for readability.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #5897: ESP32{S2/S3/C3}: Enable support for C++ applications
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5897:
URL: https://github.com/apache/incubator-nuttx/pull/5897#discussion_r836743390
##########
File path: boards/risc-v/esp32c3/esp32c3-devkit/scripts/Make.defs
##########
@@ -42,7 +42,13 @@ ifneq ($(CONFIG_DEBUG_NOOPT),y)
endif
ARCHCFLAGS = -fno-common -ffunction-sections -fdata-sections -msmall-data-limit=0
-ARCHCXXFLAGS = $(ARCHCFLAGS) -fno-exceptions -fcheck-new -fno-rtti
+ARCHCXXFLAGS = $(ARCHCFLAGS) -std=c++17
+ifneq ($(CONFIG_CXX_EXCEPTION),y)
+ ARCHCXXFLAGS += -fno-exceptions -fcheck-new
+endif
+ifneq ($(CONFIG_CXX_RTTI),y)
+ ARCHCXXFLAGS += -fno-rtti
+endif
Review comment:
Done, `CXXFLAGS` is now initialized on `Toolchain.defs` file of each architecture.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #5897: ESP32{S2/S3/C3}: Enable support for C++ applications
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5897:
URL: https://github.com/apache/incubator-nuttx/pull/5897#discussion_r836839526
##########
File path: arch/xtensa/src/lx6/Toolchain.defs
##########
@@ -55,8 +59,14 @@ else
MAXOPTIMIZATION += -fomit-frame-pointer
endif
-ifeq ($(CONFIG_MM_KASAN),y)
- ARCHCPUFLAGS += -fsanitize=kernel-address
+CXXFLAGS =
+
+ifneq ($(CONFIG_CXX_EXCEPTION),y)
+ CXXFLAGS += -fno-exceptions -fcheck-new
Review comment:
Done, PTAL again.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org