nlee created this revision.
nlee added a reviewer: lattner.
Herald added a project: All.
nlee requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Returning class or struct by value with const qualifier should be avoided since
it prevents move semantics.
clang-tidy has readability-const-return-type that catches this case, but it
also impacts move semantics, not just readability.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D125402
Files:
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaType.cpp
clang/test/Misc/warning-wall.c
clang/test/SemaCXX/warn-return-by-const-value.cpp
Index: clang/test/SemaCXX/warn-return-by-const-value.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/warn-return-by-const-value.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -fsyntax-only -Wreturn-by-const-value -Wignored-qualifiers -std=c++11 -verify %s
+
+struct S{} s;
+
+const S f1() // expected-warning {{returning by const value prevents move semantics}}
+{
+ return s;
+}
+
+// warn only on top-level const
+const S& f2()
+{
+ return s;
+}
+
+// same as f2()
+const S* f3()
+{
+ return &s;
+}
+
+// top-level const on primitive type(e.g. pointer) is handled by -Wignored-qualifiers
+S* const f4() // expected-warning {{'const' type qualifier on return type has no effect}}
+{
+ return &s;
+}
+
+// same as f4()
+const S* const f5() // expected-warning {{'const' type qualifier on return type has no effect}}
+{
+ return &s;
+}
\ No newline at end of file
Index: clang/test/Misc/warning-wall.c
===================================================================
--- clang/test/Misc/warning-wall.c
+++ clang/test/Misc/warning-wall.c
@@ -37,6 +37,7 @@
CHECK-NEXT: -Wreorder
CHECK-NEXT: -Wreorder-ctor
CHECK-NEXT: -Wreorder-init-list
+CHECK-NEXT: -Wreturn-by-const-value
CHECK-NEXT: -Wreturn-type
CHECK-NEXT: -Wreturn-type-c-linkage
CHECK-NEXT: -Wself-assign
Index: clang/lib/Sema/SemaType.cpp
===================================================================
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -5197,6 +5197,15 @@
S.Diag(DeclType.Loc, diag::warn_deprecated_volatile_return) << T;
}
+ // const qualifier on by-value return type prevents move semantics
+ if (S.getLangOpts().CPlusPlus11 &&
+ T.isConstQualified() &&
+ T->isStructureOrClassType()) {
+ const SourceLocation ConstLoc = D.getDeclSpec().getConstSpecLoc();
+ S.Diag(ConstLoc, diag::warn_return_by_const_value)
+ << T << FixItHint::CreateRemoval(ConstLoc);
+ }
+
// Objective-C ARC ownership qualifiers are ignored on the function
// return type (by type canonicalization). Complain if this attribute
// was written here.
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6618,6 +6618,10 @@
InGroup<PessimizingMove>, DefaultIgnore;
def note_remove_move : Note<"remove std::move call here">;
+def warn_return_by_const_value : Warning<
+ "returning by const value prevents move semantics">,
+ InGroup<ReturnByConstValue>, DefaultIgnore;
+
def warn_string_plus_int : Warning<
"adding %0 to a string does not append to the string">,
InGroup<StringPlusInt>;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===================================================================
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -566,6 +566,7 @@
def GNURedeclaredEnum : DiagGroup<"gnu-redeclared-enum">;
def RedundantMove : DiagGroup<"redundant-move">;
def Register : DiagGroup<"register", [DeprecatedRegister]>;
+def ReturnByConstValue : DiagGroup<"return-by-const-value">;
def ReturnTypeCLinkage : DiagGroup<"return-type-c-linkage">;
def ReturnType : DiagGroup<"return-type", [ReturnTypeCLinkage]>;
def BindToTemporaryCopy : DiagGroup<"bind-to-temporary-copy",
@@ -987,6 +988,7 @@
MultiChar,
RangeLoopConstruct,
Reorder,
+ ReturnByConstValue,
ReturnType,
SelfAssignment,
SelfMove,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
