[PATCH] D97121: [clang-tidy] Add a Standalone diagnostics mode to clang-tidy

Stephen Kelly via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Mar 14 07:25:23 PDT 2021


steveire added inline comments.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.


================
Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h:178
 
+  void setSelfContainedDiags(bool Value = true) { SelfContainedDiags = Value; }
+
----------------
I don't think this `Value` should be `= true`. It makes call sites confusing.

There is precedent though according to 

```
git grep "void set.*bool.*=.*)"
```

so consider this a nit.


================
Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h:217
   bool AllowEnablingAnalyzerAlphaCheckers;
+  bool SelfContainedDiags;
 };
----------------
The order of members here is starting to look suspicious, size-wise. Not something for this change though.


================
Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp:805
                                      LoopFixerKind FixerKind) {
-  // If we already modified the range of this for loop, don't do any further
+  // In single fix mode we don't want dependancies on other loops, otherwise, If
+  // we already modified the range of this for loop, don't do any further
----------------
Can you update this comment to not refer to "single fix" mode?


================
Comment at: clang-tools-extra/clang-tidy/utils/IncludeInserter.h:63
+  explicit IncludeInserter(IncludeSorter::IncludeStyle Style,
+                           bool SingleFixMode);
 
----------------
Should the "single fix" naming here also be changed?


================
Comment at: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp:520
 
+TEST(DiagnosticTest, ClangTidySingleFixMode) {
+  Annotations Main(R"cpp($MathHeader[[]]
----------------
More "single fix" naming here.


================
Comment at: clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp:34
+                               utils::IncludeSorter::IS_Google,
+                           bool SingleFixMode = false)
+      : ClangTidyCheck(CheckName, Context), Inserter(Style, SingleFixMode) {}
----------------
"Single fix" naming here too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97121



More information about the cfe-commits mailing list