Another option is to disable the warning locally […] But this is compiler-specific and noisy. It also does not explain the real issue to anyone reading the code.
I don’t understand this logic. Surely disabling the warning is directly explaining the actual issue, that the code emits a false positive warning for specific compilers and conditions? That the false_but_compiler_does_not_know_it_ thing is apparently the better way to convey what’s going on seems kind of ridiculous.
My thought was that they may have to disable it differently for each compiler which might not be pretty, but yeah seems much easier to just do that as needed than to bend over backwards to look clever AND introduce unnecessary runtime effects at the same time.
My guess would be they didn’t like how verbose it was to supress the warning, or maybe they have a coding standard not to suppress warnings, and this was a workaround for that 😅
I don’t understand this logic. Surely disabling the warning is directly explaining the actual issue, that the code emits a false positive warning for specific compilers and conditions? That the false_but_compiler_does_not_know_it_ thing is apparently the better way to convey what’s going on seems kind of ridiculous.
You could say that the current way is so unclear that the guy had to write a 2 thousand word essay to explain it.
My thought was that they may have to disable it differently for each compiler which might not be pretty, but yeah seems much easier to just do that as needed than to bend over backwards to look clever AND introduce unnecessary runtime effects at the same time.
My guess would be they didn’t like how verbose it was to supress the warning, or maybe they have a coding standard not to suppress warnings, and this was a workaround for that 😅
actually not sure, as they do use
#pragma GCC diagnostic ignoredin a couple of places :https://github.com/git/git/blob/8d96f09e9245ddf80c1981476fcbac8c4bb4125f/compat/win32/headless.c#L14
https://github.com/git/git/blob/8d96f09e9245ddf80c1981476fcbac8c4bb4125f/compat/regex/regex.c#L20
but apparently that disables it for everything after that line in the file though:
https://stackoverflow.com/questions/48426484/concise-way-to-disable-specific-warning-instances-in-clang#comment83878587_48426846
I think the second answer on that stackoverflow post might be better than
false_but_the_compiler_does_not_know_it_though…