sameconrad created this revision.
sameconrad added a project: clang-tools-extra.
This adds a warning emitted by clang-reorder-fields when the reordering fields
breaks dependencies in the initializer list (such that -Wuninitialized would
warn when compiling). For example, given:
Foo::Foo(int x)
: a(x)
, b(a) {}
Reordering fields to [b,a] gives:
Foo::Foo(int x)
: b(a)
, a(x) {}
Emits the warning:
2: Warning: reordering field a after b makes a uninitialized when used in init
expression.
https://reviews.llvm.org/D35972
Files:
clang-reorder-fields/ReorderFieldsAction.cpp
test/clang-reorder-fields/FieldDependencyWarning.cpp
Index: test/clang-reorder-fields/FieldDependencyWarning.cpp
===================================================================
--- test/clang-reorder-fields/FieldDependencyWarning.cpp
+++ test/clang-reorder-fields/FieldDependencyWarning.cpp
@@ -0,0 +1,42 @@
+// RUN: clang-reorder-fields -record-name bar::Foo -fields-order y,z,c,x %s -- 2>&1 | FileCheck %s
+
+#include <string>
+
+namespace bar {
+class Foo {
+public:
+ Foo(int x, double y, char cin);
+ Foo(int nx);
+ Foo();
+ int x;
+ double y;
+ char c;
+ std::string z;
+};
+
+static char bar(char c) {
+ return c + 1;
+}
+
+Foo::Foo(int x, double y, char cin) :
+ x(x),
+ y(y),
+ c(cin),
+ z(this->x, bar(c)) // CHECK: [[@LINE]]: Warning: reordering field x after z makes x uninitialized when used in init expression
+ // CHECK-NEXT: [[@LINE-1]]: Warning: reordering field c after z makes c uninitialized when used in init expression
+{}
+
+Foo::Foo(int nx) :
+ x(nx),
+ y(x), // CHECK-NEXT: [[@LINE]]: Warning: reordering field x after y makes x uninitialized when used in init expression
+ c(0),
+ z(bar(bar(x)), c) // CHECK-NEXT: [[@LINE]]: Warning: reordering field x after z makes x uninitialized when used in init expression
+ // CHECK-NEXT: [[@LINE-1]]: Warning: reordering field c after z makes c uninitialized when used in init expression
+{}
+
+} // namespace bar
+
+int main() {
+ bar::Foo F(5, 12.8, 'c');
+ return 0;
+}
Index: clang-reorder-fields/ReorderFieldsAction.cpp
===================================================================
--- clang-reorder-fields/ReorderFieldsAction.cpp
+++ clang-reorder-fields/ReorderFieldsAction.cpp
@@ -22,12 +22,14 @@
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Lex/Lexer.h"
#include "clang/Tooling/Refactoring.h"
+#include "llvm/ADT/SetVector.h"
#include <algorithm>
#include <string>
namespace clang {
namespace reorder_fields {
using namespace clang::ast_matchers;
+using llvm::SmallSetVector;
/// \brief Finds the definition of a record by name.
///
@@ -91,6 +93,24 @@
consumeError(Replacements[R.getFilePath()].add(R));
}
+/// \brief Find all member fields used in the given init-list initializer expr
+/// that belong to the same record
+///
+/// \returns a set of field declarations, empty if none were present
+static SmallSetVector<FieldDecl *, 1>
+findMembersUsedInInitExpr(const CXXCtorInitializer *Initializer,
+ ASTContext &Context) {
+ SmallSetVector<FieldDecl *, 1> Results;
+ auto FoundExprs =
+ match(findAll(memberExpr(hasObjectExpression(cxxThisExpr())).bind("ME")),
+ *Initializer->getInit(), Context);
+ for (BoundNodes &BN : FoundExprs)
+ if (auto *MemExpr = BN.getNodeAs<MemberExpr>("ME"))
+ if (auto *FD = dyn_cast<FieldDecl>(MemExpr->getMemberDecl()))
+ Results.insert(FD);
+ return Results;
+}
+
/// \brief Reorders fields in the definition of a struct/class.
///
/// At the moment reodering of fields with
@@ -133,7 +153,7 @@
/// Thus, we need to ensure that we reorder just the initializers that are present.
static void reorderFieldsInConstructor(
const CXXConstructorDecl *CtorDecl, ArrayRef<unsigned> NewFieldsOrder,
- const ASTContext &Context,
+ ASTContext &Context,
std::map<std::string, tooling::Replacements> &Replacements) {
assert(CtorDecl && "Constructor declaration is null");
if (CtorDecl->isImplicit() || CtorDecl->getNumCtorInitializers() <= 1)
@@ -153,6 +173,22 @@
for (const auto *Initializer : CtorDecl->inits()) {
if (!Initializer->isWritten())
continue;
+
+ // Warn if this reordering violates initialization expr dependencies
+ const auto UsedMembers = findMembersUsedInInitExpr(Initializer, Context);
+ const FieldDecl *ThisM = Initializer->getMember();
+ for (const FieldDecl *UM : UsedMembers) {
+ if (NewFieldsPositions[UM->getFieldIndex()] >
+ NewFieldsPositions[ThisM->getFieldIndex()]) {
+ llvm::errs() << Context.getSourceManager().getSpellingLineNumber(
+ Initializer->getSourceLocation())
+ << ": Warning: reordering field " << UM->getName()
+ << " after " << ThisM->getName() << " makes "
+ << UM->getName()
+ << " uninitialized when used in init expression\n";
+ }
+ }
+
OldWrittenInitializersOrder.push_back(Initializer);
NewWrittenInitializersOrder.push_back(Initializer);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
