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

3088–3090

I'm open to always using the RParenLoc.

[aaron.ballman]

clang/lib/Sema/SemaType.cpp

3088–3090

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

Should we assert(HasTrailingReturnType);?

[aaronpuchert]

clang/include/clang/Sema/DeclSpec.h

1509

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

Fine by me!

[aaronpuchert]

Added the assertions in a follow-up change rGebfc427bbe08f0c36af9721d5a4e6d3ffe2e4bf5.

clang/include/clang/Sema/DeclSpec.h

 //===--- DeclSpec.h - Parsed declaration specifiers -------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
union {
       /// C++.
       NamedDecl **DeclsInPrototype;
     };
 
     /// If HasTrailingReturnType is true, this is the trailing return
     /// type specified.
     UnionParsedType TrailingReturnType;
 
+    /// If HasTrailingReturnType is true, this is the location of the trailing
+    /// return type.
+    unsigned TrailingReturnTypeLoc;
+
     /// Reset the parameter list to having zero parameters.
     ///
     /// This is used in various places for error recovery.
     void freeParams() {
       for (unsigned I = 0; I < NumParams; ++I)
         Params[I].DefaultArgTokens.reset();
       if (DeleteParams) {
         delete[] Params;
struct FunctionTypeInfo {
     }
 
     /// Determine whether this function declarator had a
     /// trailing-return-type.
     bool hasTrailingReturnType() const { return HasTrailingReturnType; }
 
     /// Get the trailing-return-type for this function declarator.
     ParsedType getTrailingReturnType() const { return TrailingReturnType; }
+
+    /// Get the trailing-return-type location for this function declarator.
+    SourceLocation getTrailingReturnTypeLoc() const {
+      return SourceLocation::getFromRawEncoding(TrailingReturnTypeLoc);
+    }
   };
 
   struct BlockPointerTypeInfo {
     /// For now, sema will catch these as invalid.
     /// The type qualifiers: const/volatile/restrict/__unaligned/_Atomic.
     unsigned TypeQuals : 5;
 
     void destroy() {
static DeclaratorChunk getArray(unsigned TypeQuals,
     I.Arr.hasStatic = isStatic;
     I.Arr.isStar    = isStar;
     I.Arr.NumElts   = NumElts;
     return I;
   }
 
   /// DeclaratorChunk::getFunction - Return a DeclaratorChunk for a function.
   /// "TheDeclarator" is the declarator that this will be added to.
   static DeclaratorChunk getFunction(bool HasProto,
                                      bool IsAmbiguous,
                                      SourceLocation LParenLoc,
                                      ParamInfo *Params, unsigned NumParams,
                                      SourceLocation EllipsisLoc,
                                      SourceLocation RParenLoc,
                                      bool RefQualifierIsLvalueRef,
                                      SourceLocation RefQualifierLoc,
                                      SourceLocation MutableLoc,
                                      ExceptionSpecificationType ESpecType,
                                      SourceRange ESpecRange,
                                      ParsedType *Exceptions,
                                      SourceRange *ExceptionRanges,
                                      unsigned NumExceptions,
                                      Expr *NoexceptExpr,
                                      CachedTokens *ExceptionSpecTokens,
                                      ArrayRef<NamedDecl *> DeclsInPrototype,
                                      SourceLocation LocalRangeBegin,
                                      SourceLocation LocalRangeEnd,
                                      Declarator &TheDeclarator,
                                      TypeResult TrailingReturnType =
                                                     TypeResult(),
+                                     SourceLocation TrailingReturnTypeLoc =
+                                                    SourceLocation(),
                                      DeclSpec *MethodQualifiers = nullptr);
 
   /// Return a DeclaratorChunk for a block.
   static DeclaratorChunk getBlockPointer(unsigned TypeQuals,
                                          SourceLocation Loc) {
     DeclaratorChunk I;
     I.Kind          = BlockPointer;
     I.Loc           = Loc;

clang/lib/Parse/ParseDecl.cpp

 //===--- ParseDecl.cpp - Declaration Parsing --------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 //  This file implements the Declaration portions of the Parser interfaces.
void Parser::ParseFunctionDeclarator(Declarator &D,
   ExceptionSpecificationType ESpecType = EST_None;
   SourceRange ESpecRange;
   SmallVector<ParsedType, 2> DynamicExceptions;
   SmallVector<SourceRange, 2> DynamicExceptionRanges;
   ExprResult NoexceptExpr;
   CachedTokens *ExceptionSpecTokens = nullptr;
   ParsedAttributesWithRange FnAttrs(AttrFactory);
   TypeResult TrailingReturnType;
+  SourceLocation TrailingReturnTypeLoc;
 
   /* LocalEndLoc is the end location for the local FunctionTypeLoc.
      EndLoc is the end location for the function declarator.
      They differ for trailing return types. */
   SourceLocation StartLoc, LocalEndLoc, EndLoc;
   SourceLocation LParenLoc, RParenLoc;
   LParenLoc = Tracker.getOpenLocation();
   StartLoc = LParenLoc;
if (getLangOpts().CPlusPlus) {
       if (getLangOpts().CPlusPlus11 && Tok.is(tok::arrow)) {
         Diag(Tok, diag::warn_cxx98_compat_trailing_return_type);
         if (D.getDeclSpec().getTypeSpecType() == TST_auto)
           StartLoc = D.getDeclSpec().getTypeSpecTypeLoc();
         LocalEndLoc = Tok.getLocation();
         SourceRange Range;
         TrailingReturnType =
             ParseTrailingReturnType(Range, D.mayBeFollowedByCXXDirectInit());
+        TrailingReturnTypeLoc = Range.getBegin();
         EndLoc = Range.getEnd();
       }
     } else if (standardAttributesAllowed()) {
       MaybeParseCXX11Attributes(FnAttrs);
     }
   }
 
   // Collect non-parameter declarations from the prototype if this is a function
   // declaration. They will be moved into the scope of the function. Only do
   // this in C and not C++, where the decls will continue to live in the
   // surrounding context.
   SmallVector<NamedDecl *, 0> DeclsInPrototype;
   if (getCurScope()->getFlags() & Scope::FunctionDeclarationScope &&
       !getLangOpts().CPlusPlus) {
     for (Decl *D : getCurScope()->decls()) {
       NamedDecl *ND = dyn_cast<NamedDecl>(D);
       if (!ND || isa<ParmVarDecl>(ND))
         continue;
       DeclsInPrototype.push_back(ND);
     }
   }
 
   // Remember that we parsed a function type, and remember the attributes.
   D.AddTypeInfo(DeclaratorChunk::getFunction(
                     HasProto, IsAmbiguous, LParenLoc, ParamInfo.data(),
                     ParamInfo.size(), EllipsisLoc, RParenLoc,
                     RefQualifierIsLValueRef, RefQualifierLoc,
                     /*MutableLoc=*/SourceLocation(),
                     ESpecType, ESpecRange, DynamicExceptions.data(),
                     DynamicExceptionRanges.data(), DynamicExceptions.size(),
                     NoexceptExpr.isUsable() ? NoexceptExpr.get() : nullptr,
                     ExceptionSpecTokens, DeclsInPrototype, StartLoc,
-                    LocalEndLoc, D, TrailingReturnType, &DS),
+                    LocalEndLoc, D, TrailingReturnType, TrailingReturnTypeLoc,
+                    &DS),
                 std::move(FnAttrs), EndLoc);
 }
 
 /// ParseRefQualifier - Parses a member function ref-qualifier. Returns
 /// true if a ref-qualifier is found.
 bool Parser::ParseRefQualifier(bool &RefQualifierIsLValueRef,
                                SourceLocation &RefQualifierLoc) {
   if (Tok.isOneOf(tok::amp, tok::ampamp)) {

clang/lib/Parse/ParseDeclCXX.cpp

if (D.isFunctionDeclarator() && Tok.is(tok::arrow) &&
 
     if (!TrailingReturnType.isInvalid()) {
       Diag(ArrowLoc,
            diag::err_requires_clause_must_appear_after_trailing_return)
           << Range;
       auto &FunctionChunk = D.getFunctionTypeInfo();
       FunctionChunk.HasTrailingReturnType = TrailingReturnType.isUsable();
       FunctionChunk.TrailingReturnType = TrailingReturnType.get();
+      FunctionChunk.TrailingReturnTypeLoc = Range.getBegin().getRawEncoding();
     } else
       SkipUntil({tok::equal, tok::l_brace, tok::arrow, tok::kw_try, tok::comma},
                 StopAtSemi | StopBeforeMatch);
   }
 }
 
 /// We have just started parsing the definition of a new class,
 /// so push that class onto our stack of classes that is currently

clang/lib/Parse/ParseExprCXX.cpp

if (Tok.is(tok::less)) {
     } else {
       Actions.ActOnLambdaExplicitTemplateParameterList(
           LAngleLoc, TemplateParams, RAngleLoc);
       ++CurTemplateDepthTracker;
     }
   }
 
   TypeResult TrailingReturnType;
+  SourceLocation TrailingReturnTypeLoc;
   if (Tok.is(tok::l_paren)) {
     ParseScope PrototypeScope(this,
                               Scope::FunctionPrototypeScope |
                               Scope::FunctionDeclarationScope |
                               Scope::DeclScope);
 
     BalancedDelimiterTracker T(*this, tok::l_paren);
     T.consumeOpen();
if (Tok.is(tok::l_paren)) {
     SourceLocation FunLocalRangeEnd = DeclEndLoc;
 
     // Parse trailing-return-type[opt].
     if (Tok.is(tok::arrow)) {
       FunLocalRangeEnd = Tok.getLocation();
       SourceRange Range;
       TrailingReturnType =
           ParseTrailingReturnType(Range, /*MayBeFollowedByDirectInit*/ false);
+      TrailingReturnTypeLoc = Range.getBegin();
       if (Range.getEnd().isValid())
         DeclEndLoc = Range.getEnd();
     }
 
     SourceLocation NoLoc;
     D.AddTypeInfo(DeclaratorChunk::getFunction(
                       /*HasProto=*/true,
                       /*IsAmbiguous=*/false, LParenLoc, ParamInfo.data(),
                       ParamInfo.size(), EllipsisLoc, RParenLoc,
                       /*RefQualifierIsLvalueRef=*/true,
                       /*RefQualifierLoc=*/NoLoc, MutableLoc, ESpecType,
                       ESpecRange, DynamicExceptions.data(),
                       DynamicExceptionRanges.data(), DynamicExceptions.size(),
                       NoexceptExpr.isUsable() ? NoexceptExpr.get() : nullptr,
                       /*ExceptionSpecTokens*/ nullptr,
                       /*DeclsInPrototype=*/None, LParenLoc, FunLocalRangeEnd, D,
-                      TrailingReturnType, &DS),
+                      TrailingReturnType, TrailingReturnTypeLoc, &DS),
                   std::move(Attr), DeclEndLoc);
 
     // Parse requires-clause[opt].
     if (Tok.is(tok::kw_requires))
       ParseTrailingRequiresClause(D);
 
     PrototypeScope.Exit();
 

clang/lib/Sema/DeclSpec.cpp

 //===--- DeclSpec.cpp - Declaration Specifier Semantic Analysis -----------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 //  This file implements semantic analysis for declaration specifiers.
CXXScopeSpec::getWithLocInContext(ASTContext &Context) const {
   if (!Builder.getRepresentation())
     return NestedNameSpecifierLoc();
 
   return Builder.getWithLocInContext(Context);
 }
 
 /// DeclaratorChunk::getFunction - Return a DeclaratorChunk for a function.
 /// "TheDeclarator" is the declarator that this will be added to.
 DeclaratorChunk DeclaratorChunk::getFunction(bool hasProto,
                                              bool isAmbiguous,
                                              SourceLocation LParenLoc,
                                              ParamInfo *Params,
                                              unsigned NumParams,
                                              SourceLocation EllipsisLoc,
                                              SourceLocation RParenLoc,
                                              bool RefQualifierIsLvalueRef,
                                              SourceLocation RefQualifierLoc,
                                              SourceLocation MutableLoc,
                                              ExceptionSpecificationType
                                                  ESpecType,
                                              SourceRange ESpecRange,
                                              ParsedType *Exceptions,
                                              SourceRange *ExceptionRanges,
                                              unsigned NumExceptions,
                                              Expr *NoexceptExpr,
                                              CachedTokens *ExceptionSpecTokens,
                                              ArrayRef<NamedDecl*>
                                                  DeclsInPrototype,
                                              SourceLocation LocalRangeBegin,
                                              SourceLocation LocalRangeEnd,
                                              Declarator &TheDeclarator,
                                              TypeResult TrailingReturnType,
+                                             SourceLocation
+                                                 TrailingReturnTypeLoc,
                                              DeclSpec *MethodQualifiers) {
   assert(!(MethodQualifiers && MethodQualifiers->getTypeQualifiers() & DeclSpec::TQ_atomic) &&
          "function cannot have _Atomic qualifier");
 
   DeclaratorChunk I;
   I.Kind                        = Function;
   I.Loc                         = LocalRangeBegin;
   I.EndLoc                      = LocalRangeEnd;
DeclaratorChunk DeclaratorChunk::getFunction(bool hasProto,
   I.Fun.ExceptionSpecLocBeg     = ESpecRange.getBegin().getRawEncoding();
   I.Fun.ExceptionSpecLocEnd     = ESpecRange.getEnd().getRawEncoding();
   I.Fun.NumExceptionsOrDecls    = 0;
   I.Fun.Exceptions              = nullptr;
   I.Fun.NoexceptExpr            = nullptr;
   I.Fun.HasTrailingReturnType   = TrailingReturnType.isUsable() ||
                                   TrailingReturnType.isInvalid();
   I.Fun.TrailingReturnType      = TrailingReturnType.get();
+  I.Fun.TrailingReturnTypeLoc   = TrailingReturnTypeLoc.getRawEncoding();
   I.Fun.MethodQualifiers        = nullptr;
   I.Fun.QualAttrFactory         = nullptr;
 
   if (MethodQualifiers && (MethodQualifiers->getTypeQualifiers() ||
                            MethodQualifiers->getAttributes().size())) {
     auto &attrs = MethodQualifiers->getAttributes();
     I.Fun.MethodQualifiers = new DeclSpec(attrs.getPool().getFactory());
     MethodQualifiers->forEachCVRUQualifier(

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()) {
-    // FIXME: TypeSourceInfo doesn't preserve location information for
-    // qualifiers.
+  const DeclaratorChunk::FunctionTypeInfo &FTI =
+      D.getTypeObject(FunctionChunkIndex).Fun;
+  if (FTI.hasTrailingReturnType()) {
     S.diagnoseIgnoredQualifiers(diag::warn_qual_return_type,
                                 RetTy.getLocalCVRQualifiers(),
-                                D.getIdentifierLoc());
+                                FTI.getTrailingReturnTypeLoc());
     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

const int volatile // expected-warning {{'const volatile' type qualifiers on return type have no effect}}
     (((parens())));
 
 _Atomic(int) atomic();
 
 _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() ->
+    const int; // expected-warning {{'const' type qualifier on return type has no effect}}
+
+auto trailing_return_type_lambda = [](const int &x) ->
+    const int // expected-warning {{'const' type qualifier on return type has no effect}}
+    { return x; };
 
 const int ret_array()[4]; // expected-error {{cannot return array}}
 }
 
 namespace PR9328 {
   typedef char *PCHAR;
   class Test 
   {