adamcz updated this revision to Diff 290313.
adamcz added a comment.
Add a missing const
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87244/new/
https://reviews.llvm.org/D87244
Files:
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/SemaCXX/warn-reorder-ctor-initialization.cpp
Index: clang/test/SemaCXX/warn-reorder-ctor-initialization.cpp
===================================================================
--- clang/test/SemaCXX/warn-reorder-ctor-initialization.cpp
+++ clang/test/SemaCXX/warn-reorder-ctor-initialization.cpp
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -fsyntax-only -Wreorder -verify %s
+// RUN: %clang_cc1 -fsyntax-only -fdiagnostics-parseable-fixits -Wreorder %s 2>&1 | FileCheck %s
struct BB {};
@@ -122,6 +123,9 @@
}
namespace PR7179 {
+struct Foo {
+ Foo();
+};
struct X
{
struct Y
@@ -129,6 +133,13 @@
template <class T> Y(T x) : X(x) { }
};
};
+
+ struct X2 {
+ struct Y {
+ template <class T> Y(T x) : a(), X(x) {}
+ Foo a;
+ };
+ };
}
namespace test3 {
@@ -141,3 +152,23 @@
}
};
}
+
+namespace fix {
+struct Foo {
+ Foo(int) {}
+};
+struct Bar {
+ Foo a, b, c;
+
+ Bar() : c(1), a(2), b(3) {} // expected-warning {{field 'c' will be initialized after field 'a'}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:13-[[@LINE-1]]:17}:"a(2)"
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:19-[[@LINE-2]]:23}:"b(3)"
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:25-[[@LINE-3]]:29}:"c(1)"
+
+ // Check that we do not generate fix it when comments are present. Ideally
+ // we would, but comments would need to be reordered as well and the current
+ // code can't do this.
+ Bar(int x) : c(x), /*foo*/ a(2), b(3) {} // expected-warning {{field 'c' will be initialized after field 'a'}}
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
+};
+} // namespace fix
Index: clang/lib/Sema/SemaDeclCXX.cpp
===================================================================
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -5192,6 +5192,26 @@
return Member->getAnyMember()->getCanonicalDecl();
}
+// Returns true iff there are comments between Begin and End. Expects that both
+// Begin and End are in the same file.
+static bool RangeContainsComments(Sema &SemaRef, SourceLocation Begin,
+ SourceLocation End) {
+ auto &SrcMgr = SemaRef.getSourceManager();
+ auto BeginLocInfo = SrcMgr.getDecomposedLoc(Begin);
+ StringRef File = SrcMgr.getBufferData(BeginLocInfo.first);
+ Lexer Lex(SrcMgr.getLocForStartOfFile(BeginLocInfo.first),
+ SemaRef.getLangOpts(), File.begin(),
+ File.begin() + BeginLocInfo.second, File.end());
+ Lex.SetCommentRetentionState(true);
+ Token Tok;
+ while (!Lex.LexFromRawLexer(Tok) &&
+ SrcMgr.isBeforeInTranslationUnit(Tok.getLocation(), End)) {
+ if (Tok.is(tok::comment))
+ return true;
+ }
+ return false;
+}
+
static void DiagnoseBaseOrMemInitializerOrder(
Sema &SemaRef, const CXXConstructorDecl *Constructor,
ArrayRef<CXXCtorInitializer *> Inits) {
@@ -5241,7 +5261,20 @@
unsigned NumIdealInits = IdealInitKeys.size();
unsigned IdealIndex = 0;
- CXXCtorInitializer *PrevInit = nullptr;
+ // Instead of emitting diagnostic right away, we keep it here and only emit
+ // when we find a new one. The last one is emitted outside of this loop, with
+ // a FixIt attached.
+ PartialDiagnosticAt PDiag = {SourceLocation(),
+ PartialDiagnostic::NullDiagnostic()};
+ // Pairs {IdealIndex, pointer-to-initializer}, valid (i.e. found in
+ // IdealInitKeys) initializers only. Last value is used to generate
+ // diagnostics, the index is then used to sort valid initializers in
+ // initialization order.
+ SmallVector<std::pair<unsigned, CXXCtorInitializer *>, 32>
+ InitsWithIdealIndex;
+ // MissingInIdeal[I] == true iff Inits[I] is not found in ideal list.
+ llvm::SmallBitVector MissingInIdeal(Inits.size());
+
for (unsigned InitIndex = 0; InitIndex != Inits.size(); ++InitIndex) {
CXXCtorInitializer *Init = Inits[InitIndex];
const void *InitKey = GetKeyForMember(SemaRef.Context, Init);
@@ -5252,35 +5285,100 @@
if (InitKey == IdealInitKeys[IdealIndex])
break;
- // If we didn't find this initializer, it must be because we
- // scanned past it on a previous iteration. That can only
- // happen if we're out of order; emit a warning.
- if (IdealIndex == NumIdealInits && PrevInit) {
- Sema::SemaDiagnosticBuilder D =
- SemaRef.Diag(PrevInit->getSourceLocation(),
- diag::warn_initializer_out_of_order);
+ const bool IsOutOfOrder = IdealIndex == NumIdealInits;
+ if (IsOutOfOrder) {
+ if (!InitsWithIdealIndex.empty()) {
+ // This is not the first initializer we process. It's either not in the
+ // ideal list at all, or it's out of order. Scan from the beginning.
+ for (IdealIndex = 0; IdealIndex != NumIdealInits; ++IdealIndex)
+ if (InitKey == IdealInitKeys[IdealIndex])
+ break;
+ }
+ if (IdealIndex == NumIdealInits) {
+ // This initializer is not in the ideal list at all. This can happen in
+ // a class with dependent base, when we cannot tell if initializer is
+ // valid or not. Just ignore it and reset IdealIndex to the previous
+ // valid initializer.
+ MissingInIdeal[InitIndex] = true;
+ IdealIndex =
+ InitsWithIdealIndex.empty() ? 0 : InitsWithIdealIndex.back().first;
+ continue;
+ }
+ }
+
+ // If we found this initializer in an idealized list, but it was before the
+ // previous (valid) initializer, emit a warning.
+ if (IsOutOfOrder && !InitsWithIdealIndex.empty()) {
+ // Emit previous diagnostic, if any.
+ if (PDiag.second.getDiagID())
+ SemaRef.Diag(PDiag.first, PDiag.second);
+
+ const auto *PrevInit = InitsWithIdealIndex.back().second;
+ PDiag = {PrevInit->getSourceLocation(),
+ SemaRef.PDiag(diag::warn_initializer_out_of_order)};
if (PrevInit->isAnyMemberInitializer())
- D << 0 << PrevInit->getAnyMember()->getDeclName();
+ PDiag.second << 0 << PrevInit->getAnyMember()->getDeclName();
else
- D << 1 << PrevInit->getTypeSourceInfo()->getType();
+ PDiag.second << 1 << PrevInit->getTypeSourceInfo()->getType();
if (Init->isAnyMemberInitializer())
- D << 0 << Init->getAnyMember()->getDeclName();
+ PDiag.second << 0 << Init->getAnyMember()->getDeclName();
else
- D << 1 << Init->getTypeSourceInfo()->getType();
+ PDiag.second << 1 << Init->getTypeSourceInfo()->getType();
+ }
- // Move back to the initializer's location in the ideal list.
- for (IdealIndex = 0; IdealIndex != NumIdealInits; ++IdealIndex)
- if (InitKey == IdealInitKeys[IdealIndex])
- break;
+ InitsWithIdealIndex.push_back({IdealIndex, Init});
+ }
- assert(IdealIndex < NumIdealInits &&
- "initializer not found in initializer list");
+ if (!PDiag.second.getDiagID())
+ return;
+
+ // If possible, add fix to the last diagnostic. The fix will reorder all
+ // initializers. This is preferable to attaching overlapping fixes for each
+ // initializer, which the user would then have to apply one by one.
+ auto &SrcMgr = SemaRef.getSourceManager();
+ // Things get complicated when comments, macros or multiple files are
+ // involved. Check if that's the case and if so, do not emit the fix it.
+ // Ideally we would reorder the comments as well, but that is a lot more
+ // complicated.
+ // FIXME: Support re-ordering initializers with comments.
+ bool ShouldEmitFix = true;
+ for (const auto &Init : Inits) {
+ if (Init->getSourceLocation().isMacroID() ||
+ !SrcMgr.isWrittenInSameFile(Init->getSourceLocation(),
+ Inits.front()->getSourceLocation())) {
+ ShouldEmitFix = false;
+ break;
}
+ }
+ if (ShouldEmitFix)
+ ShouldEmitFix =
+ !RangeContainsComments(SemaRef, Inits.front()->getSourceLocation(),
+ Inits.back()->getSourceLocation());
- PrevInit = Init;
+ if (ShouldEmitFix) {
+ // Construct a FixIt for re-ordering the initializers.
+ llvm::sort(InitsWithIdealIndex.begin(), InitsWithIdealIndex.end());
+ unsigned ReorderInitIndex = 0;
+ for (const auto &InitIndexPair : InitsWithIdealIndex) {
+ auto *InitToInsert = InitIndexPair.second;
+ const CXXCtorInitializer *InitToReplace = Inits[ReorderInitIndex];
+ PDiag.second << FixItHint::CreateReplacement(
+ InitToReplace->getSourceRange(),
+ clang::Lexer::getSourceText(
+ CharSourceRange(InitToInsert->getSourceRange(), true), SrcMgr,
+ SemaRef.getLangOpts()));
+
+ // Advance ReorderInitIndex to next valid initializer.
+ do {
+ ++ReorderInitIndex;
+ } while (ReorderInitIndex < Inits.size() &&
+ MissingInIdeal[ReorderInitIndex]);
+ }
}
+
+ SemaRef.Diag(PDiag.first, PDiag.second);
}
namespace {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
