baloghadamsoftware updated this revision to Diff 299378.
baloghadamsoftware added a comment.
Tests added, code reformatted.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89380/new/
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
@@ -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
+// <scratch space>: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)
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<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->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;
}
}
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
