[PATCH] D150226: [Clang] Remove ability to downgrade warning on the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

David Blaikie via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 15 13:01:38 PDT 2023


dblaikie added a comment.

> My concern with `ShowInSystemHeaders` is that this seems like a bad user experience.

I guess you're comparing this user experience to the one when this feature was a normal warning/without `ShowInSystemHeaders` - I'm comparing this situation to the future where this becomes a hard error with no escape hatch. `ShowInSystemHeaders` is more aggressive than the former & yeah, isn't terribly nice for users - but it's less aggressive than the latter, and gives users some escape hatch for now until the thing becomes a hard error & at least they know it's coming, maybe? (with enough text in the warning, etc)

So, yeah, I'd be inclined to make the change to `ShowInSystemHeaders` close to when you were going to make it a hard error, pushing back the hard-error change a little bit (maybe just one clang release? Maybe a couple? Not sure).

> If the system header triggers an error 1) some users aren't going to know how to fix that by downgrading the diagnostic, so that may cause them to go "Clang is buggy because <other compiler> accepts this fine" (not the end of the world, but frustrates both us and users). 2) the only recourse users have is to downgrade/disable the diagnostic (otherwise they'd have to change system header code), which they may likely do with a command line flag rather than something more targeted like diagnostic pragmas around the include, which increases the risk of users not seeing the issues in code they can control.

They'd already have had a chance to deal with their code when this was a warning-default-error without "ShowInSystemHeaders"? (or, if the yhaven't picked up a new compiler often enough, and they go from "a warning we didn't care about" to "warning-default-error-with-ShowInSystemHeaders" - they're still better off than if it'd gone straight to hard error, some chance to cleanup while disabling the warning/error before picking up a compiler version that makes it a hard error)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150226/new/

https://reviews.llvm.org/D150226



More information about the cfe-commits mailing list