Szelethus added a comment.

> @Szelethus: I could have stored `PathDiagnosticConsumerOptions` in 
> `AnalyzerOptions` by value and pass a const reference around, but it wasn't 
> pleasant to integrate with `AnalyzerOptions.def`. I.e., i'd have to implement 
> a new kind of option that doesn't allocate its own field but instead re-uses 
> a field within a sub-object. Do you want me to go for it or is this 
> implementation good enough?

I don't really like this direction :^) Imagine how tedious and non-obvious 
it'll be to add a new pathdiagnostic option. I don't like we're copying this 
around. There is also a discussion to be had about whether having to specify 
`-analyzer-config` to a no longer analyzer specific feature is any good.

> or do you have other approaches in mind?

I do! The level idea is to completely separate pathdiagnostic configs from the 
`AnalyzerOptions`, I believe this is more in line with your goal.

- Let's create the equivalent of `-analyzer-config`, `-pathdiagnostic-config`, 
and things like `-pathdiagnostic-config-help`.
- Avoid getting a parsing error for specifying `-analyzer-config 
macro-expansion` instead of  `-pathdiagnostic-config macro-expansion`.
  - Create a field similar to `AnalyzerOptions::AnalyzerConfigCmdFlags` called 
`AnalyzerOptions::AnalyzerConfigAliases` with the 4 configs that we're moving 
over. In compatibility mode, `AnalyzerOptions::isUnknownAnalyzerConfig()` shall 
search in this field as well. In non-compatibility mode, this will result in an 
error.
- Make `PathDiagnosticConsumerOptions` field of `CompilerInvocation`, similar 
to `AnalyzerOptions`.
- Your idea of implementing a `DIAGNOSTIC_OPTION` macro sounds nice.
  - Delete these entries from `AnalyzerOptions.def`.
  - Create `clang/include/Analysis/PathDiagnostics.def`, list them there.
  - Generate fields to `PathDiagnosticConsumerOptions` similar to how 
`AnalyzerOptions` does it.
- Parse the options `CompilerInvocation.cpp`
  - Most of the analyzer config parsing functions can be reused!
  - For these select 4 options that suffer compatibility issues, manually check 
`AnalyzerOptions::ConfigTable`.

WDYT? I am happy to help out, I realize this is plenty of code to write, if we 
go for it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67420



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to