On 21.08.2013, at 20:33, Richard Smith <[email protected]> wrote:
> On Wed, Aug 21, 2013 at 4:45 AM, Benjamin Kramer <[email protected]> > 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 > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
