[clang-tools-extra] r251694 - Only copy small types in modernize-loop-convert.
Angel Garcia Gomez via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 30 02:37:57 PDT 2015
Author: angelgarcia
Date: Fri Oct 30 04:37:57 2015
New Revision: 251694
URL: http://llvm.org/viewvc/llvm-project?rev=251694&view=rev
Log:
Only copy small types in modernize-loop-convert.
Summary: If the size of the type is above a certain bound, we'll take a const reference. This bound can be set as an option. For now, the default value is 16 bytes.
Reviewers: klimek
Subscribers: alexfh, cfe-commits
Differential Revision: http://reviews.llvm.org/D14176
Modified:
clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp
clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.h
clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp
clang-tools-extra/trunk/test/clang-tidy/Inputs/modernize-loop-convert/structures.h
clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp
Modified: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp?rev=251694&r1=251693&r2=251694&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp Fri Oct 30 04:37:57 2015
@@ -230,18 +230,18 @@ StatementMatcher makePseudoArrayLoopMatc
// 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::Range
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(Strin
.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);
Modified: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.h?rev=251694&r1=251693&r2=251694&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.h (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.h Fri Oct 30 04:37:57 2015
@@ -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;
};
Modified: clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp?rev=251694&r1=251693&r2=251694&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp Fri Oct 30 04:37:57 2015
@@ -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".
Modified: clang-tools-extra/trunk/test/clang-tidy/Inputs/modernize-loop-convert/structures.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/Inputs/modernize-loop-convert/structures.h?rev=251694&r1=251693&r2=251694&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/Inputs/modernize-loop-convert/structures.h (original)
+++ clang-tools-extra/trunk/test/clang-tidy/Inputs/modernize-loop-convert/structures.h Fri Oct 30 04:37:57 2015
@@ -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;
Modified: clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp?rev=251694&r1=251693&r2=251694&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp Fri Oct 30 04:37:57 2015
@@ -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)
More information about the cfe-commits
mailing list