Author: d0k Date: Wed Aug 30 13:18:40 2017 New Revision: 312166 URL: http://llvm.org/viewvc/llvm-project?rev=312166&view=rev Log: [cppcoreguidelines] Don't rely on SmallPtrSet iteration order.
The fixit emission breaks if the iteration order changes and also missed to emit fixits for some edge cases. Modified: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp Modified: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp?rev=312166&r1=312165&r2=312166&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp Wed Aug 30 13:18:40 2017 @@ -32,22 +32,16 @@ AST_MATCHER(CXXRecordDecl, hasDefaultCon } // Iterate over all the fields in a record type, both direct and indirect (e.g. -// if the record contains an anonmyous struct). If OneFieldPerUnion is true and -// the record type (or indirect field) is a union, forEachField will stop after -// the first field. +// if the record contains an anonmyous struct). template <typename T, typename Func> -void forEachField(const RecordDecl &Record, const T &Fields, - bool OneFieldPerUnion, Func &&Fn) { +void forEachField(const RecordDecl &Record, const T &Fields, Func &&Fn) { for (const FieldDecl *F : Fields) { if (F->isAnonymousStructOrUnion()) { if (const CXXRecordDecl *R = F->getType()->getAsCXXRecordDecl()) - forEachField(*R, R->fields(), OneFieldPerUnion, Fn); + forEachField(*R, R->fields(), Fn); } else { Fn(F); } - - if (OneFieldPerUnion && Record.isUnion()) - break; } } @@ -227,7 +221,7 @@ void getInitializationsInOrder(const CXX Decls.emplace_back(Decl); } } - forEachField(ClassDecl, ClassDecl.fields(), false, + forEachField(ClassDecl, ClassDecl.fields(), [&](const FieldDecl *F) { Decls.push_back(F); }); } @@ -353,7 +347,7 @@ void ProTypeMemberInitCheck::checkMissin // Gather all fields (direct and indirect) that need to be initialized. SmallPtrSet<const FieldDecl *, 16> FieldsToInit; - forEachField(ClassDecl, ClassDecl.fields(), false, [&](const FieldDecl *F) { + forEachField(ClassDecl, ClassDecl.fields(), [&](const FieldDecl *F) { if (!F->hasInClassInitializer() && utils::type_traits::isTriviallyDefaultConstructible(F->getType(), Context) && @@ -379,12 +373,12 @@ void ProTypeMemberInitCheck::checkMissin // Collect all fields in order, both direct fields and indirect fields from // anonmyous record types. SmallVector<const FieldDecl *, 16> OrderedFields; - forEachField(ClassDecl, ClassDecl.fields(), false, + forEachField(ClassDecl, ClassDecl.fields(), [&](const FieldDecl *F) { OrderedFields.push_back(F); }); // Collect all the fields we need to initialize, including indirect fields. SmallPtrSet<const FieldDecl *, 16> AllFieldsToInit; - forEachField(ClassDecl, FieldsToInit, false, + forEachField(ClassDecl, FieldsToInit, [&](const FieldDecl *F) { AllFieldsToInit.insert(F); }); if (AllFieldsToInit.empty()) return; @@ -404,11 +398,16 @@ void ProTypeMemberInitCheck::checkMissin // Collect all fields but only suggest a fix for the first member of unions, // as initializing more than one union member is an error. SmallPtrSet<const FieldDecl *, 16> FieldsToFix; - forEachField(ClassDecl, FieldsToInit, true, [&](const FieldDecl *F) { + SmallPtrSet<const RecordDecl *, 4> UnionsSeen; + forEachField(ClassDecl, OrderedFields, [&](const FieldDecl *F) { + if (!FieldsToInit.count(F)) + return; // Don't suggest fixes for enums because we don't know a good default. // Don't suggest fixes for bitfields because in-class initialization is not // possible. - if (!F->getType()->isEnumeralType() && !F->isBitField()) + if (F->getType()->isEnumeralType() || F->isBitField()) + return; + if (!F->getParent()->isUnion() || UnionsSeen.insert(F->getParent()).second) FieldsToFix.insert(F); }); if (FieldsToFix.empty()) Modified: clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp?rev=312166&r1=312165&r2=312166&view=diff ============================================================================== --- clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp (original) +++ clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp Wed Aug 30 13:18:40 2017 @@ -312,6 +312,19 @@ union PositiveUnion { // CHECK-FIXES-NOT: float Y{}; }; +union PositiveUnionReversed { + PositiveUnionReversed() : X() {} // No message as a union can only initialize one member. + PositiveUnionReversed(int) {} + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: union constructor should initialize one of these fields: Y, X + + // Make sure we don't give Y an initializer. + TestEnum Y; + // CHECK-FIXES-NOT: TestEnum Y{}; + + int X; + // CHECK-FIXES: int X{}; +}; + struct PositiveAnonymousUnionAndStruct { PositiveAnonymousUnionAndStruct() {} // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: A, B, Y, Z, C, D, E, F, X _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits