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
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to