diff options
Diffstat (limited to 'clang-tools-extra')
5 files changed, 37 insertions, 15 deletions
diff --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp index 81aa0287293..81724fa7d0c 100644 --- a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp @@ -230,18 +230,18 @@ StatementMatcher makePseudoArrayLoopMatcher() { // FIXME: Also, a record doesn't necessarily need begin() and end(). Free // functions called begin() and end() taking the container as an argument // are also allowed. - TypeMatcher RecordWithBeginEnd = qualType( - anyOf(qualType(isConstQualified(), - hasDeclaration(cxxRecordDecl( - hasMethod(cxxMethodDecl(hasName("begin"), isConst())), - hasMethod(cxxMethodDecl(hasName("end"), - isConst())))) // hasDeclaration - ), // qualType - qualType(unless(isConstQualified()), - hasDeclaration( - cxxRecordDecl(hasMethod(hasName("begin")), + TypeMatcher RecordWithBeginEnd = qualType(anyOf( + qualType(isConstQualified(), + hasDeclaration(cxxRecordDecl( + hasMethod(cxxMethodDecl(hasName("begin"), isConst())), + hasMethod(cxxMethodDecl(hasName("end"), + isConst())))) // hasDeclaration + ), // qualType + qualType( + unless(isConstQualified()), + hasDeclaration(cxxRecordDecl(hasMethod(hasName("begin")), hasMethod(hasName("end"))))) // qualType - )); + )); StatementMatcher SizeCallMatcher = cxxMemberCallExpr( argumentCountIs(0), @@ -409,6 +409,7 @@ LoopConvertCheck::RangeDescriptor::RangeDescriptor() LoopConvertCheck::LoopConvertCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), TUInfo(new TUTrackingInfo), + MaxCopySize(std::stoull(Options.get("MaxCopySize", "16"))), MinConfidence(StringSwitch<Confidence::Level>( Options.get("MinConfidence", "reasonable")) .Case("safe", Confidence::CL_Safe) @@ -422,6 +423,7 @@ LoopConvertCheck::LoopConvertCheck(StringRef Name, ClangTidyContext *Context) .Default(VariableNamer::NS_CamelCase)) {} void LoopConvertCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "MaxCopySize", std::to_string(MaxCopySize)); SmallVector<std::string, 3> Confs{"risky", "reasonable", "safe"}; Options.store(Opts, "MinConfidence", Confs[static_cast<int>(MinConfidence)]); @@ -561,18 +563,20 @@ void LoopConvertCheck::doConversion( // If the new variable name is from the aliased variable, then the reference // type for the new variable should only be used if the aliased variable was // declared as a reference. - bool IsTriviallyCopyable = + bool IsCheapToCopy = !Descriptor.ElemType.isNull() && - Descriptor.ElemType.isTriviallyCopyableType(*Context); + Descriptor.ElemType.isTriviallyCopyableType(*Context) && + // TypeInfo::Width is in bits. + Context->getTypeInfo(Descriptor.ElemType).Width <= 8 * MaxCopySize; bool UseCopy = CanCopy && ((VarNameFromAlias && !AliasVarIsRef) || - (Descriptor.DerefByConstRef && IsTriviallyCopyable)); + (Descriptor.DerefByConstRef && IsCheapToCopy)); if (!UseCopy) { if (Descriptor.DerefByConstRef) { Type = Context->getLValueReferenceType(Context->getConstType(Type)); } else if (Descriptor.DerefByValue) { - if (!IsTriviallyCopyable) + if (!IsCheapToCopy) Type = Context->getRValueReferenceType(Type); } else { Type = Context->getLValueReferenceType(Type); diff --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.h b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.h index cf8ca19b7b1..b8288591ae0 100644 --- a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.h @@ -66,6 +66,7 @@ private: const ForStmt *Loop, LoopFixerKind FixerKind); std::unique_ptr<TUTrackingInfo> TUInfo; + const unsigned long long MaxCopySize; const Confidence::Level MinConfidence; const VariableNamer::NamingStyle NamingStyle; }; diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp index 924461ca331..01c111ecab0 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -47,6 +47,10 @@ public: ClangTidyOptions getModuleOptions() override { ClangTidyOptions Options; auto &Opts = Options.CheckOptions; + // For types whose size in bytes is above this threshold, we prefer taking a + // const-reference than making a copy. + Opts["modernize-loop-convert.MaxCopySize"] = "16"; + Opts["modernize-loop-convert.MinConfidence"] = "reasonable"; Opts["modernize-loop-convert.NamingStyle"] = "CamelCase"; Opts["modernize-pass-by-value.IncludeStyle"] = "llvm"; // Also: "google". diff --git a/clang-tools-extra/test/clang-tidy/Inputs/modernize-loop-convert/structures.h b/clang-tools-extra/test/clang-tidy/Inputs/modernize-loop-convert/structures.h index fbdbc8def9c..6eecae97ee4 100644 --- a/clang-tools-extra/test/clang-tidy/Inputs/modernize-loop-convert/structures.h +++ b/clang-tools-extra/test/clang-tidy/Inputs/modernize-loop-convert/structures.h @@ -23,6 +23,11 @@ struct NonTriviallyCopyable { int X; }; +struct TriviallyCopyableButBig { + int X; + char Array[16]; +}; + struct S { typedef MutableVal *iterator; typedef const MutableVal *const_iterator; diff --git a/clang-tools-extra/test/clang-tidy/modernize-loop-convert-basic.cpp b/clang-tools-extra/test/clang-tidy/modernize-loop-convert-basic.cpp index f98795f82f9..13a8226d18f 100644 --- a/clang-tools-extra/test/clang-tidy/modernize-loop-convert-basic.cpp +++ b/clang-tools-extra/test/clang-tidy/modernize-loop-convert-basic.cpp @@ -113,6 +113,14 @@ const int *constArray() { // CHECK-FIXES: for (const auto & Elem : NonCopy) // CHECK-FIXES-NEXT: printf("2 * %d = %d\n", Elem.X, Elem.X + Elem.X); + const TriviallyCopyableButBig Big[N]{}; + for (int I = 0; I < N; ++I) { + printf("2 * %d = %d\n", Big[I].X, Big[I].X + Big[I].X); + } + // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead + // CHECK-FIXES: for (const auto & Elem : Big) + // CHECK-FIXES-NEXT: printf("2 * %d = %d\n", Elem.X, Elem.X + Elem.X); + bool Something = false; for (int I = 0; I < N; ++I) { if (Something) |