[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