[PATCH] D96274: [clang][cli] Generate and round-trip Diagnostic options

Jan Svoboda via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Feb 9 06:06:43 PST 2021


jansvoboda11 added a comment.

Got rid of `{Warnings,Remarks}AsWritten` entirely.



================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:745
+
+    A->render(Args, Rendered);
   }
----------------
dexonsmith wrote:
> It's not obvious why this renders the args instead of calling `A->getAsString()` to get the literal command-line argument and putting it directly in DiagnosticsAsWritten. If that'll work, I suggest switching to that, since it's a bit more straightforward; if there's some reason you can't do that, please leave a comment explaining why.
I removed `DiagnosticsAsWritten` completely.

Previously, the code looped over `A->getValues()`, which gave me the impression it handles a flag that might indeed have multiple values (e.g. `-Wsomething=a,b,c`). If the code only stored `{a,b,c}` into `Diagnostics`, `GenerateDiagnosticArgs` could never reconstruct the original argument.

However, after looking closely at all `-W` and `-R` options, this never happens and `A->getValues()` always has exactly one element. I've refactored the code and clarified the comment for the branch above.


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2230-2240
+  for (const auto &Warning : Opts.WarningsAsWritten) {
+    // This option also maps to UndefPrefixes that generates it automatically.
+    if (StringRef(Warning).startswith("-Wundef-prefix="))
+      continue;
+    // TODO: Consider ignoring '-Wno-rewrite-macros' that maps to
+    // NoRewriteMacros.
+    Args.push_back(SA(Warning));
----------------
dexonsmith wrote:
> Does this mean that if some client is programmatically modifying the `Warnings` array (which is what `CompilerInstance` reads) they need to also update `WarningsAsWritten` array identically to get command-line generation to work?
> 
> If so, then this is a bit non-obvious / error-prone. Maybe these can encapsulated together:
> ```
> struct DiagnosticArg {
>   std::string Name;
>   std::string AsWritten;
> };
> std::vector<DiagnosticArg> Warnings;
> std::vector<DiagnosticArg> Remarks;
> ```
I double-checked and managed to get rid of this entirely, see the other comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96274



More information about the cfe-commits mailing list