[Clang] Fix false positive -Wignored-qualifiers

[cor3ntin]

A deduced return type can be an object type, in which case const can have an effect.
Delay the diagnostic to the point at which the type is deduced.
Add tests for lambdas.

Fixes #43054

Note that there is a discussion in #43054 about adding a separate warning for "const return types are weird" for the class type cases, but it would have to be a separate warning - warning which currently exists in clang-tidy as readability-const-return-type.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Corentin Jabot (cor3ntin)

Changes

A deduced return type can be an object type, in which case const can have an effect.
Delay the diagnostic to the point at which the type is deduced.
Add tests for lambdas.

Fixes #43054

Note that there is a discussion in #43054 about adding a separate warning for "const return types are weird" for the class type cases, but it would have to be a separate warning - warning which currently exists in clang-tidy as readability-const-return-type.

---

Full diff: https://github.com/llvm/llvm-project/pull/169664.diff

4 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+1)
• (modified) clang/lib/Sema/SemaStmt.cpp (+5)
• (modified) clang/lib/Sema/SemaType.cpp (+2-1)
• (modified) clang/test/SemaCXX/return.cpp (+25)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 8161ccdffca82..c43bf2c7ff99a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -392,6 +392,7 @@ Improvements to Clang's diagnostics
 - Fixed false positives in ``-Waddress-of-packed-member`` diagnostics when
   potential misaligned members get processed before they can get discarded.
   (#GH144729)
+- Fix a false positive warning in ``-Wignored-qualifiers`` when the return type is undeduced. (#GH43054)
 
 - Clang now emits a diagnostic with the correct message in case of assigning to const reference captured in lambda. (#GH105647)
 
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 655fa31bbf5c7..6bb1a27d1800c 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -3889,6 +3889,11 @@ bool Sema::DeduceFunctionTypeFromReturnExpr(FunctionDecl *FD,
     // Update all declarations of the function to have the deduced return type.
     Context.adjustDeducedFunctionResultType(FD, Deduced);
 
+  if (!Deduced->isDependentType() && !Deduced->isRecordType() &&
+      !FD->isFunctionTemplateSpecialization())
+    diagnoseIgnoredQualifiers(
+        diag::warn_qual_return_type,
+        FD->getDeclaredReturnType().getLocalCVRQualifiers(), FD->getLocation());
   return false;
 }
 
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index eb8b1352d1be1..3a8dd9df60827 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -5068,7 +5068,8 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
       // class type in C++.
       if ((T.getCVRQualifiers() || T->isAtomicType()) &&
           !(S.getLangOpts().CPlusPlus &&
-            (T->isDependentType() || T->isRecordType()))) {
+            (T->isDependentType() || T->isRecordType() ||
+             T->isUndeducedAutoType()))) {
         if (T->isVoidType() && !S.getLangOpts().CPlusPlus &&
             D.getFunctionDefinitionKind() ==
                 FunctionDefinitionKind::Definition) {
diff --git a/clang/test/SemaCXX/return.cpp b/clang/test/SemaCXX/return.cpp
index 796c9ae91dedc..92be66c24489e 100644
--- a/clang/test/SemaCXX/return.cpp
+++ b/clang/test/SemaCXX/return.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 %s -std=c++11 -fcxx-exceptions -fexceptions -fsyntax-only -Wignored-qualifiers -verify
+// RUN: %clang_cc1 %s -std=c++14 -fcxx-exceptions -fexceptions -fsyntax-only -Wignored-qualifiers -verify
 
 int test1() {
   throw;
@@ -132,3 +133,27 @@ void cxx_unresolved_expr() {
   // expr doesn't assert.
   return int(undeclared, 4; // expected-error {{use of undeclared identifier 'undeclared'}}
 }
+
+#if __cplusplus >= 201402L
+namespace GH43054 {
+struct S{};
+const auto foo() { return 0; } // expected-warning {{'const' type qualifier on return type has no effect}}
+const auto bar() { return S{}; }
+template <typename T>
+const auto baz() { return T{}; }
+
+void test() {
+  baz<int>();
+  baz<S>();
+
+  []() -> const auto { // expected-warning {{'const' type qualifier on return type has no effect}}
+    return 0;
+  }();
+
+  []() -> const auto {
+    return S{};
+  }();
+}
+}
+
+#endif

[erichkeane]

This looks fine, though I would love some better comments here.

[erichkeane]

Both of these checks are pretty subtle( in both places!) can we get a comment explaining what they are there for, and why it is the right list?