[clang] Diagnose const string f() under -Wqual-class-return-type

If this is too noisy, you can turn it off with `-Wno-qual-class-return-type`, which, despite appearances, is independent of `-Wqual-return-type`. The latter continues to control only qualified *non-class* return types.
Quuxplusone · Oct 9, 2024 · 37913b7 · 37913b7
1 parent 03ecd7e
commit 37913b7
Show file tree

Hide file tree

Showing 11 changed files with 27 additions and 14 deletions.
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -476,7 +476,8 @@ def PureVirtualCallFromCtorDtor: DiagGroup<"call-to-pure-virtual-from-ctor-dtor"
 def GNUImaginaryConstant : DiagGroup<"gnu-imaginary-constant">;
 def IgnoredGCH : DiagGroup<"ignored-gch">;
 def IgnoredReferenceQualifiers : DiagGroup<"ignored-reference-qualifiers">;
-def IgnoredQualifiers : DiagGroup<"ignored-qualifiers", [IgnoredReferenceQualifiers]>;
+def QualClassReturnType : DiagGroup<"qual-class-return-type">;
+def IgnoredQualifiers : DiagGroup<"ignored-qualifiers", [IgnoredReferenceQualifiers, QualClassReturnType]>;
 def : DiagGroup<"import">;
 def GNUIncludeNext : DiagGroup<"gnu-include-next">;
 def IncompatibleMSStruct : DiagGroup<"incompatible-ms-struct">;

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -482,6 +482,9 @@ def err_noreturn_non_function : Error<
 def warn_qual_return_type : Warning<
   "'%0' type qualifier%s1 on return type %plural{1:has|:have}1 no effect">,
   InGroup<IgnoredQualifiers>, DefaultIgnore;
+def warn_qual_class_return_type : Warning<
+  "'%0' type qualifier%s1 on return type %plural{1:is|:are}1 a bad idea">,
+  InGroup<QualClassReturnType>, DefaultIgnore;
 def warn_deprecated_redundant_constexpr_static_def : Warning<
   "out-of-line definition of constexpr static data member is redundant "
   "in C++17 and is deprecated">,

diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
@@ -2915,13 +2915,13 @@ void Sema::diagnoseIgnoredQualifiers(unsigned DiagID, unsigned Quals,
 }
 
 // Diagnose pointless type qualifiers on the return type of a function.
