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

Include directories should be added as system include directories #924

Closed
ooxi opened this issue Jul 17, 2022 · 9 comments
Closed

Include directories should be added as system include directories #924

ooxi opened this issue Jul 17, 2022 · 9 comments
Assignees
Labels
Milestone

Comments

@ooxi
Copy link

ooxi commented Jul 17, 2022

Currently all include directories are added as none system include directories. This causes warnings and errors when compiling my own source code

In file included from /pico-sdk/src/common/pico_stdlib/include/pico/stdlib.h:13,
                 from /blink.c++:2:
/pico-sdk/src/rp2_common/hardware_gpio/include/hardware/gpio.h:882:33: error: ISO C++ does not permit named variadic macros [-Werror=variadic-macros]
  882 | #define CU_REGISTER_DEBUG_PINS(p...) \
      |                                 ^~~

@kilograham
Copy link
Contributor

This error seems unrelated to whether things are system includes or not. What compiler version are you using?

@ooxi
Copy link
Author

ooxi commented Jul 20, 2022

@kilograham I'm using Fedora 36, this is the compiler identification according to CMake

-- The C compiler identification is GNU 11.1.0
-- The CXX compiler identification is GNU 11.1.0

I have created a minimal example on GitHub where you can see the full compiler output https://github.com/ooxi/pico-sdk-pedantic/runs/7435146674?check_suite_focus=true

These are the compile settings for my source file which includes pico/stdlib.h

	-std=c++20

	-Wall
	-Wextra
	-Werror
	-Wpedantic

	-Wnon-virtual-dtor
	-Wcast-align
	-Wunused
	-Woverloaded-virtual
	-Wdouble-promotion
	-Wformat=2

@kilograham
Copy link
Contributor

kilograham commented Jul 20, 2022

The SDK will not compile with both -Wpedantic and -Werror ; the variadic macro thing is something that could be fixed easily enough, but it is hardly a warning of a possible bug

@ooxi
Copy link
Author

ooxi commented Jul 20, 2022

@kilograham first of all, thank you very much for your fast reaction! Very appreciated :-)

The SDK will not compile with both -Wpedantic and -Werror

I do not want to compile the SDK with -Wpedantic and -Werror, just my own sources. Therefore I used set_source_files_properties to set those compile options only on my own sources, not all sources of the library.

Using target_include_directories with SYSTEM INTERFACE instead of INTERFACE will let the compiler know to not apply those settings on the include.

I will come back with a proof of concept :-)

@ooxi
Copy link
Author

ooxi commented Jul 20, 2022

@kilograham I have created a proof of concept which is sufficient to compile the example with -Wpedantic and -Werror

I would be happy to clean up the patch and prepare a PR if you consider accepting it.

@kilograham
Copy link
Contributor

Ah, ok i see the point... warnings are suppressed in system headers.

@kilograham kilograham added this to the 1.5.0 milestone Jul 20, 2022
@ooxi
Copy link
Author

ooxi commented Jul 21, 2022

@kilograham feel free to assign the issue to me, I will prepare a PR against develop

@kilograham
Copy link
Contributor

duplicate of #82

@kilograham kilograham reopened this Dec 6, 2022
@kilograham
Copy link
Contributor

oh this has a PR ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants