Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop using gcc's "-include" flag #7216

Closed
SebastianBoe opened this issue Apr 26, 2018 · 4 comments
Closed

Stop using gcc's "-include" flag #7216

SebastianBoe opened this issue Apr 26, 2018 · 4 comments
Assignees
Labels
area: Build System area: Configuration System Enhancement Changes/Updates/Additions to existing features Wont Fix Not going to fix or implement

Comments

@SebastianBoe
Copy link
Collaborator

SebastianBoe commented Apr 26, 2018

Both the linux kernel and Zephyr uses gcc's "-include" feature to insert Kconfig defines into the top of every source file that is built.

The main advantage of using "-include" is that it is impossible for a C source file to accidentally forget to include the Kconfig header file[0].

But protecting the user from this user-error has several disadvantages.

IDE support

IDE's and other C tooling that rely on best-effort parsing of C source code will struggle to resolve where the CONFIG_ defines come from because there is no #include that points to where the CONFIG_ defines come from.

E.g. opening a hello world Zephyr application with Eclipse presents the user with a compilation error because CONFIG_ARCH is not being resolved.

Even CLion, which is supposed to have CMake-integration had for many years an open bug about "-include" not being parsed correctly.

Human readability

Not only tools, but also humans are more confused by "-include" than "#include". If a firmware developer
wants to get familiar with Zephyr and find out where the CONFIG_ macros come from he will be able to follow "#include's" without assistance or reading documentation. But self-discovering that the build system is using the niche "-include" flag is not as easy.

It is not clear-cut, but I believe that out-of-the-box IDE support and a simplified build system will outweigh the confusion from accidentally omitting #include "conf.h". The best resolution to this issue however would be some mechanism that could verify that all C sources include directly or indirectly autoconf.h, then we would have all of the advantages, but none of the disadvantages of -include.

EDIT: After thinking more about this, testing for this would be very difficult, because not only would you need to prove that #include "conf.h" occurs, but also that it occurs before any CONFIG_ define is used ...

[0] TODO: Is there any way to automatically enforce/test that all C source files either directly or indirectly #include autoconf.h ?

@SebastianBoe
Copy link
Collaborator Author

Mynewt has an interesting solution to this problem.

They disallow directly accessing CONFIG_ARM, and instead require that one accesses the options through a macro with an argument, CONFIG(ARM), since macro's with arguments can't be undefined, you get an error, and can therefore be safe that autoconf.h has been defined.

This would require search replacement of all Kconfig's in and out of tree, but might be worth it in the long term ...

@marc-hb
Copy link
Collaborator

marc-hb commented Jun 19, 2020

In case anyone is reading this issue in 2020 (couldn't resist a YouTube comment), -include ${AUTOCONF_H} is still here except it was changed in late 2017 to -imacros ${AUTOCONF_H} in giant new feature v1.11.0-rc1~1162-g 76f7644 / #4174. I suspect this was done because the same commit also added -include arch/posix/include/posix_cheats.h and wanted ${AUTOCONF_H} to win the "race to the top". @aescolar could you confirm something that far back?

Fast forwarding a few seasons (tig blame is so much more fun than TV), in commits v2.0.0-rc1~1380-g f57ba2d / #16645 and v2.0.0-rc1~1072-g e53c0d0 / #17267 make compilation with xcc work again @npitre won a short fight with (multiple) -imacros and CMake that I find worth mentioning:

  # We cannot use the "-imacros foo" form here as CMake insists on
  # deduplicating arguments, meaning that subsequent usages after the
  # first one will see the "-imacros " part removed.
  #
  # gcc and clang support the "--imacros=foo" form but not xcc.
  # Let's use the "combined" form (without space) which is supported
  # by everyone so far.

Kudos to @jajanusz for remembering about this issue, always a pleasure to watch some old @SebastianBoe :-)

PS: I've been told @SebastianBoe is "off the grid" right now and his github status seems to confirm that.

@tejlmand
Copy link
Collaborator

tejlmand commented Oct 1, 2020

@npitre won a short fight with (multiple) -imacros and CMake that I find worth mentioning:

  # We cannot use the "-imacros foo" form here as CMake insists on
  # deduplicating arguments, meaning that subsequent usages after the
  # first one will see the "-imacros " part removed.
  #
  # gcc and clang support the "--imacros=foo" form but not xcc.
  # Let's use the "combined" form (without space) which is supported
  # by everyone so far.

Which has now been resolved and has been introduced with the new Toolchain abstration:
#24851 (comment)

Well, that was true for older CMake versions, but since CMake v3.12, you can avoid the deduplication:

While beneficial for individual options, the de-duplication step can break up option groups. For example, -D A -D B becomes -D A B. One may specify a group of options using shell-like quoting along with a SHELL: prefix. The SHELL: prefix is dropped and the rest of the option string is parsed using the separate_arguments() UNIX_COMMAND mode. For example, "SHELL:-D A" "SHELL:-D B" becomes -D A -D B.

https://cmake.org/cmake/help/v3.12/command/target_compile_options.html

@nashif
Copy link
Member

nashif commented May 28, 2021

AFAIK this was addressed, so closing

@nashif nashif closed this as completed May 28, 2021
@marc-hb marc-hb added the Wont Fix Not going to fix or implement label May 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Build System area: Configuration System Enhancement Changes/Updates/Additions to existing features Wont Fix Not going to fix or implement
Projects
None yet
Development

No branches or pull requests

4 participants