[SERVER-51178] Add ability to disable specific compiler warnings-as-errors from SCons Created: 28/Sep/20  Updated: 27/Oct/23  Resolved: 27/Oct/23

Status: Closed
Project: Core Server
Component/s: Build
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Major - P3
Reporter: Ryan Egesdahl (Inactive) Assignee: [DO NOT ASSIGN] Backlog - Server Development Platform Team (SDP) (Inactive)
Resolution: Won't Do Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Duplicate
is duplicated by SERVER-9568 Add new vars C and C++ warning flags ... Closed
Issue split
split from SERVER-51084 MongoDB fails to compile on Mac with ... Closed
Assigned Teams:
Server Development Platform
Participants:

 Description   

We currently have only two ways to disable compiler warnings-as-errors: either make none of them errors using --disable-warnings-as-errors or disable all errors with a compiler flag. There is no way to disable specific warnings-as-errors from the SCons command line due to the order of the compiler flags. This matters sometimes when we have a new issue pop up (such as in a new compiler version) where we want to see the warning but not fail the build.



 Comments   
Comment by Andrew Morrow (Inactive) [ 06/Jan/21 ]

ryan.egesdahl - Generally, I don't like providing --option-to-do-something-to-flags when the same effect can be achieved by simply adding FLAG=value to the command line. Build system users in general (i.e. not just us) expect to be able to provide these sorts of flags via the usual command line mechanisms (CFLAGS, etc.). To the extent possible, we should be encouraging people to use them. The existence of --disable-warnings-as-errors is a consequence of the ordering bug around how we inject -Wall and -Werror. If we fixed that bug, we wouldn't actually need --disable-warnings-as-errors at all, because saying CCFLAGS=-Wno-error or the Windows analogue would achieve the same effect. My suggestion is that we should in fact fix that bug, now, rather than trying to introduce a smart facility that "knows" even more about certain compilers and their options.

Comment by Ryan Egesdahl (Inactive) [ 05/Jan/21 ]

acm - For your second point, the -Wall flag should only be in CCFLAGS (which is where we have it now) because CCFLAGS automatically applies to all stages of the compiler, which is what we want error and warning flags to do.

To your first point, I don't think we actually need to work so hard to make this happen. I would prefer not to be teaching SCons about any of these flags. I originally imagined this as simply copying the flag name from the appropriate part of the error code into the list of suppressed errors and then constructing the correct arguments accordingly before inserting them into the right place in the compiler command. Clang, GCC, and MSVC should all have the property that the error message tells you what flag to use, and the flags are all fairly regular in their construction. If you get the argument(s) wrong, the compiler is going to be plenty verbose about what you did. I don't think SCons needs to be in the way here, and it's better to be simple where we can be.

To your third point - we can simply have another argument with a similar structure that issues a -Wno-XXX (or similar for MSVC) if we really want it. But I am not so sure we should be controlling such things via the compiler command like that. Suppressing an error is one thing, but telling the compiler we're smarter than it is strikes me as the wrong thing to do. I would much rather wait until we actually need that functionality for some reason before we add it. I don't expect it would be complicated if we get error suppression right.

Comment by Andrew Morrow (Inactive) [ 05/Jan/21 ]

Some background on this that I'm transferring from SERVER-51084:

I wrote:

Ryan Egesdahl - The warning flag ordering issue is a longstanding nuisance. We had a (not very detailed) ticket for that work, once: SERVER-9568. Perhaps we should re-open it, because I agree that it is surprising that adding warnings suppressions to CXXFLAGS should work. Part of the issue, IIRC, is that CCFLAGS appears after CFLAGS or CXXFLAGS on the C[XX]COM variable expansion, meaning that if we add -Werror to CCFLAGS, it will always appear later on the compile line than the warning suppression you are trying to add with via CFLAGS or CXXFLAGS. And, if won't work to add your warning suppression flag to CCFLAGS on the command line either, because we AppendUnique -Werror. However, if we instead added -Werror to both CFLAGS and CXXFLAGS via Prepend, maybe it all works?

And ryan.egesdahl replied:

Andrew Morrow I think what we settled on was wanting to be able to add specific no-error flags to the build like this:

buildscripts/scons.py --ssl --variables-files=etc/scons/xcode_macosx.vars --libc++ --detect-odr-violations --dbg=on --opt=off --ninja --build-tools=next --disable-warnings-as-errors=range-loop-analysis VARIANT_DIR=ninja generate-ninja
The idea was that we could exclude specific warnings from being marked as errors until we could fix them but not affect anything else that might pop up. The ordering of -Wall in the arguments wasn't as much of a problem as being able to turn off specific warnings-as-errors, which weren't being added in the right place by CFLAGS/CXXFLAGS.

I'll admit I find the proposed solution (--disable-warnings-as-errors=range-loop-analysis) appealing, but there are a few things that give me pause:

  • We will need to teach SCons about how to map each of these arguments into the appropriate warning suppression for Windows and gcc and clang. That isn't necessarily hard, but it is work.
  • There is an inherent ambiguity. On systems other than MSVC which distinguish the drivers for C and C++, given --disable-warnings-as-errors=x, should we append -Wno-error=x to CCFLAGS, CFLAGS, or CXXFLAGS? Perhaps all of them? What if a finicky compiler rejects a C++ specific Wno-error on C files, or vice versa, or starts to in the future?
  • There are cases where you don't just want to demote the warning back from error to warning, but to suppress the warning entirely, because it is interfering with, say icecream, or produces a hideous diagnostics. This flag can't handle that, so you still end up needing to manually inject warning control flags via one of CCFLAGS, CFLAGS, or CXXFLAGS. I'd prefer the mechanism to achieve both disabling a warning entirely and demoting it from an error to a warning be the same.

In general, I have a preference for treating flags to the control the compiler as passthroughs from the user to the extent that we can. Users of the build system need to be aware of how to use the command line ability to control C and C++ flags to achieve their aims. So, having thought about it, my preference would be that we repair the issue with where -Wall and -Werror appear on the command line so that overrides provided on the command line work as expected.

Generated at Thu Feb 08 05:24:44 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.