[PATCH] D67420: [analyzer] NFC: Separate PathDiagnosticConsumer options from AnalyzerOptions.

Kristóf Umann via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Sep 11 07:27:04 PDT 2019


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





More information about the cfe-commits mailing list