[Sema] Improve -Wrange-loop-analysis warnings.
No longer generate a diagnostic when a small trivially copyable type is
used without a reference. Before the test looked for a POD type and had no
size restriction. Since the range-based for loop is only available in
C++11 and POD types are trivially copyable in C++11 it's not required to
test for a POD type.
Since copying a large object will be expensive its size has been
restricted. 64 bytes is a common size of a cache line and if the object is
aligned the copy will be cheap. No performance impact testing has been
done.
Differential Revision: https://reviews.llvm.org/D72212
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 1e7cc50..d6c3af9 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -2779,6 +2779,15 @@
}
}
+/// Determines whether the @p VariableType's declaration is a record with the
+/// clang::trivial_abi attribute.
+static bool hasTrivialABIAttr(QualType VariableType) {
+ if (CXXRecordDecl *RD = VariableType->getAsCXXRecordDecl())
+ return RD->hasAttr<TrivialABIAttr>();
+
+ return false;
+}
+
// Warns when the loop variable can be changed to a reference type to
// prevent a copy. For instance, if given "for (const Foo x : Range)" suggest
// "for (const Foo &x : Range)" if this form does not make a copy.
@@ -2800,10 +2809,13 @@
return;
}
- // TODO: Determine a maximum size that a POD type can be before a diagnostic
- // should be emitted. Also, only ignore POD types with trivial copy
- // constructors.
- if (VariableType.isPODType(SemaRef.Context))
+ // Small trivially copyable types are cheap to copy. Do not emit the
+ // diagnostic for these instances. 64 bytes is a common size of a cache line.
+ // (The function `getTypeSize` returns the size in bits.)
+ ASTContext &Ctx = SemaRef.Context;
+ if (Ctx.getTypeSize(VariableType) <= 64 * 8 &&
+ (VariableType.isTriviallyCopyableType(Ctx) ||
+ hasTrivialABIAttr(VariableType)))
return;
// Suggest changing from a const variable to a const reference variable