From 4dbf06c23f94160a2853466dada01e7b7eae7dad Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Fri, 19 Dec 2014 02:07:47 +0000 Subject: [PATCH] PR21969: Improve diagnostics for a conversion function that has any pieces of a declared return type (including a trailing-return-type in C++14). git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@224561 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Basic/DiagnosticSemaKinds.td | 8 +- include/clang/Sema/DeclSpec.h | 17 +++- lib/Parse/ParseDecl.cpp | 2 +- lib/Sema/SemaDeclCXX.cpp | 92 +++++++++++++++++++++- test/CXX/drs/dr3xx.cpp | 2 +- test/FixIt/fixit.cpp | 6 ++ test/SemaCXX/conversion-function.cpp | 10 +-- test/SemaCXX/cxx1y-deduced-return-type.cpp | 13 +++ 8 files changed, 136 insertions(+), 14 deletions(-) diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index a1b4b7dfa0..dadf3756d8 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -6451,7 +6451,13 @@ def err_conv_function_to_array : Error< def err_conv_function_to_function : Error< "conversion function cannot convert to a function type">; def err_conv_function_with_complex_decl : Error< - "must use a typedef to declare a conversion to %0">; + "cannot specify any part of a return type in the " + "declaration of a conversion function" + "%select{" + "; put the complete type after 'operator'|" + "; use a typedef to declare a conversion to %1|" + "; use an alias template to declare a conversion to %1|" + "}0">; def err_conv_function_redeclared : Error< "conversion function cannot be redeclared">; def warn_conv_to_self_not_used : Warning< diff --git a/include/clang/Sema/DeclSpec.h b/include/clang/Sema/DeclSpec.h index 43fcde1abf..d368826602 100644 --- a/include/clang/Sema/DeclSpec.h +++ b/include/clang/Sema/DeclSpec.h @@ -1070,6 +1070,12 @@ struct DeclaratorChunk { /// EndLoc - If valid, the place where this chunck ends. SourceLocation EndLoc; + SourceRange getSourceRange() const { + if (EndLoc.isInvalid()) + return SourceRange(Loc, Loc); + return SourceRange(Loc, EndLoc); + } + struct TypeInfoCommon { AttributeList *AttrList; }; @@ -1493,7 +1499,8 @@ struct DeclaratorChunk { SourceLocation Loc) { DeclaratorChunk I; I.Kind = MemberPointer; - I.Loc = Loc; + I.Loc = SS.getBeginLoc(); + I.EndLoc = Loc; I.Mem.TypeQuals = TypeQuals; I.Mem.AttrList = nullptr; new (I.Mem.ScopeMem.Mem) CXXScopeSpec(SS); @@ -1916,6 +1923,14 @@ public: return DeclTypeInfo[i]; } + typedef SmallVectorImpl::const_iterator type_object_iterator; + typedef llvm::iterator_range type_object_range; + + /// Returns the range of type objects, from the identifier outwards. + type_object_range type_objects() const { + return type_object_range(DeclTypeInfo.begin(), DeclTypeInfo.end()); + } + void DropFirstTypeObject() { assert(!DeclTypeInfo.empty() && "No type chunks to drop."); DeclTypeInfo.front().destroy(); diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp index 888e8eea25..64665fc287 100644 --- a/lib/Parse/ParseDecl.cpp +++ b/lib/Parse/ParseDecl.cpp @@ -4725,7 +4725,7 @@ void Parser::ParseDeclaratorInternal(Declarator &D, // Sema will have to catch (syntactically invalid) pointers into global // scope. It has to catch pointers into namespace scope anyway. D.AddTypeInfo(DeclaratorChunk::getMemberPointer(SS,DS.getTypeQualifiers(), - Loc), + DS.getLocEnd()), DS.getAttributes(), /* Don't replace range end. */SourceLocation()); return; diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index 96149c5e04..5737901ec6 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/lib/Sema/SemaDeclCXX.cpp @@ -6796,6 +6796,22 @@ QualType Sema::CheckDestructorDeclarator(Declarator &D, QualType R, return Context.getFunctionType(Context.VoidTy, None, EPI); } +static void extendLeft(SourceRange &R, const SourceRange &Before) { + if (Before.isInvalid()) + return; + R.setBegin(Before.getBegin()); + if (R.getEnd().isInvalid()) + R.setEnd(Before.getEnd()); +} + +static void extendRight(SourceRange &R, const SourceRange &After) { + if (After.isInvalid()) + return; + if (R.getBegin().isInvalid()) + R.setBegin(After.getBegin()); + R.setEnd(After.getEnd()); +} + /// CheckConversionDeclarator - Called by ActOnDeclarator to check the /// well-formednes of the conversion function declarator @p D with /// type @p R. If there are any errors in the declarator, this routine @@ -6817,7 +6833,9 @@ void Sema::CheckConversionDeclarator(Declarator &D, QualType &R, SC = SC_None; } - QualType ConvType = GetTypeFromParser(D.getName().ConversionFunctionId); + TypeSourceInfo *ConvTSI = nullptr; + QualType ConvType = + GetTypeFromParser(D.getName().ConversionFunctionId, &ConvTSI); if (D.getDeclSpec().hasTypeSpecifier() && !D.isInvalidType()) { // Conversion functions don't have return types, but the parser will @@ -6851,9 +6869,75 @@ void Sema::CheckConversionDeclarator(Declarator &D, QualType &R, // Diagnose "&operator bool()" and other such nonsense. This // is actually a gcc extension which we don't support. if (Proto->getReturnType() != ConvType) { - Diag(D.getIdentifierLoc(), diag::err_conv_function_with_complex_decl) - << Proto->getReturnType(); - D.setInvalidType(); + bool NeedsTypedef = false; + SourceRange Before, After; + + // Walk the chunks and extract information on them for our diagnostic. + bool PastFunctionChunk = false; + for (auto &Chunk : D.type_objects()) { + switch (Chunk.Kind) { + case DeclaratorChunk::Function: + if (!PastFunctionChunk) { + if (Chunk.Fun.HasTrailingReturnType) { + TypeSourceInfo *TRT = nullptr; + GetTypeFromParser(Chunk.Fun.getTrailingReturnType(), &TRT); + if (TRT) extendRight(After, TRT->getTypeLoc().getSourceRange()); + } + PastFunctionChunk = true; + break; + } + // Fall through. + case DeclaratorChunk::Array: + NeedsTypedef = true; + extendRight(After, Chunk.getSourceRange()); + break; + + case DeclaratorChunk::Pointer: + case DeclaratorChunk::BlockPointer: + case DeclaratorChunk::Reference: + case DeclaratorChunk::MemberPointer: + extendLeft(Before, Chunk.getSourceRange()); + break; + + case DeclaratorChunk::Paren: + extendLeft(Before, Chunk.Loc); + extendRight(After, Chunk.EndLoc); + break; + } + } + + SourceLocation Loc = Before.isValid() ? Before.getBegin() : + After.isValid() ? After.getBegin() : + D.getIdentifierLoc(); + auto &&DB = Diag(Loc, diag::err_conv_function_with_complex_decl); + DB << Before << After; + + if (!NeedsTypedef) { + DB << /*don't need a typedef*/0; + + // If we can provide a correct fix-it hint, do so. + if (After.isInvalid() && ConvTSI) { + SourceLocation InsertLoc = + PP.getLocForEndOfToken(ConvTSI->getTypeLoc().getLocEnd()); + DB << FixItHint::CreateInsertion(InsertLoc, " ") + << FixItHint::CreateInsertionFromRange( + InsertLoc, CharSourceRange::getTokenRange(Before)) + << FixItHint::CreateRemoval(Before); + } + } else if (!Proto->getReturnType()->isDependentType()) { + DB << /*typedef*/1 << Proto->getReturnType(); + } else if (getLangOpts().CPlusPlus11) { + DB << /*alias template*/2 << Proto->getReturnType(); + } else { + DB << /*might not be fixable*/3; + } + + // Recover by incorporating the other type chunks into the result type. + // Note, this does *not* change the name of the function. This is compatible + // with the GCC extension: + // struct S { &operator int(); } s; + // int &r = s.operator int(); // ok in GCC + // S::operator int&() {} // error in GCC, function name is 'operator int'. ConvType = Proto->getReturnType(); } diff --git a/test/CXX/drs/dr3xx.cpp b/test/CXX/drs/dr3xx.cpp index 62cf21494a..cea4d64e7e 100644 --- a/test/CXX/drs/dr3xx.cpp +++ b/test/CXX/drs/dr3xx.cpp @@ -1219,7 +1219,7 @@ namespace dr391 { // dr391: yes c++11 namespace dr395 { // dr395: yes struct S { - template (&operator T())[N]; // expected-error {{must use a typedef}} + template (&operator T())[N]; // expected-error {{cannot specify any part of a return type}} template operator(T (&)[N])(); // expected-error {{expected ')'}} expected-note {{to match this '('}} expected-error +{{}} template operator T *() const { return 0; } template operator T U::*() const { return 0; } diff --git a/test/FixIt/fixit.cpp b/test/FixIt/fixit.cpp index 20b5b52c11..585c216f90 100644 --- a/test/FixIt/fixit.cpp +++ b/test/FixIt/fixit.cpp @@ -381,3 +381,9 @@ struct H : A // expected-error{{expected '{' after base class list}} }; #endif } + +struct conversion_operator { + conversion_operator::* const operator int(); // expected-error {{put the complete type after 'operator'}} + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:32}:"" + // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:44-[[@LINE-2]]:44}:" conversion_operator::* const" +}; diff --git a/test/SemaCXX/conversion-function.cpp b/test/SemaCXX/conversion-function.cpp index ede23a2767..077e4d9f3c 100644 --- a/test/SemaCXX/conversion-function.cpp +++ b/test/SemaCXX/conversion-function.cpp @@ -172,12 +172,10 @@ namespace source_locations { namespace crazy_declarators { struct A { - (&operator bool())(); // expected-error {{must use a typedef to declare a conversion to 'bool (&)()'}} - - // FIXME: This diagnostic is misleading (the correct spelling - // would be 'operator int*'), but it's a corner case of a - // rarely-used syntax extension. - *operator int(); // expected-error {{must use a typedef to declare a conversion to 'int *'}} + (&operator bool())(); // expected-error {{use a typedef to declare a conversion to 'bool (&)()'}} + *operator int(); // expected-error {{put the complete type after 'operator'}} + // No suggestion of using a typedef here; that's not possible. + template (&operator T())(); // expected-error-re {{cannot specify any part of a return type in the declaration of a conversion function{{$}}}} }; } diff --git a/test/SemaCXX/cxx1y-deduced-return-type.cpp b/test/SemaCXX/cxx1y-deduced-return-type.cpp index 6086416595..50e0cf79c5 100644 --- a/test/SemaCXX/cxx1y-deduced-return-type.cpp +++ b/test/SemaCXX/cxx1y-deduced-return-type.cpp @@ -483,3 +483,16 @@ namespace OverloadedOperators { int g = a - a; } } + +namespace TrailingReturnTypeForConversionOperator { + struct X { + operator auto() -> int { return 0; } // expected-error {{cannot specify any part of a return type in the declaration of a conversion function; put the complete type after 'operator'}} + } x; + int k = x.operator auto(); + + struct Y { + operator auto() -> int & { // expected-error {{cannot specify}} + return 0; // expected-error {{cannot bind to}} + } + }; +}; -- 2.40.0