[cfe-commits] r147730 - in /cfe/trunk: include/clang/Sema/Sema.h lib/CodeGen/CodeGenFunction.cpp lib/Parse/ParseDecl.cpp lib/Sema/SemaDecl.cpp test/CodeGen/vla.c

[John McCall]

On Jan 7, 2012, at 2:52 AM, Abramo Bagnara wrote:

> Author: abramo
> Date: Sat Jan  7 04:52:36 2012
> New Revision: 147730
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=147730&view=rev
> Log:
> Fixed TypeofExpr AST and code generation.
> 
> Modified:
>    cfe/trunk/include/clang/Sema/Sema.h
>    cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
>    cfe/trunk/lib/Parse/ParseDecl.cpp
>    cfe/trunk/lib/Sema/SemaDecl.cpp
>    cfe/trunk/test/CodeGen/vla.c
> 
> Modified: cfe/trunk/include/clang/Sema/Sema.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=147730&r1=147729&r2=147730&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Sema/Sema.h (original)
> +++ cfe/trunk/include/clang/Sema/Sema.h Sat Jan  7 04:52:36 2012
> @@ -1146,8 +1146,6 @@
>                                    DeclSpec &DS,
>                                    MultiTemplateParamsArg TemplateParams);
> 
> -  StmtResult ActOnVlaStmt(const DeclSpec &DS);
> -
>   Decl *BuildAnonymousStructOrUnion(Scope *S, DeclSpec &DS,
>                                     AccessSpecifier AS,
>                                     RecordDecl *Record);
> 
> Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=147730&r1=147729&r2=147730&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Sat Jan  7 04:52:36 2012
> @@ -929,19 +929,33 @@
> 
>   // We're going to walk down into the type and look for VLA
>   // expressions.
> -  type = type.getCanonicalType();
>   do {
>     assert(type->isVariablyModifiedType());
> 
>     const Type *ty = type.getTypePtr();
>     switch (ty->getTypeClass()) {
> +
> +    default:
> +      // Only sugared types (different from typeof_expr) can reach this point.
> +      assert(!type.isCanonical() && "unhandled canonical type!");
> +      type = type.getSingleStepDesugaredType(getContext());
> +      break;

This would look through typedefs and therefore be very wrong if it could
actually execute.  Fortunately, it can't, except with decltype.  Still, please
don't leave this code in there.

Also, rather than re-introducing a default case, please just nullify and
enumerate the metaprogramming case you care about
(NON_CANONICAL_UNLESS_DEPENDENT_TYPE).  Alternatively,
change the metaprogramming file so that it provides opt-out filters for
specific cases the same way that StmtNodes.inc does.

John.