r212154 - Introduce a FunctionDecl::getReturnTypeSourceRange() utility
Alp Toker
alp at nuanti.com
Thu Jul 3 11:18:30 PDT 2014
On 03/07/2014 03:07, Richard Smith wrote:
> On Tue, Jul 1, 2014 at 6:47 PM, Alp Toker <alp at nuanti.com
> <mailto:alp at nuanti.com>> wrote:
>
> Author: alp
> Date: Tue Jul 1 20:47:15 2014
> New Revision: 212154
>
> URL: http://llvm.org/viewvc/llvm-project?rev=212154&view=rev
> Log:
> Introduce a FunctionDecl::getReturnTypeSourceRange() utility
>
> This source range is useful for all kinds of diagnostic QOI and
> refactoring
> work, so let's make it more discoverable.
>
> This commit also makes use of the new function to enhance various
> diagnostics
> relating to return types and resolves an old FIXME.
>
> Modified:
> cfe/trunk/include/clang/AST/Decl.h
> cfe/trunk/lib/AST/Decl.cpp
> cfe/trunk/lib/Sema/SemaDecl.cpp
> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>
> Modified: cfe/trunk/include/clang/AST/Decl.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=212154&r1=212153&r2=212154&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/AST/Decl.h (original)
> +++ cfe/trunk/include/clang/AST/Decl.h Tue Jul 1 20:47:15 2014
> @@ -1896,6 +1896,8 @@ public:
> return getType()->getAs<FunctionType>()->getReturnType();
> }
>
> + SourceRange getReturnTypeSourceRange() const;
> +
> /// \brief Determine the type of an expression that calls this
> function.
> QualType getCallResultType() const {
> return
> getType()->getAs<FunctionType>()->getCallResultType(getASTContext());
>
> Modified: cfe/trunk/lib/AST/Decl.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=212154&r1=212153&r2=212154&view=diff
> ==============================================================================
> --- cfe/trunk/lib/AST/Decl.cpp (original)
> +++ cfe/trunk/lib/AST/Decl.cpp Tue Jul 1 20:47:15 2014
> @@ -2687,6 +2687,23 @@ bool FunctionDecl::doesDeclarationForceE
> return FoundBody;
> }
>
> +SourceRange FunctionDecl::getReturnTypeSourceRange() const {
> + const TypeSourceInfo *TSI = getTypeSourceInfo();
> + if (!TSI)
> + return SourceRange();
> +
> + TypeLoc TL = TSI->getTypeLoc();
> + FunctionTypeLoc FunctionTL = TL.getAs<FunctionTypeLoc>();
>
>
> You should walk through ParenTypeLocs here.
Right, noticed that in follow-up commit r212174.
It's disappointing we don't store source ranges for return type qualifiers.
Alp.
> + if (!FunctionTL)
> + return SourceRange();
> +
> + TypeLoc ResultTL = FunctionTL.getReturnLoc();
> + if (ResultTL.getUnqualifiedLoc().getAs<BuiltinTypeLoc>())
> + return ResultTL.getSourceRange();
> +
> + return SourceRange();
> +}
> +
> /// \brief For an inline function definition in C, or for a
> gnu_inline function
> /// in C++, determine whether the definition will be externally
> visible.
> ///
>
> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=212154&r1=212153&r2=212154&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Jul 1 20:47:15 2014
> @@ -2519,11 +2519,13 @@ bool Sema::MergeFunctionDecl(FunctionDec
> ResQT = Context.mergeObjCGCQualifiers(NewQType, OldQType);
> if (ResQT.isNull()) {
> if (New->isCXXClassMember() && New->isOutOfLine())
> - Diag(New->getLocation(),
> - diag::err_member_def_does_not_match_ret_type) << New;
> + Diag(New->getLocation(),
> diag::err_member_def_does_not_match_ret_type)
> + << New << New->getReturnTypeSourceRange();
> else
> - Diag(New->getLocation(), diag::err_ovl_diff_return_type);
> - Diag(OldLocation, PrevDiag) << Old << Old->getType();
> + Diag(New->getLocation(), diag::err_ovl_diff_return_type)
> + << New->getReturnTypeSourceRange();
> + Diag(OldLocation, PrevDiag) << Old << Old->getType()
> + <<
> Old->getReturnTypeSourceRange();
> return true;
> }
> else
> @@ -7494,8 +7496,10 @@ Sema::ActOnFunctionDeclarator(Scope *S,
>
> // OpenCL v1.2, s6.9 -- Kernels can only have return type void.
> if (!NewFD->getReturnType()->isVoidType()) {
> - Diag(D.getIdentifierLoc(),
> - diag::err_expected_kernel_void_return_type);
> + SourceRange RTRange = NewFD->getReturnTypeSourceRange();
> + Diag(D.getIdentifierLoc(),
> diag::err_expected_kernel_void_return_type)
> + << (RTRange.isValid() ?
> FixItHint::CreateReplacement(RTRange, "void")
> + : FixItHint());
>
>
> This fixit is not correct; it will do horrible things to
>
> void __kernel (*f())[3];
>
> It seems pretty tricky to determine whether all the tokens in the
> range are part of the return type, but that's what you'd need here.
>
> It's probably not too bad to have incorrect fixits in corner cases on
> notes (because such fixits don't get automatically applied), but on an
> error we shouldn't really be producing incorrect fixits.
>
> (I see below that we were already getting this wrong in some cases, so
> this isn't really a significant regression, but it's still something
> we ought to fix.)
>
> Suggestion: Add a flag to this function to say whether the caller
> wants only a contiguous sequence of tokens. If that flag is set,
> inspect the TypeLoc of the return type to see if there might be
> intervening tokens, and return SourceRange() if so -- so "vector<int>
> f()" is ok, but "const vector<int> f()" is not, because we can't
> easily tell if there's an 'inline' keyword between the 'const' and the
> 'vector'. We can later fix this to give a valid range in more cases.
>
> Perhaps this contiguous-tokens functionality should live on TypeLoc?
> It seems like a reasonable thing for people to want.
>
> D.setInvalidType();
> }
>
> @@ -7835,23 +7839,6 @@ bool Sema::CheckFunctionDeclaration(Scop
> return Redeclaration;
> }
>
> -static SourceRange getResultSourceRange(const FunctionDecl *FD) {
> - const TypeSourceInfo *TSI = FD->getTypeSourceInfo();
> - if (!TSI)
> - return SourceRange();
> -
> - TypeLoc TL = TSI->getTypeLoc();
> - FunctionTypeLoc FunctionTL = TL.getAs<FunctionTypeLoc>();
> - if (!FunctionTL)
> - return SourceRange();
> -
> - TypeLoc ResultTL = FunctionTL.getReturnLoc();
> - if (ResultTL.getUnqualifiedLoc().getAs<BuiltinTypeLoc>())
> - return ResultTL.getSourceRange();
> -
> - return SourceRange();
> -}
> -
> void Sema::CheckMain(FunctionDecl* FD, const DeclSpec& DS) {
> // C++11 [basic.start.main]p3:
> // A program that [...] declares main to be inline, static or
> @@ -7904,19 +7891,17 @@ void Sema::CheckMain(FunctionDecl* FD, c
> } else if (getLangOpts().GNUMode && !getLangOpts().CPlusPlus) {
> Diag(FD->getTypeSpecStartLoc(), diag::ext_main_returns_nonint);
>
> - SourceRange ResultRange = getResultSourceRange(FD);
> - if (ResultRange.isValid())
> - Diag(ResultRange.getBegin(),
> diag::note_main_change_return_type)
> - << FixItHint::CreateReplacement(ResultRange, "int");
> + SourceRange RTRange = FD->getReturnTypeSourceRange();
> + if (RTRange.isValid())
> + Diag(RTRange.getBegin(), diag::note_main_change_return_type)
> + << FixItHint::CreateReplacement(RTRange, "int");
>
> // Otherwise, this is just a flat-out error.
> } else {
> - SourceRange ResultRange = getResultSourceRange(FD);
> - if (ResultRange.isValid())
> - Diag(FD->getTypeSpecStartLoc(), diag::err_main_returns_nonint)
> - << FixItHint::CreateReplacement(ResultRange, "int");
> - else
> - Diag(FD->getTypeSpecStartLoc(), diag::err_main_returns_nonint);
> + SourceRange RTRange = FD->getReturnTypeSourceRange();
> + Diag(FD->getTypeSpecStartLoc(), diag::err_main_returns_nonint)
> + << (RTRange.isValid() ?
> FixItHint::CreateReplacement(RTRange, "int")
> + : FixItHint());
>
> FD->setInvalidDecl(true);
> }
>
> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=212154&r1=212153&r2=212154&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Tue Jul 1 20:47:15 2014
> @@ -12304,8 +12304,10 @@ bool Sema::CheckOverridingFunctionReturn
> if (NewClassTy.isNull()) {
> Diag(New->getLocation(),
> diag::err_different_return_type_for_overriding_virtual_function)
> - << New->getDeclName() << NewTy << OldTy;
> - Diag(Old->getLocation(), diag::note_overridden_virtual_function);
> + << New->getDeclName() << NewTy << OldTy
> + << New->getReturnTypeSourceRange();
> + Diag(Old->getLocation(), diag::note_overridden_virtual_function)
> + << Old->getReturnTypeSourceRange();
>
> return true;
> }
> @@ -12325,25 +12327,27 @@ bool Sema::CheckOverridingFunctionReturn
> if (!Context.hasSameUnqualifiedType(NewClassTy, OldClassTy)) {
> // Check if the new class derives from the old class.
> if (!IsDerivedFrom(NewClassTy, OldClassTy)) {
> - Diag(New->getLocation(),
> - diag::err_covariant_return_not_derived)
> - << New->getDeclName() << NewTy << OldTy;
> - Diag(Old->getLocation(),
> diag::note_overridden_virtual_function);
> + Diag(New->getLocation(),
> diag::err_covariant_return_not_derived)
> + << New->getDeclName() << NewTy << OldTy
> + << New->getReturnTypeSourceRange();
> + Diag(Old->getLocation(),
> diag::note_overridden_virtual_function)
> + << Old->getReturnTypeSourceRange();
> return true;
> }
>
> // Check if we the conversion from derived to base is valid.
> - if (CheckDerivedToBaseConversion(NewClassTy, OldClassTy,
> - diag::err_covariant_return_inaccessible_base,
> - diag::err_covariant_return_ambiguous_derived_to_base_conv,
> - // FIXME: Should this point to the return type?
> - New->getLocation(), SourceRange(),
> New->getDeclName(),
> - nullptr)) {
> + if (CheckDerivedToBaseConversion(
> + NewClassTy, OldClassTy,
> + diag::err_covariant_return_inaccessible_base,
> + diag::err_covariant_return_ambiguous_derived_to_base_conv,
> + New->getLocation(), New->getReturnTypeSourceRange(),
> + New->getDeclName(), nullptr)) {
> // FIXME: this note won't trigger for delayed access control
> // diagnostics, and it's impossible to get an undelayed error
> // here from access control during the original parse because
> // the ParsingDeclSpec/ParsingDeclarator are still in scope.
> - Diag(Old->getLocation(),
> diag::note_overridden_virtual_function);
> + Diag(Old->getLocation(),
> diag::note_overridden_virtual_function)
> + << Old->getReturnTypeSourceRange();
> return true;
> }
> }
> @@ -12352,8 +12356,10 @@ bool Sema::CheckOverridingFunctionReturn
> if (NewTy.getLocalCVRQualifiers() !=
> OldTy.getLocalCVRQualifiers()) {
> Diag(New->getLocation(),
> diag::err_covariant_return_type_different_qualifications)
> - << New->getDeclName() << NewTy << OldTy;
> - Diag(Old->getLocation(), diag::note_overridden_virtual_function);
> + << New->getDeclName() << NewTy << OldTy
> + << New->getReturnTypeSourceRange();
> + Diag(Old->getLocation(), diag::note_overridden_virtual_function)
> + << Old->getReturnTypeSourceRange();
> return true;
> };
>
> @@ -12362,8 +12368,10 @@ bool Sema::CheckOverridingFunctionReturn
> if (NewClassTy.isMoreQualifiedThan(OldClassTy)) {
> Diag(New->getLocation(),
> diag::err_covariant_return_type_class_type_more_qualified)
> - << New->getDeclName() << NewTy << OldTy;
> - Diag(Old->getLocation(), diag::note_overridden_virtual_function);
> + << New->getDeclName() << NewTy << OldTy
> + << New->getReturnTypeSourceRange();
> + Diag(Old->getLocation(), diag::note_overridden_virtual_function)
> + << Old->getReturnTypeSourceRange();
> return true;
> };
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu <mailto:cfe-commits at cs.uiuc.edu>
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
--
http://www.nuanti.com
the browser experts
More information about the cfe-commits
mailing list