Index: clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp +++ clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp @@ -123,9 +123,8 @@ for (const Stmt *S : Body->body()) { if (S->getBeginLoc().isMacroID()) { - StringRef MacroName = - Lexer::getImmediateMacroName(S->getBeginLoc(), *Result.SourceManager, - getLangOpts()); + StringRef MacroName = Lexer::getImmediateMacroName( + S->getBeginLoc(), *Result.SourceManager, getLangOpts()); if (MacroName.contains_lower("assert")) return; } @@ -144,99 +143,125 @@ const FieldDecl *Field; const Expr *InitValue; std::tie(Field, InitValue) = isAssignmentToMemberOf(Class, S); - if (Field) { - if (IsUseDefaultMemberInitEnabled && getLangOpts().CPlusPlus11 && - Ctor->isDefaultConstructor() && - (getLangOpts().CPlusPlus20 || !Field->isBitField()) && - (!isa(Class->getDeclContext()) || - !cast(Class->getDeclContext())->isUnion()) && - shouldBeDefaultMemberInitializer(InitValue)) { - auto Diag = - diag(S->getBeginLoc(), "%0 should be initialized in an in-class" - " default member initializer") - << Field; - - SourceLocation FieldEnd = - Lexer::getLocForEndOfToken(Field->getSourceRange().getEnd(), 0, - *Result.SourceManager, getLangOpts()); - Diag << FixItHint::CreateInsertion(FieldEnd, - UseAssignment ? " = " : "{") - << FixItHint::CreateInsertionFromRange( - FieldEnd, - CharSourceRange(InitValue->getSourceRange(), true)) - << FixItHint::CreateInsertion(FieldEnd, UseAssignment ? "" : "}"); - - SourceLocation SemiColonEnd = - Lexer::findNextToken(S->getEndLoc(), *Result.SourceManager, - getLangOpts()) - ->getEndLoc(); - CharSourceRange StmtRange = - CharSourceRange::getCharRange(S->getBeginLoc(), SemiColonEnd); - - Diag << FixItHint::CreateRemoval(StmtRange); + if (!Field) + continue; + assert(InitValue && "An assigment to a field must also have an assigned" + " value"); + + SourceLocation BeginLoc = S->getBeginLoc(); + SourceLocation EndLoc = S->getEndLoc(); + SourceRange InitValueRange = InitValue->getSourceRange(); + + if (BeginLoc.isMacroID()) { + BeginLoc = Result.SourceManager->getSpellingLoc(BeginLoc); + EndLoc = Result.SourceManager->getSpellingLoc(EndLoc); + InitValueRange = SourceRange( + Result.SourceManager->getSpellingLoc(InitValueRange.getBegin()), + Result.SourceManager->getSpellingLoc(InitValueRange.getEnd())); + } + + if (IsUseDefaultMemberInitEnabled && getLangOpts().CPlusPlus11 && + Ctor->isDefaultConstructor() && + (getLangOpts().CPlusPlus20 || !Field->isBitField()) && + (!isa(Class->getDeclContext()) || + !cast(Class->getDeclContext())->isUnion()) && + shouldBeDefaultMemberInitializer(InitValue)) { + auto Diag = diag(BeginLoc, "%0 should be initialized in an in-class" + " default member initializer") + << Field; + + SourceLocation FieldEnd = + Lexer::getLocForEndOfToken(Field->getSourceRange().getEnd(), 0, + *Result.SourceManager, getLangOpts()); + Diag << FixItHint::CreateInsertion(FieldEnd, UseAssignment ? " = " : "{") + << FixItHint::CreateInsertionFromRange( + FieldEnd, CharSourceRange(InitValueRange, true)) + << FixItHint::CreateInsertion(FieldEnd, UseAssignment ? "" : "}"); + + SourceLocation SemiColonEnd = + Lexer::findNextToken(EndLoc, *Result.SourceManager, getLangOpts()) + ->getEndLoc(); + CharSourceRange StmtRange = + CharSourceRange::getCharRange(BeginLoc, SemiColonEnd); + + Diag << FixItHint::CreateRemoval(StmtRange); + } else { + auto Diag = diag(BeginLoc, "'%0' should be initialized in a member" + " initializer of the constructor") + << Field->getName(); + + bool AddComma = false; + if (!Ctor->getNumCtorInitializers() && FirstToCtorInits) { + SourceLocation BodyPos = Ctor->getBody()->getBeginLoc(); + if (BodyPos.isMacroID()) + BodyPos = Result.SourceManager->getSpellingLoc(BodyPos); + SourceLocation NextPos = Ctor->getBeginLoc(); + if (NextPos.isMacroID()) + NextPos = Result.SourceManager->getSpellingLoc(NextPos); + do { + InsertPos = NextPos; + NextPos = Lexer::findNextToken(NextPos, *Result.SourceManager, + getLangOpts()) + ->getLocation(); + } while (NextPos != BodyPos); + InsertPos = Lexer::getLocForEndOfToken( + InsertPos, 0, *Result.SourceManager, getLangOpts()); + + Diag << FixItHint::CreateInsertion(InsertPos, " : "); } else { - auto Diag = - diag(S->getBeginLoc(), "%0 should be initialized in a member" - " initializer of the constructor") - << Field; - - bool AddComma = false; - if (!Ctor->getNumCtorInitializers() && FirstToCtorInits) { - SourceLocation BodyPos = Ctor->getBody()->getBeginLoc(); - SourceLocation NextPos = Ctor->getBeginLoc(); - do { - InsertPos = NextPos; - NextPos = Lexer::findNextToken(NextPos, *Result.SourceManager, - getLangOpts()) - ->getLocation(); - } while (NextPos != BodyPos); - InsertPos = Lexer::getLocForEndOfToken( - InsertPos, 0, *Result.SourceManager, getLangOpts()); - - Diag << FixItHint::CreateInsertion(InsertPos, " : "); - } else { - bool Found = false; - for (const auto *Init : Ctor->inits()) { - if (Init->isMemberInitializer()) { - if (Result.SourceManager->isBeforeInTranslationUnit( - Field->getLocation(), Init->getMember()->getLocation())) { - InsertPos = Init->getSourceLocation(); - Found = true; - break; - } + bool Found = false; + for (const auto *Init : Ctor->inits()) { + if (Init->isMemberInitializer()) { + SourceLocation InitLoc = Init->getSourceLocation(); + if (InitLoc.isMacroID()) + InitLoc = Result.SourceManager->getSpellingLoc(InitLoc); + SourceLocation InitMemberLoc = Init->getMember()->getLocation(); + if (InitMemberLoc.isMacroID()) + InitMemberLoc = + Result.SourceManager->getSpellingLoc(InitMemberLoc); + SourceLocation FieldLoc = Field->getLocation(); + if (FieldLoc.isMacroID()) + FieldLoc = Result.SourceManager->getSpellingLoc(FieldLoc); + if (Result.SourceManager->isBeforeInTranslationUnit( + FieldLoc, InitMemberLoc)) { + InsertPos = InitLoc; + Found = true; + break; } } + } - if (!Found) { - if (Ctor->getNumCtorInitializers()) { - InsertPos = Lexer::getLocForEndOfToken( - (*Ctor->init_rbegin())->getSourceRange().getEnd(), 0, - *Result.SourceManager, getLangOpts()); - } - Diag << FixItHint::CreateInsertion(InsertPos, ", "); - } else { - AddComma = true; + if (!Found) { + if (Ctor->getNumCtorInitializers()) { + SourceLocation LastInitEnd = + (*Ctor->init_rbegin())->getSourceRange().getEnd(); + if (LastInitEnd.isMacroID()) + LastInitEnd = Result.SourceManager->getSpellingLoc(LastInitEnd); + InsertPos = Lexer::getLocForEndOfToken( + LastInitEnd, 0, *Result.SourceManager, getLangOpts()); } - } - Diag << FixItHint::CreateInsertion(InsertPos, Field->getName()) - << FixItHint::CreateInsertion(InsertPos, "(") - << FixItHint::CreateInsertionFromRange( - InsertPos, - CharSourceRange(InitValue->getSourceRange(), true)) - << FixItHint::CreateInsertion(InsertPos, ")"); - if (AddComma) Diag << FixItHint::CreateInsertion(InsertPos, ", "); - - SourceLocation SemiColonEnd = - Lexer::findNextToken(S->getEndLoc(), *Result.SourceManager, - getLangOpts()) - ->getEndLoc(); - CharSourceRange StmtRange = - CharSourceRange::getCharRange(S->getBeginLoc(), SemiColonEnd); - - Diag << FixItHint::CreateRemoval(StmtRange); - FirstToCtorInits = false; + } else { + AddComma = true; + } } + + Diag << FixItHint::CreateInsertion(InsertPos, Field->getName()) + << FixItHint::CreateInsertion(InsertPos, "(") + << FixItHint::CreateInsertionFromRange( + InsertPos, CharSourceRange(InitValueRange, true)) + << FixItHint::CreateInsertion(InsertPos, ")"); + if (AddComma) + Diag << FixItHint::CreateInsertion(InsertPos, ", "); + + SourceLocation SemiColonEnd = + Lexer::findNextToken(EndLoc, *Result.SourceManager, getLangOpts()) + ->getEndLoc(); + CharSourceRange StmtRange = + CharSourceRange::getCharRange(BeginLoc, SemiColonEnd); + + Diag << FixItHint::CreateRemoval(StmtRange); + FirstToCtorInits = false; } } } Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp +++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp @@ -1,5 +1,11 @@ // RUN: %check_clang_tidy %s cppcoreguidelines-prefer-member-initializer %t -- -- -fcxx-exceptions +// CHECK-MESSAGES: warning: 'qqqq' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer] +// FIXME: The source location of the assignment in MACRO6 (see towards the end +// of this test file) is the macro itself and the spelling location is +// :6:1 for some strange reason. This results this warning +// to be placed at the very beginning of the message list. + extern void __assert_fail (__const char *__assertion, __const char *__file, unsigned int __line, __const char *__function) __attribute__ ((__noreturn__)); @@ -488,3 +494,39 @@ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'x' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer] // CHECK-FIXES: {{^\ *$}} } + +#define MACRO1 struct InMacro1 { int i; InMacro1() { i = 0; } }; +// CHECK-MESSAGES: :[[@LINE-1]]:54: warning: 'i' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer] +// CHECK-FIXES: #define MACRO1 struct InMacro1 { int i; InMacro1() : i(0) { } }; +MACRO1 + +#define MACRO2 struct InMacro2 { int i, j; InMacro2() : i(0) { j = 1; } }; +// CHECK-MESSAGES: :[[@LINE-1]]:64: warning: 'j' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer] +// CHECK-FIXES: #define MACRO2 struct InMacro2 { int i, j; InMacro2() : i(0), j(1) { } }; +MACRO2 + +#define MACRO3 struct InMacro3 { int i, j; InMacro3() : j(1) { i = 0; } }; +// CHECK-MESSAGES: :[[@LINE-1]]:64: warning: 'i' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer] +// CHECK-FIXES: #define MACRO3 struct InMacro3 { int i, j; InMacro3() : i(0), j(1) { } }; +MACRO3 + +#define MACRO4(field) struct InMacro4 { int field; InMacro4() { field = 0; } }; +// C_HECK-MESSAGES: :[[@LINE-1]]:65: warning: 'field' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer] +// C_HECK-FIXES: #define MACRO4(field) struct InMacro4 { int field; InMacro4() : field(0) { } }; +MACRO4(q) +// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'q' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer] +// FIXME: The source location of the assignment is the macro itself and the +// spelling location is the argument in the macro call, instead of the +// place where it is used inside the macro. + +#define MACRO5(X) X +MACRO5(struct InMacro5 { int field; InMacro5() { field = 0; } };) +// CHECK-MESSAGES: :[[@LINE-1]]:50: warning: 'field' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer] +// CHECK-FIXES: MACRO5(struct InMacro5 { int field; InMacro5() : field(0) { } };) + +#define MACRO6(field) struct InMacro6 { int qqq ## field; InMacro6() { \ + qqq ## field = 0; } }; +// C_HECK-MESSAGES: :[[@LINE-1]]:3: warning: 'qqq ## field' should be initialized in a member initializer of the constructor [cppcoreguidelines-prefer-member-initializer] +// C_HECK-FIXES: #define MACRO6(field) struct InMacro6 { int qqq ### field; InMacro6() : qqq ## field(0) { \ +// C_HECK-FIXES: } }; +MACRO6(q)