[PATCH] D75340: [clang-tidy] Change checks to use new isLanguageVersionSupported restriction

Dmitri Gribenko via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 28 05:00:05 PST 2020


gribozavr2 added a comment.

Looks good, but please move non-mechanical changes (potentially-semantic changes) into a separate patch.



================
Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewAlignmentCheck.h:30
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus && !LangOpts.CPlusPlus17;
+  }
----------------
I think CPlusPlus17 implies CPlusPlus.


================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.h:28
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus;
----------------
> (from a parent review) Not a critical thing, but I feel like it would be better to put isLanguageVersionSupported before register functions. That logical order makes more sense to a reader that is not familiar with the class.

If you end up making that change, it would be great if you could also apply it throughout this patch.


================
Comment at: clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.h:36
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus11;
+  }
----------------
Shouldn't this be a non-11 check?


================
Comment at: clang-tools-extra/clang-tidy/modernize/DeprecatedIosBaseAliasesCheck.h:30
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus11;
+  }
----------------
Also should be non-11 to preserve existing behavior?


================
Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.h:26
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus11;
+  }
----------------
Ditto, non-11.

It seems however that this check indeed wants C++11. If you would like to make that change, please make pull it out into a separate patch.


================
Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.h:52
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus11;
+  }
----------------
Ditto, please make semantic changes in a separate patch.


================
Comment at: clang-tools-extra/clang-tidy/modernize/UseAutoCheck.h:25
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus11;
+  }
----------------
Ditto.


================
Comment at: clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.h:44
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus11;
+  }
----------------
Ditto.


================
Comment at: clang-tools-extra/clang-tidy/modernize/UseEqualsDeleteCheck.h:46
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus11;
+  }
----------------
Ditto.


================
Comment at: clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.h:34
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus11;
+  }
----------------
Ditto.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75340





More information about the cfe-commits mailing list