-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Maybe add -Wformat-type-confusion to the build as much as we can #7935
Comments
There is an idiom for relaxing warnings for third party in the GN: configs -= [ "//buildsys/1st_party_warnings" ] configs += [ "//buildsys/3rd_party_warnings" ] The trouble is that "//buildsys/1st_party_warnings" is an object from the core build system that we have to name which introduces some coupling (e.g., projects may have the same pattern with different target names and this would not work). To minimize this coupling, indirect this through the build_overrides directory where the top level project can provide suitable targets (or not, if the defaults already work). This is provided for discussion in issue project-chip#7935
There is an idiom for relaxing warnings for third party in the GN: configs -= [ "//buildsys/1st_party_warnings" ] configs += [ "//buildsys/3rd_party_warnings" ] The trouble is that "//buildsys/1st_party_warnings" is an object from the core build system that we have to name which introduces some coupling (e.g., projects may have the same pattern with different target names and this would not work). To minimize this coupling, indirect this through the build_overrides directory where the top level project can provide suitable targets (or not, if the defaults already work). This is provided for discussion in issue project-chip#7935
We actually already are disabling warnings in lwip
We can split the warnings_default config into warnings_default and warnings_third_party if we want but it's a little bit tricky to do it without introducing coupling with the name & organization of the core build rules (which I'm really trying to minimize, so that you can drop in CHIP into a project with different versions of them which is something that we are doing). One way we could do it is #7959 |
There is an idiom for relaxing warnings for third party in GN: configs -= [ "//buildsys/1st_party_warnings" ] configs += [ "//buildsys/3rd_party_warnings" ] The trouble is that "//buildsys/1st_party_warnings" is an object from the core build system that we have to name which introduces some coupling (e.g., projects may have the same pattern with different target names and this would not work). To minimize this coupling, indirect this through the build_overrides directory where the top level project can provide suitable targets (or not, if the defaults already work). This is provided for discussion in issue project-chip#7935
…rrors. This is supported by clang, not gcc, hence adding this only to the clang cflags in the strict warnings config. lwip triggers this warning a lot, so we disable it there. Fixes project-chip#7935
Perfect, thank you! |
There is an idiom for relaxing warnings for third party in GN: configs -= [ "//buildsys/1st_party_warnings" ] configs += [ "//buildsys/3rd_party_warnings" ] The trouble is that "//buildsys/1st_party_warnings" is an object from the core build system that we have to name which introduces some coupling (e.g., projects may have the same pattern with different target names and this would not work). To minimize this coupling, indirect this through the build_overrides directory where the top level project can provide suitable targets (or not, if the defaults already work). This is provided for discussion in issue project-chip#7935
There is an idiom for relaxing warnings for third party in GN: configs -= [ "//buildsys/1st_party_warnings" ] configs += [ "//buildsys/3rd_party_warnings" ] The trouble is that "//buildsys/1st_party_warnings" is an object from the core build system that we have to name which introduces some coupling (e.g., projects may have the same pattern with different target names and this would not work). To minimize this coupling, indirect this through the build_overrides directory where the top level project can provide suitable targets (or not, if the defaults already work). This is provided for discussion in issue #7935
…rrors. (project-chip#7993) This is supported by clang, not gcc, hence adding this only to the clang cflags in the strict warnings config. lwip triggers this warning a lot, so we disable it there. Fixes project-chip#7935
There is an idiom for relaxing warnings for third party in GN: configs -= [ "//buildsys/1st_party_warnings" ] configs += [ "//buildsys/3rd_party_warnings" ] The trouble is that "//buildsys/1st_party_warnings" is an object from the core build system that we have to name which introduces some coupling (e.g., projects may have the same pattern with different target names and this would not work). To minimize this coupling, indirect this through the build_overrides directory where the top level project can provide suitable targets (or not, if the defaults already work). This is provided for discussion in issue project-chip#7935
Problem
We are not getting all possible format confusion warnings right now.
Proposed Solution
See #7921 (comment) -- add
-Wformat-type-confusion
to catch more.I tried adding it in
build/config/compiler/BUILD.gn
but then I get errors in third-party code like so:@mspang is there somewhere we can add this that will get used for our code but not third-party code?
The text was updated successfully, but these errors were encountered: