trns1997 commented on PR #17968: URL: https://github.com/apache/nuttx/pull/17968#issuecomment-3767219257
> > > > That said, I do have one open question for CXX users; When building with the external C++ toolchain, is it expected that NuttX’s `include/cxx` headers are always used? > > > > My understanding is that these headers are required to maintain NuttX C++ compatibility (wrapping or redefining parts of the standard library to coexist with NuttX’s libc), so including them even with a full C++ toolchain seems intentional and correct. But if the long-term expectation is that a “pure” toolchain C++ build should not rely on `include/cxx`, then the Makefile behavior itself might need revisiting. > > > > What do you think? I would also like @leducp @xiaoxiang781216 input on this :) > > > > > > > > > LIBCXXMINI should be changed to LIBMINIABI, since include/cxx provide the implementation similar to libcxxabi, not libcxx. Which mean we should skip include/cxx if user select other c++ abi implementation. > > > > > > @xiaoxiang781216 I’m still not entirely sure I understand what you’re suggesting, so I’d like to restate my understanding and check if it’s correct. > > I’m not using `LIBCXXMINI`, which is why that part confused me a bit. > > Currently, the Makefile behavior in `tools/Config.mk` is: > > > > * `CONFIG_LIBCXX` → include `include/libcxx` > > * `CONFIG_UCLIBCXX` → include `include/uClibc++` > > * otherwise → include `include/cxx` > > this search path should be guarded by LIBCXXMINI or LIBMINIABI. > > > When I select `CONFIG_LIBCXXTOOLCHAIN`, I fall into the “otherwise” case, so `include/cxx` is included in the Makefile build. This works there. > > In the CMake build, this include logic seems to be missing, which is why I proposed adding it to mirror the Makefile behavior. > > My question is whether this fallback to `include/cxx` is correct when building using `CONFIG_LIBCXXTOOLCHAIN` , or if it’s something that only works by accident today. If `include/cxx` should not be used with `CONFIG_LIBCXXTOOLCHAIN`, then it seems the Makefile logic itself would need revisiting, not just CMake. > > Since cxxxx header files should be provided by c++ standard library, we should add include/cxx to search path only when we enable LIBCXXMINI or LIBMINIABI. > > > Happy to adjust the fix either way — just want to make sure I’m doing the right thing. I understand the intent as: `include/cxx` should only be added when the mini C++ ABI is enabled (i.e. `LIBCXXMINI` / `LIBMINIABI`), and not by default when using `CONFIG_LIBCXXTOOLCHAIN`. In that case, the current Makefile behavior is indeed too permissive and should be tightened with proper guards. This does sound like a slightly broader change than just fixing the CMake build. For next steps, would you prefer that I: - First update the Makefile logic (add the `LIBCXXMINI` / `LIBMINIABI` guards) and then adapt CMake accordingly in this PR, or - Keep this PR minimal by making CMake mirror the current Makefile behavior, and address the config/Makefile cleanup in a separate follow-up PR? I was thinking of taking up the second option, that way we unblock users and then i will go ahead and address the fundamental guard issue. Let me know if that works for you guys? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
