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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
