dexonsmith added inline comments.
================ Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2347-2353 + // This warning was manufactured, don't put it on the command line. + if (Warning == "no-stdlibcxx-not-found" && T.isOSDarwin() && + DashX.isPreprocessed()) + continue; + // This warning was manufactured, don't put it on the command line. + if (Warning == "spir-compat" && T.isSPIR()) + continue; ---------------- It seems reasonable to skip generating them if they're implied by other command-line options, but I'm not sure "manufactured" is the right word to use as a distinguishing characteristic. The entire CompilerInvocation could have been created programmatically. I suggest instead saying the warning flag is implied by the other command-line options. Also, note that when created programmatically, one could have pushed `stdlibcxx-not-found` *after* pushing `no-stdlibcxx-not-found`. Since that's impossible to recreate, maybe there should be an assertion to catch this? Alternatively, should this kind of imply-diagnostic-options logic be moved to the driver? Relatedly, unlike most command-line options, GenerateDiagnosticArgs is not canonicalizing the options. For example, if `-Wabc` implies `-Wdef`, it'd be nice to generate just `-Wabc` from initial command-lines of either `-Wabc -Wdef` or `-Wdef -Wabc` / to drop the first of `-Wno-abc -Wabc` / etc. IMO, something akin to an initial DiagnosticsEngine::DiagState (likely renamed) could be stored in DiagnosticOptions (effectively, the resulting state from calling ProcessWarningOptions). Parsing could translate command-line options to this initial state. The state could be modified programmatically; it'd also be used to initialize DiagnosticsEngine. Generating command-line options would emit a canonical set of options that would recreate the state. But that's a pretty big refactoring, and I think it's okay to make progress without that. As an initial fix, this is probably fine, but I think the comments and/or FIXMEs should acknowledge that it's a bit fragile and point in a more sound / less fragile direction (doesn't have to be my suggestion). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96848/new/ https://reviews.llvm.org/D96848 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
