diff options
Diffstat (limited to 'clang-tools-extra/clang-tidy/modernize')
3 files changed, 63 insertions, 40 deletions
diff --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp index b6cb64fa952..9af797927e2 100644 --- a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp @@ -375,7 +375,7 @@ static bool usagesAreConst(const UsageResult &Usages) { /// by reference. static bool usagesReturnRValues(const UsageResult &Usages) { for (const auto &U : Usages) { - if (!U.Expression->isRValue()) + if (U.Expression && !U.Expression->isRValue()) return false; } return true; @@ -400,8 +400,7 @@ void LoopConvertCheck::doConversion( ASTContext *Context, const VarDecl *IndexVar, const VarDecl *MaybeContainer, StringRef ContainerString, const UsageResult &Usages, const DeclStmt *AliasDecl, bool AliasUseRequired, bool AliasFromForInit, - const ForStmt *TheLoop, bool ContainerNeedsDereference, bool DerefByValue, - bool DerefByConstRef) { + const ForStmt *TheLoop, RangeDescriptor Descriptor) { auto Diag = diag(TheLoop->getForLoc(), "use range-based for loop instead"); std::string VarName; @@ -457,16 +456,17 @@ void LoopConvertCheck::doConversion( // If an iterator's operator*() returns a 'T&' we can bind that to 'auto&'. // If operator*() returns 'T' we can bind that to 'auto&&' which will deduce // to 'T&&&'. - if (DerefByValue) { - AutoRefType = Context->getRValueReferenceType(AutoRefType); + if (Descriptor.DerefByValue) { + if (!Descriptor.IsTriviallyCopyable) + AutoRefType = Context->getRValueReferenceType(AutoRefType); } else { - if (DerefByConstRef) + if (Descriptor.DerefByConstRef) AutoRefType = Context->getConstType(AutoRefType); AutoRefType = Context->getLValueReferenceType(AutoRefType); } } - StringRef MaybeDereference = ContainerNeedsDereference ? "*" : ""; + StringRef MaybeDereference = Descriptor.ContainerNeedsDereference ? "*" : ""; std::string TypeString = AutoRefType.getAsString(); std::string Range = ("(" + TypeString + " " + VarName + " : " + MaybeDereference + ContainerString + ")") @@ -518,11 +518,11 @@ StringRef LoopConvertCheck::checkRejections(ASTContext *Context, /// of the index variable and convert the loop if possible. void LoopConvertCheck::findAndVerifyUsages( ASTContext *Context, const VarDecl *LoopVar, const VarDecl *EndVar, - const Expr *ContainerExpr, const Expr *BoundExpr, - bool ContainerNeedsDereference, bool DerefByValue, bool DerefByConstRef, - const ForStmt *TheLoop, LoopFixerKind FixerKind) { + const Expr *ContainerExpr, const Expr *BoundExpr, const ForStmt *TheLoop, + LoopFixerKind FixerKind, RangeDescriptor Descriptor) { ForLoopIndexUseVisitor Finder(Context, LoopVar, EndVar, ContainerExpr, - BoundExpr, ContainerNeedsDereference); + BoundExpr, + Descriptor.ContainerNeedsDereference); if (ContainerExpr) { ComponentFinderASTVisitor ComponentFinder; @@ -544,15 +544,28 @@ void LoopConvertCheck::findAndVerifyUsages( !isDirectMemberExpr(ContainerExpr)) ConfidenceLevel.lowerTo(Confidence::CL_Risky); } else if (FixerKind == LFK_PseudoArray) { - if (!DerefByValue && !DerefByConstRef) { + if (!Descriptor.DerefByValue && !Descriptor.DerefByConstRef) { const UsageResult &Usages = Finder.getUsages(); if (usagesAreConst(Usages)) { - // FIXME: check if the type is trivially copiable. - DerefByConstRef = true; + Descriptor.DerefByConstRef = true; } else if (usagesReturnRValues(Usages)) { // If the index usages (dereference, subscript, at) return RValues, // then we should not use a non-const reference. - DerefByValue = true; + Descriptor.DerefByValue = true; + // Try to find the type of the elements on the container from the + // usages. + for (const Usage &U : Usages) { + if (!U.Expression || U.Expression->getType().isNull()) + continue; + QualType Type = U.Expression->getType().getCanonicalType(); + if (U.IsArrow) { + if (!Type->isPointerType()) + continue; + Type = Type->getPointeeType(); + } + Descriptor.IsTriviallyCopyable = + Type.isTriviallyCopyableType(*Context); + } } } } @@ -565,7 +578,7 @@ void LoopConvertCheck::findAndVerifyUsages( doConversion(Context, LoopVar, getReferencedVariable(ContainerExpr), ContainerString, Finder.getUsages(), Finder.getAliasDecl(), Finder.aliasUseRequired(), Finder.aliasFromForInit(), TheLoop, - ContainerNeedsDereference, DerefByValue, DerefByConstRef); + Descriptor); } void LoopConvertCheck::registerMatchers(MatchFinder *Finder) { @@ -618,15 +631,13 @@ void LoopConvertCheck::check(const MatchFinder::MatchResult &Result) { ConfidenceLevel.lowerTo(Confidence::CL_Reasonable); const Expr *ContainerExpr = nullptr; - bool DerefByValue = false; - bool DerefByConstRef = false; - bool ContainerNeedsDereference = false; + RangeDescriptor Descriptor{false, false, false, false}; // FIXME: Try to put most of this logic inside a matcher. Currently, matchers // don't allow the ight-recursive checks in digThroughConstructors. if (FixerKind == LFK_Iterator) { ContainerExpr = findContainer(Context, LoopVar->getInit(), EndVar ? EndVar->getInit() : EndCall, - &ContainerNeedsDereference); + &Descriptor.ContainerNeedsDereference); QualType InitVarType = InitVar->getType(); QualType CanonicalInitVarType = InitVarType.getCanonicalType(); @@ -643,6 +654,8 @@ void LoopConvertCheck::check(const MatchFinder::MatchResult &Result) { // un-qualified pointee types match otherwise we don't use auto. if (!Context->hasSameUnqualifiedType(InitPointeeType, BeginPointeeType)) return; + Descriptor.IsTriviallyCopyable = + BeginPointeeType.isTriviallyCopyableType(*Context); } else { // Check for qualified types to avoid conversions from non-const to const // iterator types. @@ -650,17 +663,19 @@ void LoopConvertCheck::check(const MatchFinder::MatchResult &Result) { return; } - DerefByValue = Nodes.getNodeAs<QualType>(DerefByValueResultName) != nullptr; - if (!DerefByValue) { + const auto *DerefByValueType = + Nodes.getNodeAs<QualType>(DerefByValueResultName); + Descriptor.DerefByValue = DerefByValueType; + if (!Descriptor.DerefByValue) { if (const auto *DerefType = Nodes.getNodeAs<QualType>(DerefByRefResultName)) { // A node will only be bound with DerefByRefResultName if we're dealing // with a user-defined iterator type. Test the const qualification of // the reference type. - DerefByConstRef = (*DerefType) - ->getAs<ReferenceType>() - ->getPointeeType() - .isConstQualified(); + Descriptor.DerefByConstRef = (*DerefType) + ->getAs<ReferenceType>() + ->getPointeeType() + .isConstQualified(); } else { // By nature of the matcher this case is triggered only for built-in // iterator types (i.e. pointers). @@ -671,12 +686,14 @@ void LoopConvertCheck::check(const MatchFinder::MatchResult &Result) { // If the initializer and variable have both the same type just use auto // otherwise we test for const qualification of the pointed-at type. if (!Context->hasSameType(InitPointeeType, BeginPointeeType)) - DerefByConstRef = InitPointeeType.isConstQualified(); + Descriptor.DerefByConstRef = InitPointeeType.isConstQualified(); } } else { // If the dereference operator returns by value then test for the // canonical const qualification of the init variable type. - DerefByConstRef = CanonicalInitVarType.isConstQualified(); + Descriptor.DerefByConstRef = CanonicalInitVarType.isConstQualified(); + Descriptor.IsTriviallyCopyable = + DerefByValueType->isTriviallyCopyableType(*Context); } } else if (FixerKind == LFK_PseudoArray) { if (!EndCall) @@ -685,7 +702,7 @@ void LoopConvertCheck::check(const MatchFinder::MatchResult &Result) { const auto *Member = dyn_cast<MemberExpr>(EndCall->getCallee()); if (!Member) return; - ContainerNeedsDereference = Member->isArrow(); + Descriptor.ContainerNeedsDereference = Member->isArrow(); } // We must know the container or an array length bound. @@ -696,8 +713,7 @@ void LoopConvertCheck::check(const MatchFinder::MatchResult &Result) { return; findAndVerifyUsages(Context, LoopVar, EndVar, ContainerExpr, BoundExpr, - ContainerNeedsDereference, DerefByValue, DerefByConstRef, - TheLoop, FixerKind); + TheLoop, FixerKind, Descriptor); } } // namespace modernize diff --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.h b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.h index b7590638d77..6773b1918e3 100644 --- a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.h @@ -25,22 +25,27 @@ public: void check(const ast_matchers::MatchFinder::MatchResult &Result) override; private: + struct RangeDescriptor { + bool ContainerNeedsDereference; + bool DerefByValue; + bool IsTriviallyCopyable; + bool DerefByConstRef; + }; + void doConversion(ASTContext *Context, const VarDecl *IndexVar, const VarDecl *MaybeContainer, StringRef ContainerString, const UsageResult &Usages, const DeclStmt *AliasDecl, bool AliasUseRequired, bool AliasFromForInit, - const ForStmt *TheLoop, bool ContainerNeedsDereference, - bool DerefByValue, bool DerefByConstRef); + const ForStmt *TheLoop, RangeDescriptor Descriptor); StringRef checkRejections(ASTContext *Context, const Expr *ContainerExpr, const ForStmt *TheLoop); void findAndVerifyUsages(ASTContext *Context, const VarDecl *LoopVar, const VarDecl *EndVar, const Expr *ContainerExpr, - const Expr *BoundExpr, - bool ContainerNeedsDereference, bool DerefByValue, - bool DerefByConstRef, const ForStmt *TheLoop, - LoopFixerKind FixerKind); + const Expr *BoundExpr, const ForStmt *TheLoop, + LoopFixerKind FixerKind, RangeDescriptor Descriptor); + std::unique_ptr<TUTrackingInfo> TUInfo; Confidence::Level MinConfidence; }; diff --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp index 116ec5c46c5..d0b2e8177a9 100644 --- a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp +++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp @@ -349,11 +349,13 @@ static bool isAliasDecl(ASTContext *Context, const Decl *TheDecl, // Check that the declared type is the same as (or a reference to) the // container type. + QualType InitType = Init->getType(); QualType DeclarationType = VDecl->getType(); - if (DeclarationType->isReferenceType()) + if (!DeclarationType.isNull() && DeclarationType->isReferenceType()) DeclarationType = DeclarationType.getNonReferenceType(); - QualType InitType = Init->getType(); - if (!Context->hasSameUnqualifiedType(DeclarationType, InitType)) + + if (InitType.isNull() || DeclarationType.isNull() || + !Context->hasSameUnqualifiedType(DeclarationType, InitType)) return false; switch (Init->getStmtClass()) { |
