r188900 - Sema: Use the right type for PredefinedExpr when it's in a lambda.
Benjamin Kramer
benny.kra at gmail.com
Sun Aug 25 11:33:27 PDT 2013
On 21.08.2013, at 20:33, Richard Smith <richard at metafoo.co.uk> wrote:
> On Wed, Aug 21, 2013 at 4:45 AM, Benjamin Kramer <benny.kra at googlemail.com> wrote:
> Author: d0k
> Date: Wed Aug 21 06:45:27 2013
> New Revision: 188900
>
> URL: http://llvm.org/viewvc/llvm-project?rev=188900&view=rev
> Log:
> Sema: Use the right type for PredefinedExpr when it's in a lambda.
>
> 1. We now print the return type of lambdas and return type deduced functions
> as "auto". Trailing return types with decltype print the underlying type.
> 2. Use the lambda or block scope for the PredefinedExpr type instead of the
> parent function. This fixes PR16946, a strange mismatch between type of the
> expression and the actual result.
> 3. Verify the type in CodeGen.
> 4. The type for blocks is still wrong. They are numbered and the name is not
> known until CodeGen.
>
> Added:
> cfe/trunk/test/SemaCXX/predefined-expr.cpp
> Modified:
> cfe/trunk/lib/AST/Expr.cpp
> cfe/trunk/lib/CodeGen/CGExpr.cpp
> cfe/trunk/lib/Sema/SemaExpr.cpp
>
> Modified: cfe/trunk/lib/AST/Expr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=188900&r1=188899&r2=188900&view=diff
> ==============================================================================
> --- cfe/trunk/lib/AST/Expr.cpp (original)
> +++ cfe/trunk/lib/AST/Expr.cpp Wed Aug 21 06:45:27 2013
> @@ -585,7 +585,18 @@ std::string PredefinedExpr::ComputeName(
>
> POut.flush();
>
> - if (!isa<CXXConstructorDecl>(FD) && !isa<CXXDestructorDecl>(FD))
> + // Print "auto" for all deduced return types. This includes C++1y return
> + // type deduction and lambdas. For trailing return types resolve the
> + // decltype expression. Otherwise print the real type when this is
> + // not a constructor or destructor.
> + if ((isa<CXXMethodDecl>(FD) &&
> + cast<CXXMethodDecl>(FD)->getParent()->isLambda()) ||
> + (FT && FT->getResultType()->getAs<AutoType>()))
> + Proto = "auto " + Proto;
>
> This looks wrong in a couple of ways:
>
> * For a lambda with an explicitly-specified return type, we'll print 'auto' anyway
> * For a function with a return type of, say, "auto *", we'll not hit this special case
>
> Instead, you should use the declared return type (the type as written, from the TypeSourceInfo). It looks like we'll unhelpfully put 'DependentTy' there if no return type is specified; perhaps we should put an undeduced 'auto' type there instead.
Avoiding <dependent type> printing is the whole point of this strange code. I think using the type from TypeSourceInfo also added parentheses in places where they don't belong. Any ideas on how to fix it?
auto* just crashes :(
> + else if (FT && FT->getResultType()->getAs<DecltypeType>())
> + FT->getResultType()->getAs<DecltypeType>()->getUnderlyingType()
> + .getAsStringInternal(Proto, Policy);
>
> Why perform this partial desugaring?
I tried to get the same output GCC has for __PRETTY_FUNCTION__ here, but maybe emitting the decltype expression as written would be better?
> + else if (!isa<CXXConstructorDecl>(FD) && !isa<CXXDestructorDecl>(FD))
>
> What about conversion operators? They also have no declared return type. (And 'operator auto()' in C++14 is another case we need to handle carefully.)
We currently print "auto Class::operator auto()", that should be fixed.
>
> AFT->getResultType().getAsStringInternal(Proto, Policy);
>
> Out << Proto;
>
> Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=188900&r1=188899&r2=188900&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGExpr.cpp Wed Aug 21 06:45:27 2013
> @@ -1937,7 +1937,7 @@ LValue CodeGenFunction::EmitPredefinedLV
> case PredefinedExpr::Function:
> case PredefinedExpr::LFunction:
> case PredefinedExpr::PrettyFunction: {
> - unsigned IdentType = E->getIdentType();
> + PredefinedExpr::IdentType IdentType = E->getIdentType();
> std::string GlobalVarName;
>
> switch (IdentType) {
> @@ -1961,17 +1961,24 @@ LValue CodeGenFunction::EmitPredefinedLV
> FnName = FnName.substr(1);
> GlobalVarName += FnName;
>
> + // If this is outside of a function use the top level decl.
> const Decl *CurDecl = CurCodeDecl;
> - if (CurDecl == 0)
> + if (CurDecl == 0 || isa<VarDecl>(CurDecl))
> CurDecl = getContext().getTranslationUnitDecl();
>
> - std::string FunctionName =
> - (isa<BlockDecl>(CurDecl)
> - ? FnName.str()
> - : PredefinedExpr::ComputeName((PredefinedExpr::IdentType)IdentType,
> - CurDecl));
> + const Type *ElemType = E->getType()->getArrayElementTypeNoTypeQual();
> + std::string FunctionName;
> + if (isa<BlockDecl>(CurDecl)) {
> + // Blocks use the mangled function name.
> + // FIXME: ComputeName should handle blocks.
> + FunctionName = FnName.str();
> + } else {
> + FunctionName = PredefinedExpr::ComputeName(IdentType, CurDecl);
> + assert(cast<ConstantArrayType>(E->getType())->getSize() - 1 ==
> + FunctionName.size() &&
> + "Computed __func__ length differs from type!");
> + }
>
> - const Type* ElemType = E->getType()->getArrayElementTypeNoTypeQual();
> llvm::Constant *C;
> if (ElemType->isWideCharType()) {
> SmallString<32> RawChars;
>
> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=188900&r1=188899&r2=188900&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Wed Aug 21 06:45:27 2013
> @@ -2759,14 +2759,14 @@ ExprResult Sema::ActOnPredefinedExpr(Sou
> // Pre-defined identifiers are of type char[x], where x is the length of the
> // string.
>
> - Decl *currentDecl = getCurFunctionOrMethodDecl();
> - // Blocks and lambdas can occur at global scope. Don't emit a warning.
> - if (!currentDecl) {
> - if (const BlockScopeInfo *BSI = getCurBlock())
> - currentDecl = BSI->TheDecl;
> - else if (const LambdaScopeInfo *LSI = getCurLambda())
> - currentDecl = LSI->CallOperator;
> - }
> + // Pick the current block, lambda or function.
> + Decl *currentDecl;
> + if (const BlockScopeInfo *BSI = getCurBlock())
> + currentDecl = BSI->TheDecl;
> + else if (const LambdaScopeInfo *LSI = getCurLambda())
> + currentDecl = LSI->CallOperator;
> + else
> + currentDecl = getCurFunctionOrMethodDecl();
>
> I don't think this correctly handles the case where FunctionScopes.back() is an SK_CapturedRegion (inside, say, a lambda).
Wei Pan caught this in http://llvm-reviews.chandlerc.com/D1491 . What we used to emit was just wrong and with this patch it hit the assert I put into CodeGen.
- Ben
>
>
> if (!currentDecl) {
> Diag(Loc, diag::ext_predef_outside_function);
>
> Added: cfe/trunk/test/SemaCXX/predefined-expr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/predefined-expr.cpp?rev=188900&view=auto
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/predefined-expr.cpp (added)
> +++ cfe/trunk/test/SemaCXX/predefined-expr.cpp Wed Aug 21 06:45:27 2013
> @@ -0,0 +1,40 @@
> +// RUN: %clang_cc1 -std=c++1y -fblocks -fsyntax-only -verify %s
> +// PR16946
> +// expected-no-diagnostics
> +
> +auto foo() {
> + static_assert(sizeof(__func__) == 4, "foo");
> + static_assert(sizeof(__FUNCTION__) == 4, "foo");
> + static_assert(sizeof(__PRETTY_FUNCTION__) == 11, "auto foo()");
> + return 0;
> +}
> +
> +auto bar() -> decltype(42) {
> + static_assert(sizeof(__func__) == 4, "bar");
> + static_assert(sizeof(__FUNCTION__) == 4, "bar");
> + static_assert(sizeof(__PRETTY_FUNCTION__) == 10, "int bar()");
> + return 0;
> +}
> +
> +int main() {
> + static_assert(sizeof(__func__) == 5, "main");
> + static_assert(sizeof(__FUNCTION__) == 5, "main");
> + static_assert(sizeof(__PRETTY_FUNCTION__) == 11, "int main()");
> +
> + []() {
> + static_assert(sizeof(__func__) == 11, "operator()");
> + static_assert(sizeof(__FUNCTION__) == 11, "operator()");
> + static_assert(sizeof(__PRETTY_FUNCTION__) == 51,
> + "auto main()::<anonymous class>::operator()() const");
> + return 0;
> + }
> + ();
> +
> + ^{
> + // FIXME: This is obviously wrong.
> + static_assert(sizeof(__func__) == 1, "__main_block_invoke");
> + static_assert(sizeof(__FUNCTION__) == 1, "__main_block_invoke");
> + static_assert(sizeof(__PRETTY_FUNCTION__) == 1, "__main_block_invoke");
> + }
> + ();
> +}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
More information about the cfe-commits
mailing list