baloghadamsoftware created this revision.
baloghadamsoftware added reviewers: aaron.ballman, gribozavr2.
baloghadamsoftware added a project: clang-tools-extra.
Herald added subscribers: martong, gamesh411, Szelethus, dkrupp, rnkovacs,
kbarton, xazax.hun, whisperity, nemanjai.
Herald added a project: clang.
baloghadamsoftware requested review of this revision.
`cppcoreguidelines-prefer-member-initializer` crashes on classes declared
inside macros (see bug 47778 <https://bugs.llvm.org/show_bug.cgi?id=47778>).
This patch fixes this issue.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D89380
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp
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
@@ -488,3 +488,18 @@
// 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
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
@@ -144,99 +144,132 @@
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<RecordDecl>(Class->getDeclContext()) ||
- !cast<RecordDecl>(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<RecordDecl>(Class->getDeclContext()) ||
+ !cast<RecordDecl>(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;
+
+ 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;
}
}
}
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits