Better source location for -Wignored-qualifiers on trailing return types

[aaronpuchert]

Summary

We collect the source location of a trailing return type in the parser,
improving the location for regular functions and providing a location
for lambdas, where previously there was none.

Fixes PR47732.

[aaronpuchert]

clang/lib/Sema/SemaType.cpp

3092–3094

I'm open to always using the RParenLoc.

[aaron.ballman]

clang/lib/Sema/SemaType.cpp

3092–3094

Any reason not to track the trailing return type location on the DeclaratorChunk::FunctionTypeInfo object? That already tracks whether the return type is trailing or not as well as a bunch of other source location information.

The right paren loc isn't necessarily a good place given the grammar allows a *lot* of optional stuff between the closing paren and the trailing return type: http://eel.is/c++draft/dcl.decl#general-5

[aaronpuchert]

Collect location of a trailing return type in the parser, use that for the warning.

I resisted clang-format's urge to reflow the large parameter lists, I hope that's the right thing here.

[aaron.ballman]

LGTM aside from a question about a potential assert to add.

clang/include/clang/Sema/DeclSpec.h

1509 ↗

(On Diff #301161)

Should we assert(HasTrailingReturnType);?

[aaronpuchert]

clang/include/clang/Sema/DeclSpec.h

1509 ↗

(On Diff #301161)

Makes sense to me. Ok if I add the assertion to this and the previous function for consistency?

[aaron.ballman]

clang/include/clang/Sema/DeclSpec.h

1509 ↗

(On Diff #301161)

Fine by me!

[aaronpuchert]

Added the assertions in a follow-up change rGebfc427bbe08f0c36af9721d5a4e6d3ffe2e4bf5.

clang/lib/Sema/SemaType.cpp

void Sema::diagnoseIgnoredQualifiers(unsigned DiagID, unsigned Quals,
   Diag(Loc.isInvalid() ? FallbackLoc : Loc, DiagID)
     << QualStr << NumQuals << FixIts[0] << FixIts[1] << FixIts[2] << FixIts[3];
 }
 
 // Diagnose pointless type qualifiers on the return type of a function.
 static void diagnoseRedundantReturnTypeQualifiers(Sema &S, QualType RetTy,
                                                   Declarator &D,
                                                   unsigned FunctionChunkIndex) {
-  if (D.getTypeObject(FunctionChunkIndex).Fun.hasTrailingReturnType()) {
+  const DeclaratorChunk::FunctionTypeInfo &FTI =
+      D.getTypeObject(FunctionChunkIndex).Fun;
+  if (FTI.hasTrailingReturnType()) {
     // FIXME: TypeSourceInfo doesn't preserve location information for
     // qualifiers.
+    SourceLocation Loc = D.getIdentifierLoc();
+    if (Loc.isInvalid())
+      Loc = FTI.getRParenLoc();
     S.diagnoseIgnoredQualifiers(diag::warn_qual_return_type,
-                                RetTy.getLocalCVRQualifiers(),
-                                D.getIdentifierLoc());
+                                RetTy.getLocalCVRQualifiers(), Loc);
     return;
   }
 
   for (unsigned OuterChunkIndex = FunctionChunkIndex + 1,
                 End = D.getNumTypeObjects();
        OuterChunkIndex != End; ++OuterChunkIndex) {
     DeclaratorChunk &OuterChunk = D.getTypeObject(OuterChunkIndex);
     switch (OuterChunk.Kind) {

clang/test/SemaCXX/return.cpp

 _Atomic // expected-warning {{'_Atomic' type qualifier on return type has no effect}}
     int
     atomic();
 
 auto
     trailing_return_type() -> // expected-warning {{'const' type qualifier on return type has no effect}}
     const int;
 
+auto trailing_return_type_lambda =
+    [](const int &x) -> // expected-warning {{'const' type qualifier on return type has no effect}}
+    const int { return x; };
+
 const int ret_array()[4]; // expected-error {{cannot return array}}
 }
 
 namespace PR9328 {
   typedef char *PCHAR;
   class Test 
   {
     const PCHAR GetName() { return 0; } // expected-warning{{'const' type qualifier on return type has no effect}}