Index: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.h =================================================================== --- clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.h +++ clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.h @@ -66,6 +66,7 @@ const ForStmt *Loop, LoopFixerKind FixerKind); std::unique_ptr TUInfo; + const unsigned long long MaxCopySize; const Confidence::Level MinConfidence; const VariableNamer::NamingStyle NamingStyle; }; Index: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp +++ clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp @@ -230,18 +230,18 @@ // 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::LoopConvertCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), TUInfo(new TUTrackingInfo), + MaxCopySize(std::stoull(Options.get("MaxCopySize", "16"))), MinConfidence(StringSwitch( Options.get("MinConfidence", "reasonable")) .Case("safe", Confidence::CL_Safe) @@ -422,6 +423,7 @@ .Default(VariableNamer::NS_CamelCase)) {} void LoopConvertCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "MaxCopySize", std::to_string(MaxCopySize)); SmallVector Confs{"risky", "reasonable", "safe"}; Options.store(Opts, "MinConfidence", Confs[static_cast(MinConfidence)]); @@ -561,18 +563,20 @@ // 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); Index: clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp +++ clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -47,6 +47,10 @@ 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". Index: clang-tools-extra/trunk/test/clang-tidy/Inputs/modernize-loop-convert/structures.h =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/Inputs/modernize-loop-convert/structures.h +++ clang-tools-extra/trunk/test/clang-tidy/Inputs/modernize-loop-convert/structures.h @@ -23,6 +23,11 @@ int X; }; +struct TriviallyCopyableButBig { + int X; + char Array[16]; +}; + struct S { typedef MutableVal *iterator; typedef const MutableVal *const_iterator; Index: clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp +++ clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp @@ -113,6 +113,14 @@ // 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)