-static void diagnoseRedundantReturnTypeQualifiers(Sema &S, QualType RetTy,
+static void diagnoseRedundantReturnTypeQualifiers(unsigned DiagID, Sema &S, QualType RetTy,
                                                   Declarator &D,
                                                   unsigned FunctionChunkIndex) {
   const DeclaratorChunk::FunctionTypeInfo &FTI =
       D.getTypeObject(FunctionChunkIndex).Fun;
   if (FTI.hasTrailingReturnType()) {
-    S.diagnoseIgnoredQualifiers(diag::warn_qual_return_type,
+    S.diagnoseIgnoredQualifiers(DiagID,
                                 RetTy.getLocalCVRQualifiers(),
                                 FTI.getTrailingReturnTypeLoc());
     return;
@@ -2938,7 +2938,7 @@ static void diagnoseRedundantReturnTypeQualifiers(Sema &S, QualType RetTy,
     case DeclaratorChunk::Pointer: {
       DeclaratorChunk::PointerTypeInfo &PTI = OuterChunk.Ptr;
       S.diagnoseIgnoredQualifiers(
-          diag::warn_qual_return_type,
+          DiagID,
           PTI.TypeQuals,
           SourceLocation(),
           PTI.ConstQualLoc,
@@ -2958,7 +2958,7 @@ static void diagnoseRedundantReturnTypeQualifiers(Sema &S, QualType RetTy,
       // FIXME: We can't currently provide an accurate source location and a
       // fix-it hint for these.
       unsigned AtomicQual = RetTy->isAtomicType() ? DeclSpec::TQ_atomic : 0;
-      S.diagnoseIgnoredQualifiers(diag::warn_qual_return_type,
+      S.diagnoseIgnoredQualifiers(DiagID,
                                   RetTy.getCVRQualifiers() | AtomicQual,
                                   D.getIdentifierLoc());
       return;
@@ -2975,7 +2975,7 @@ static void diagnoseRedundantReturnTypeQualifiers(Sema &S, QualType RetTy,
 
   // Just parens all the way out to the decl specifiers. Diagnose any qualifiers
   // which are present there.
-  S.diagnoseIgnoredQualifiers(diag::warn_qual_return_type,
+  S.diagnoseIgnoredQualifiers(DiagID,
                               D.getDeclSpec().getTypeQualifiers(),
                               D.getIdentifierLoc(),
                               D.getDeclSpec().getConstSpecLoc(),
@@ -5006,18 +5006,19 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
 
       // cv-qualifiers on return types are pointless except when the type is a
       // class type in C++.
-      if ((T.getCVRQualifiers() || T->isAtomicType()) &&
-          !(S.getLangOpts().CPlusPlus &&
-            (T->isDependentType() || T->isRecordType()))) {
-        if (T->isVoidType() && !S.getLangOpts().CPlusPlus &&
+      if (T.getCVRQualifiers() || T->isAtomicType()) {
+        if (S.getLangOpts().CPlusPlus && (T->isDependentType() || T->isRecordType())) {
+          // Diagnose with a different diagnostic.
+          diagnoseRedundantReturnTypeQualifiers(diag::warn_qual_class_return_type, S, T, D, chunkIndex);
+        } else if (T->isVoidType() && !S.getLangOpts().CPlusPlus &&
             D.getFunctionDefinitionKind() ==
                 FunctionDefinitionKind::Definition) {
           // [6.9.1/3] qualified void return is invalid on a C
           // function definition.  Apparently ok on declarations and
           // in C++ though (!)
           S.Diag(DeclType.Loc, diag::err_func_returning_qualified_void) << T;
         } else
-          diagnoseRedundantReturnTypeQualifiers(S, T, D, chunkIndex);
+          diagnoseRedundantReturnTypeQualifiers(diag::warn_qual_return_type, S, T, D, chunkIndex);
 
       }
 

diff --git a/clang/test/SemaCXX/return.cpp b/clang/test/SemaCXX/return.cpp
@@ -20,8 +20,8 @@ T h() {
 // Don't warn on cv-qualified class return types, only scalar return types.
 namespace ignored_quals {
 struct S {};
-const S class_c();
-const volatile S class_cv();
+const S class_c(); // expected-warning{{'const' type qualifier on return type is a bad idea}}
+const volatile S class_cv(); // expected-warning{{'const volatile' type qualifiers on return type are a bad idea}}
 
 const int scalar_c(); // expected-warning{{'const' type qualifier on return type has no effect}}
 int const scalar_c2(); // expected-warning{{'const' type qualifier on return type has no effect}}

diff --git a/libcxx/test/std/ranges/range.adaptors/range.lazy.split/types.h b/libcxx/test/std/ranges/range.adaptors/range.lazy.split/types.h
@@ -102,7 +102,7 @@ class almost_forward_iterator {
 
     constexpr almost_forward_iterator& operator++() { ++it_; return *this; }
     // Notice the slightly different return type.
-    constexpr const almost_forward_iterator operator++(int) { return almost_forward_iterator(it_); }
+    constexpr void operator++(int) { ++it_; }
 
     friend constexpr bool operator==(const almost_forward_iterator& x, const almost_forward_iterator& y) {
       return x.it_ == y.it_;

diff --git a/libcxx/test/std/utilities/function.objects/func.invoke/invoke.pass.cpp b/libcxx/test/std/utilities/function.objects/func.invoke/invoke.pass.cpp
@@ -10,6 +10,7 @@
 
 // <functional>
 
+// ADDITIONAL_COMPILE_FLAGS(clang): -Wno-qual-class-return-type
 // ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_ENABLE_CXX20_REMOVED_TYPE_TRAITS
 // ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_DISABLE_DEPRECATION_WARNINGS
 

diff --git a/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.elem/get_const_rv.verify.cpp b/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.elem/get_const_rv.verify.cpp
@@ -6,6 +6,8 @@
 //
 //===----------------------------------------------------------------------===//
 
+// ADDITIONAL_COMPILE_FLAGS(clang): -Wno-qual-class-return-type
+
 // <tuple>
 
 // template <class... Types> class tuple;

diff --git a/libcxx/test/std/utilities/utility/forward/forward.pass.cpp b/libcxx/test/std/utilities/utility/forward/forward.pass.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 // UNSUPPORTED: c++03
+// ADDITIONAL_COMPILE_FLAGS(clang): -Wno-qual-class-return-type
 
 // test forward
 

diff --git a/libcxx/test/std/utilities/utility/forward/forward.verify.cpp b/libcxx/test/std/utilities/utility/forward/forward.verify.cpp
@@ -18,6 +18,7 @@ struct A
 
 A source() {return A();}
 const A csource() {return A();}
+  // expected-warning@-1{{'const' type qualifier on return type is a bad idea}}
 
 int main(int, char**)
 {

diff --git a/libcxx/test/std/utilities/utility/forward/move.pass.cpp b/libcxx/test/std/utilities/utility/forward/move.pass.cpp
@@ -6,6 +6,8 @@
 //
 //===----------------------------------------------------------------------===//
 
+// ADDITIONAL_COMPILE_FLAGS(clang): -Wno-qual-class-return-type
+
 // test move
 
 #include <utility>

diff --git a/libcxx/test/std/utilities/utility/forward/move.verify.cpp b/libcxx/test/std/utilities/utility/forward/move.verify.cpp
@@ -11,6 +11,7 @@
 // defaulted using extensions.
 
 // XFAIL: c++03
+// ADDITIONAL_COMPILE_FLAGS(clang): -Wno-qual-class-return-type
 
 // test move