[clang] 279ea93 - [clang] Add fixit for Wreorder-ctor
Nathan James via cfe-commits
cfe-commits at lists.llvm.org
Wed Mar 24 12:23:08 PDT 2021
Author: Nathan James
Date: 2021-03-24T19:22:53Z
New Revision: 279ea930fa21b283688b2598816095a48d0ca4d7
URL: https://github.com/llvm/llvm-project/commit/279ea930fa21b283688b2598816095a48d0ca4d7
DIFF: https://github.com/llvm/llvm-project/commit/279ea930fa21b283688b2598816095a48d0ca4d7.diff
LOG: [clang] Add fixit for Wreorder-ctor
Create fix-it hints to fix the order of constructors.
To make this a lot simpler, I've grouped all the warnings for each out of order initializer into 1.
This is necessary as fixing one initializer would often interfere with other initializers.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D98745
Added:
clang/test/FixIt/fixit-cxx-init-order.cpp
Modified:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/SemaCXX/constructor-initializer.cpp
clang/test/SemaCXX/warn-reorder-ctor-initialization.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 492ff63fe5ad..58e221a00468 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8606,6 +8606,15 @@ def warn_initializer_out_of_order : Warning<
"%select{field|base class}0 %1 will be initialized after "
"%select{field|base}2 %3">,
InGroup<ReorderCtor>, DefaultIgnore;
+
+def warn_some_initializers_out_of_order : Warning<
+ "initializer order does not match the declaration order">,
+ InGroup<ReorderCtor>, DefaultIgnore;
+
+def note_initializer_out_of_order : Note<
+ "%select{field|base class}0 %1 will be initialized after "
+ "%select{field|base}2 %3">;
+
def warn_abstract_vbase_init_ignored : Warning<
"initializer for virtual base class %0 of abstract class %1 "
"will never be used">,
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 8470fad39854..f54dd4cb6f43 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -5234,6 +5234,20 @@ static const void *GetKeyForMember(ASTContext &Context,
return Member->getAnyMember()->getCanonicalDecl();
}
+static void AddInitializerToDiag(const Sema::SemaDiagnosticBuilder &Diag,
+ const CXXCtorInitializer *Previous,
+ const CXXCtorInitializer *Current) {
+ if (Previous->isAnyMemberInitializer())
+ Diag << 0 << Previous->getAnyMember();
+ else
+ Diag << 1 << Previous->getTypeSourceInfo()->getType();
+
+ if (Current->isAnyMemberInitializer())
+ Diag << 0 << Current->getAnyMember();
+ else
+ Diag << 1 << Current->getTypeSourceInfo()->getType();
+}
+
static void DiagnoseBaseOrMemInitializerOrder(
Sema &SemaRef, const CXXConstructorDecl *Constructor,
ArrayRef<CXXCtorInitializer *> Inits) {
@@ -5283,10 +5297,15 @@ static void DiagnoseBaseOrMemInitializerOrder(
unsigned NumIdealInits = IdealInitKeys.size();
unsigned IdealIndex = 0;
- CXXCtorInitializer *PrevInit = nullptr;
+ // Track initializers that are in an incorrect order for either a warning or
+ // note if multiple ones occur.
+ SmallVector<unsigned> WarnIndexes;
+ // Correlates the index of an initializer in the init-list to the index of
+ // the field/base in the class.
+ SmallVector<std::pair<unsigned, unsigned>, 32> CorrelatedInitOrder;
+
for (unsigned InitIndex = 0; InitIndex != Inits.size(); ++InitIndex) {
- CXXCtorInitializer *Init = Inits[InitIndex];
- const void *InitKey = GetKeyForMember(SemaRef.Context, Init);
+ const void *InitKey = GetKeyForMember(SemaRef.Context, Inits[InitIndex]);
// Scan forward to try to find this initializer in the idealized
// initializers list.
@@ -5297,20 +5316,8 @@ static void DiagnoseBaseOrMemInitializerOrder(
// 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);
-
- if (PrevInit->isAnyMemberInitializer())
- D << 0 << PrevInit->getAnyMember()->getDeclName();
- else
- D << 1 << PrevInit->getTypeSourceInfo()->getType();
-
- if (Init->isAnyMemberInitializer())
- D << 0 << Init->getAnyMember()->getDeclName();
- else
- D << 1 << Init->getTypeSourceInfo()->getType();
+ if (IdealIndex == NumIdealInits && InitIndex) {
+ WarnIndexes.push_back(InitIndex);
// Move back to the initializer's location in the ideal list.
for (IdealIndex = 0; IdealIndex != NumIdealInits; ++IdealIndex)
@@ -5320,8 +5327,54 @@ static void DiagnoseBaseOrMemInitializerOrder(
assert(IdealIndex < NumIdealInits &&
"initializer not found in initializer list");
}
+ CorrelatedInitOrder.emplace_back(IdealIndex, InitIndex);
+ }
- PrevInit = Init;
+ if (WarnIndexes.empty())
+ return;
+
+ // Sort based on the ideal order, first in the pair.
+ llvm::sort(CorrelatedInitOrder,
+ [](auto &LHS, auto &RHS) { return LHS.first < RHS.first; });
+
+ // Introduce a new scope as SemaDiagnosticBuilder needs to be destroyed to
+ // emit the diagnostic before we can try adding notes.
+ {
+ Sema::SemaDiagnosticBuilder D = SemaRef.Diag(
+ Inits[WarnIndexes.front() - 1]->getSourceLocation(),
+ WarnIndexes.size() == 1 ? diag::warn_initializer_out_of_order
+ : diag::warn_some_initializers_out_of_order);
+
+ for (unsigned I = 0; I < CorrelatedInitOrder.size(); ++I) {
+ if (CorrelatedInitOrder[I].second == I)
+ continue;
+ // Ideally we would be using InsertFromRange here, but clang doesn't
+ // appear to handle InsertFromRange correctly when the source range is
+ // modified by another fix-it.
+ D << FixItHint::CreateReplacement(
+ Inits[I]->getSourceRange(),
+ Lexer::getSourceText(
+ CharSourceRange::getTokenRange(
+ Inits[CorrelatedInitOrder[I].second]->getSourceRange()),
+ SemaRef.getSourceManager(), SemaRef.getLangOpts()));
+ }
+
+ // If there is only 1 item out of order, the warning expects the name and
+ // type of each being added to it.
+ if (WarnIndexes.size() == 1) {
+ AddInitializerToDiag(D, Inits[WarnIndexes.front() - 1],
+ Inits[WarnIndexes.front()]);
+ return;
+ }
+ }
+ // More than 1 item to warn, create notes letting the user know which ones
+ // are bad.
+ for (unsigned WarnIndex : WarnIndexes) {
+ const clang::CXXCtorInitializer *PrevInit = Inits[WarnIndex - 1];
+ auto D = SemaRef.Diag(PrevInit->getSourceLocation(),
+ diag::note_initializer_out_of_order);
+ AddInitializerToDiag(D, PrevInit, Inits[WarnIndex]);
+ D << PrevInit->getSourceRange();
}
}
@@ -5389,7 +5442,7 @@ bool CheckRedundantUnionInit(Sema &S,
return false;
}
-}
+} // namespace
/// ActOnMemInitializers - Handle the member initializers for a constructor.
void Sema::ActOnMemInitializers(Decl *ConstructorDecl,
diff --git a/clang/test/FixIt/fixit-cxx-init-order.cpp b/clang/test/FixIt/fixit-cxx-init-order.cpp
new file mode 100644
index 000000000000..f39c8bf2a15c
--- /dev/null
+++ b/clang/test/FixIt/fixit-cxx-init-order.cpp
@@ -0,0 +1,22 @@
+// Due to the fix having multiple edits we can't use
+// '-fdiagnostics-parseable-fixits' to determine if fixes are correct. However,
+// running fixit recompile with 'Werror' should fail if the fixes are invalid.
+
+// RUN: %clang_cc1 %s -Werror=reorder-ctor -fixit-recompile -fixit-to-temporary
+// RUN: %clang_cc1 %s -Wreorder-ctor -verify -verify-ignore-unexpected=note
+
+struct Foo {
+ int A, B, C;
+
+ Foo() : A(1), B(3), C(2) {}
+ Foo(int) : A(1), C(2), B(3) {} // expected-warning {{field 'C' will be initialized after field 'B'}}
+ Foo(unsigned) : C(2), B(3), A(1) {} // expected-warning {{initializer order does not match the declaration order}}
+};
+
+struct Bar : Foo {
+ int D, E, F;
+
+ Bar() : Foo(), D(1), E(2), F(3) {}
+ Bar(int) : D(1), E(2), F(3), Foo(4) {} // expected-warning {{field 'F' will be initialized after base 'Foo'}}
+ Bar(unsigned) : F(3), E(2), D(1), Foo(4) {} // expected-warning {{initializer order does not match the declaration order}}
+};
diff --git a/clang/test/SemaCXX/constructor-initializer.cpp b/clang/test/SemaCXX/constructor-initializer.cpp
index df8991416712..874682944bb8 100644
--- a/clang/test/SemaCXX/constructor-initializer.cpp
+++ b/clang/test/SemaCXX/constructor-initializer.cpp
@@ -91,13 +91,14 @@ struct Derived : Base, Base1, virtual V {
struct Current : Derived {
int Derived;
- Current() : Derived(1), ::Derived(), // expected-warning {{field 'Derived' will be initialized after base '::Derived'}} \
- // expected-warning {{base class '::Derived' will be initialized after base 'Derived::V'}}
- ::Derived::Base(), // expected-error {{type '::Derived::Base' is not a direct or virtual base of 'Current'}}
- Derived::Base1(), // expected-error {{type 'Derived::Base1' is not a direct or virtual base of 'Current'}}
- Derived::V(),
- ::NonExisting(), // expected-error {{member initializer 'NonExisting' does not name a non-static data member or}}
- INT::NonExisting() {} // expected-error {{'INT' (aka 'int') is not a class, namespace, or enumeration}} \
+ Current() : Derived(1), ::Derived(), // expected-warning {{initializer order does not match the declaration order}} \
+ // expected-note {{field 'Derived' will be initialized after base '::Derived'}} \
+ // expected-note {{base class '::Derived' will be initialized after base 'Derived::V'}}
+ ::Derived::Base(), // expected-error {{type '::Derived::Base' is not a direct or virtual base of 'Current'}}
+ Derived::Base1(), // expected-error {{type 'Derived::Base1' is not a direct or virtual base of 'Current'}}
+ Derived::V(),
+ ::NonExisting(), // expected-error {{member initializer 'NonExisting' does not name a non-static data member or}}
+ INT::NonExisting() {} // expected-error {{'INT' (aka 'int') is not a class, namespace, or enumeration}} \
// expected-error {{member initializer 'NonExisting' does not name a non-static data member or}}
};
diff --git a/clang/test/SemaCXX/warn-reorder-ctor-initialization.cpp b/clang/test/SemaCXX/warn-reorder-ctor-initialization.cpp
index 6d38ec95fbfb..47588660d204 100644
--- a/clang/test/SemaCXX/warn-reorder-ctor-initialization.cpp
+++ b/clang/test/SemaCXX/warn-reorder-ctor-initialization.cpp
@@ -4,15 +4,14 @@ struct BB {};
struct BB1 {};
-class complex : public BB, BB1 {
-public:
+class complex : public BB, BB1 {
+public:
complex()
- : s2(1), // expected-warning {{field 's2' will be initialized after field 's1'}}
- s1(1),
- s3(3), // expected-warning {{field 's3' will be initialized after base 'BB1'}}
- BB1(), // expected-warning {{base class 'BB1' will be initialized after base 'BB'}}
- BB()
- {}
+ : s2(1), // expected-warning {{initializer order does not match the declaration order}} expected-note {{field 's2' will be initialized after field 's1'}}
+ s1(1),
+ s3(3), // expected-note {{field 's3' will be initialized after base 'BB1'}}
+ BB1(), // expected-note {{base class 'BB1' will be initialized after base 'BB'}}
+ BB() {}
int s1;
int s2;
int s3;
More information about the cfe-commits
mailing list