llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: Baranov Victor (vbvictor) <details> <summary>Changes</summary> Fixes https://github.com/llvm/llvm-project/issues/152024. --- Full diff: https://github.com/llvm/llvm-project/pull/190530.diff 6 Files Affected: - (modified) clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp (+26-1) - (modified) clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h (+3-1) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5) - (modified) clang-tools-extra/docs/clang-tidy/checks/readability/redundant-member-init.rst (+5) - (added) clang-tools-extra/test/clang-tidy/checkers/readability/redundant-member-init-ignore-macros.cpp (+79) - (modified) clang-tools-extra/test/clang-tidy/checkers/readability/redundant-member-init.cpp (+21) ``````````diff diff --git a/clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp index f891c954b19ea..fd94e5bac30c9 100644 --- a/clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp @@ -33,9 +33,30 @@ getFullInitRangeInclWhitespaces(SourceRange Range, const SourceManager &SM, {PrevToken->getLocation(), Range.getEnd()}, SM, LangOpts); } +namespace { +// Matches a ``CXXConstructExpr`` whose written argument list (i.e. the +// source text between the parentheses or braces) involves a macro. +AST_MATCHER(CXXConstructExpr, initListContainsMacro) { + const SourceRange InitRange = Node.getParenOrBraceRange(); + if (InitRange.isInvalid()) + return false; + if (InitRange.getBegin().isMacroID() || InitRange.getEnd().isMacroID()) + return true; + const ASTContext &Context = Finder->getASTContext(); + const std::optional<Token> NextTok = + utils::lexer::findNextTokenSkippingComments( + InitRange.getBegin(), Context.getSourceManager(), + Context.getLangOpts()); + if (!NextTok) + return true; + return NextTok->getLocation() != InitRange.getEnd(); +} +} // namespace + void RedundantMemberInitCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "IgnoreBaseInCopyConstructors", IgnoreBaseInCopyConstructors); + Options.store(Opts, "IgnoreMacros", IgnoreMacros); } void RedundantMemberInitCheck::registerMatchers(MatchFinder *Finder) { @@ -44,7 +65,11 @@ void RedundantMemberInitCheck::registerMatchers(MatchFinder *Finder) { argumentCountIs(0), hasDeclaration(cxxConstructorDecl( ofClass(cxxRecordDecl(unless(isTriviallyDefaultConstructible())) - .bind("class"))))) + .bind("class")))), + IgnoreMacros + ? unless(initListContainsMacro()) + : static_cast<ast_matchers::internal::Matcher<CXXConstructExpr>>( + anything())) .bind("construct"); auto HasUnionAsParent = hasParent(recordDecl(isUnion())); diff --git a/clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h b/clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h index ff8b02d141a46..ea8f61d60bbed 100644 --- a/clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h +++ b/clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h @@ -23,7 +23,8 @@ class RedundantMemberInitCheck : public ClangTidyCheck { RedundantMemberInitCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), IgnoreBaseInCopyConstructors( - Options.get("IgnoreBaseInCopyConstructors", false)) {} + Options.get("IgnoreBaseInCopyConstructors", false)), + IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", false)) {} bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { return LangOpts.CPlusPlus; } @@ -36,6 +37,7 @@ class RedundantMemberInitCheck : public ClangTidyCheck { private: bool IgnoreBaseInCopyConstructors; + bool IgnoreMacros; }; } // namespace clang::tidy::readability diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 69dc5b9633398..b6f487d2e4ff2 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -442,6 +442,11 @@ Changes in existing checks positives on parameters used in dependent expressions (e.g. inside generic lambdas). +- Improved :doc:`readability-redundant-member-init + <clang-tidy/checks/readability/redundant-member-init>` check by adding an + `IgnoreMacros` option to suppress warnings when the initializer involves + macros that may expand differently in other configurations. + - Improved :doc:`readability-redundant-preprocessor <clang-tidy/checks/readability/redundant-preprocessor>` check by fixing a false positive for nested ``#if`` directives using different builtin diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-member-init.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-member-init.rst index c26aaf73b16f8..aab2431db6aba 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-member-init.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-member-init.rst @@ -24,6 +24,11 @@ Example Options ------- +.. option:: IgnoreMacros + + When `true`, the check will ignore member initializations where the + initializer involves a macro expansion. Default is `false`. + .. option:: IgnoreBaseInCopyConstructors Default is `false`. diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-member-init-ignore-macros.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-member-init-ignore-macros.cpp new file mode 100644 index 0000000000000..f391b175cff0f --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-member-init-ignore-macros.cpp @@ -0,0 +1,79 @@ +// RUN: %check_clang_tidy %s readability-redundant-member-init %t \ +// RUN: -config="{CheckOptions: \ +// RUN: {readability-redundant-member-init.IgnoreMacros: true}}" + +struct S { + S() = default; + S(int i) : i(i) {} + S(const char *s) : s(s) {} + int i = 1; + const char *s = nullptr; +}; + +#define BRACES {} +#define EMPTY +#define NUMBER 1 +#define NAME + +struct WithMacro1 { + S s BRACES; +}; + +struct WithMacro2 { + S s {EMPTY}; +}; + +struct WithMacro3 { + S s {NUMBER}; +}; + +struct WithMacro4 { + WithMacro4() : s(EMPTY) {}; + + S s; +}; + +struct WithMacro5 { + S s{NAME}; +}; + +struct WithoutMacro { + S s {}; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: initializer for member 's' is redundant + // CHECK-FIXES: S s; +}; + +struct WithBlockCommentInInit { + S s {/* default-construct */}; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: initializer for member 's' is redundant + // CHECK-FIXES: S s; +}; + +struct WithCommentInCtorInit { + WithCommentInCtorInit() : s(/* default */) {}; + // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: initializer for member 's' is redundant + // CHECK-FIXES: WithCommentInCtorInit() {}; + + S s; +}; +struct WithCommentBeforeMacro { + S s {/* leading */ EMPTY}; +}; + +struct WithCommentAfterMacro { + S s {EMPTY /* trailing */}; +}; + +struct WithCommentAroundMacroCtor { + WithCommentAroundMacroCtor() : s(/* begin */ EMPTY /* end */) {}; + + S s; +}; + +struct WithoutMacroCtor { + WithoutMacroCtor() : s() {}; + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: initializer for member 's' is redundant + // CHECK-FIXES: WithoutMacroCtor() {}; + + S s; +}; diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-member-init.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-member-init.cpp index 0eebac82c9060..053bb15e25b89 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-member-init.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-member-init.cpp @@ -318,3 +318,24 @@ struct D9 { D9() : ss(S()) {} SS ss; }; + +#define EMPTY +#define BRACES {} + +struct WithMacro1 { + S s BRACES; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: initializer for member 's' is redundant +}; + +struct WithoutMacroCtor { + WithoutMacroCtor() : s(EMPTY) {}; + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: initializer for member 's' is redundant + // CHECK-FIXES: WithoutMacroCtor() {}; + S s; +}; + +struct WithBlockCommentInInit { + S s {/* default-construct */}; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: initializer for member 's' is redundant + // CHECK-FIXES: S s; +}; `````````` </details> https://github.com/llvm/llvm-project/pull/190530 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
