diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp --- 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(); + + 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 diff --git a/clang/test/SemaCXX/warn-range-loop-analysis-trivially-copyable.cpp b/clang/test/SemaCXX/warn-range-loop-analysis-trivially-copyable.cpp new file mode 100644 --- /dev/null +++ b/clang/test/SemaCXX/warn-range-loop-analysis-trivially-copyable.cpp @@ -0,0 +1,89 @@ +// RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wloop-analysis -verify %s +// RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wrange-loop-analysis -verify %s + +void test_POD_64_bytes() { + struct Record { + char a[64]; + }; + + Record records[8]; + for (const auto r : records) + (void)r; +} + +void test_POD_65_bytes() { + struct Record { + char a[65]; + }; + + // expected-warning@+3 {{loop variable 'r' of type 'const Record' creates a copy from type 'const Record'}} + // expected-note@+2 {{use reference type 'const Record &' to prevent copying}} + Record records[8]; + for (const auto r : records) + (void)r; +} + +void test_TriviallyCopyable_64_bytes() { + struct Record { + Record() {} + char a[64]; + }; + + Record records[8]; + for (const auto r : records) + (void)r; +} + +void test_TriviallyCopyable_65_bytes() { + struct Record { + Record() {} + char a[65]; + }; + + // expected-warning@+3 {{loop variable 'r' of type 'const Record' creates a copy from type 'const Record'}} + // expected-note@+2 {{use reference type 'const Record &' to prevent copying}} + Record records[8]; + for (const auto r : records) + (void)r; +} + +void test_NonTriviallyCopyable() { + struct Record { + Record() {} + ~Record() {} + volatile int a; + int b; + }; + + // expected-warning@+3 {{loop variable 'r' of type 'const Record' creates a copy from type 'const Record'}} + // expected-note@+2 {{use reference type 'const Record &' to prevent copying}} + Record records[8]; + for (const auto r : records) + (void)r; +} + +void test_TrivialABI_64_bytes() { + struct [[clang::trivial_abi]] Record { + Record() {} + ~Record() {} + char a[64]; + }; + + Record records[8]; + for (const auto r : records) + (void)r; +} + +void test_TrivialABI_65_bytes() { + struct [[clang::trivial_abi]] Record { + Record() {} + ~Record() {} + char a[65]; + }; + + // expected-warning@+3 {{loop variable 'r' of type 'const Record' creates a copy from type 'const Record'}} + // expected-note@+2 {{use reference type 'const Record &' to prevent copying}} + Record records[8]; + for (const auto r : records) + (void)r; +} diff --git a/clang/test/SemaCXX/warn-range-loop-analysis.cpp b/clang/test/SemaCXX/warn-range-loop-analysis.cpp --- a/clang/test/SemaCXX/warn-range-loop-analysis.cpp +++ b/clang/test/SemaCXX/warn-range-loop-analysis.cpp @@ -20,6 +20,10 @@ struct Foo {}; struct Bar { + // Small trivially copyable types do not show a warning when copied in a + // range-based for loop. This size ensures the object is not considered + // small. + char s[128]; Bar(Foo); Bar(int); operator int();