Repeated expression in AST

The typescript above shows that clang put *two* references to the same
expr in the AST (one as TypeOfExprType argument and another as
CompoundStmt child).

This leads to shown wrong warning and (I guess) to other problems.

Is it expected?

$ cat bug.c
int f(int);
void h() {
  __typeof(*(int (*)[f(1)]) 0) x;
}

$ _clang -cc1 -ast-print bug.c
bug.c:3:12: warning: expression result unused
  __typeof(*(int (*)[f(1)]) 0) x;
           ^~~~~~~~~~~~~~~~~~
struct __va_list_tag {
    unsigned int gp_offset;
    unsigned int fp_offset;
    void *overflow_arg_area;
    void *reg_save_area;
};
typedef struct __va_list_tag __va_list_tag;
int f(int);
void h() {
    (*(int (*)[f(1)])0);
    typeof (*(int (*)[f(1)])0) x;
}

1 warning generated.

$ _clang -cc1 -ast-dump bug.c
bug.c:3:12: warning: expression result unused
  __typeof(*(int (*)[f(1)]) 0) x;
           ^~~~~~~~~~~~~~~~~~
typedef __int128_t __int128_t;
typedef __uint128_t __uint128_t;
struct __va_list_tag {
    unsigned int gp_offset;
    unsigned int fp_offset;
    void *overflow_arg_area;
    void *reg_save_area;
};
typedef struct __va_list_tag __va_list_tag;
typedef __va_list_tag __builtin_va_list[1];
int f(int);
void h() (CompoundStmt 0x48a5448 <bug.c:2:10, line:4:1>
  (ImplicitCastExpr 0x48a5378 <line:3:11, col:30> 'int *'
<ArrayToPointerDecay>
    (ParenExpr 0x48a5330 <col:11, col:30> 'int [f(1)]':'int [f(1)]' lvalue
      (UnaryOperator 0x48a5310 <col:12, col:29> 'int [f(1)]':'int
[f(1)]' lvalue prefix '*'
        (CStyleCastExpr 0x48a52e8 <col:13, col:29> 'int (*)[f(1)]'
<NullToPointer>
          (IntegerLiteral 0x48a51c0 <col:29> 'int' 0)))))
  (DeclStmt 0x48a5430 <col:3, col:33>
    0x48a53d0 "typeof (*(int (*)[f(1)])0) x"))

1 warning generated.

Ping.

Ping^2.

Please find attached a patch for review that, among other things, should
fix the issue reported by Abramo in the mail below:

The typescript above shows that clang put *two* references to the same
expr in the AST (one as TypeOfExprType argument and another as
CompoundStmt child).

This leads to shown wrong warning and (I guess) to other problems.

Is it expected?

$ cat bug.c
int f(int);
void h() {
  __typeof(*(int (*)[f(1)]) 0) x;
}

$ _clang -cc1 -ast-print bug.c
bug.c:3:12: warning: expression result unused
  __typeof(*(int (*)[f(1)]) 0) x;
           ^~~~~~~~~~~~~~~~~~
struct __va_list_tag {
    unsigned int gp_offset;
    unsigned int fp_offset;
    void *overflow_arg_area;
    void *reg_save_area;
};
typedef struct __va_list_tag __va_list_tag;
int f(int);
void h() {
    (*(int (*)[f(1)])0);
    typeof (*(int (*)[f(1)])0) x;
}

1 warning generated.

$ _clang -cc1 -ast-dump bug.c
bug.c:3:12: warning: expression result unused
  __typeof(*(int (*)[f(1)]) 0) x;
           ^~~~~~~~~~~~~~~~~~
typedef __int128_t __int128_t;
typedef __uint128_t __uint128_t;
struct __va_list_tag {
    unsigned int gp_offset;
    unsigned int fp_offset;
    void *overflow_arg_area;
    void *reg_save_area;
};
typedef struct __va_list_tag __va_list_tag;
typedef __va_list_tag __builtin_va_list[1];
int f(int);
void h() (CompoundStmt 0x48a5448 <bug.c:2:10, line:4:1>
  (ImplicitCastExpr 0x48a5378 <line:3:11, col:30> 'int *'
<ArrayToPointerDecay>
    (ParenExpr 0x48a5330 <col:11, col:30> 'int [f(1)]':'int [f(1)]' lvalue
      (UnaryOperator 0x48a5310 <col:12, col:29> 'int [f(1)]':'int
[f(1)]' lvalue prefix '*'
        (CStyleCastExpr 0x48a52e8 <col:13, col:29> 'int (*)[f(1)]'
<NullToPointer>
          (IntegerLiteral 0x48a51c0 <col:29> 'int' 0)))))
  (DeclStmt 0x48a5430 <col:3, col:33>
    0x48a53d0 "typeof (*(int (*)[f(1)])0) x"))

1 warning generated.

The patch gets rid of method

    StmtResult Sema::ActOnVlaStmt(const DeclSpec &DS);

and its (one and only) use in lib/Parse/ParseDecl.cpp,
which was causing the duplicate expressions (and spurious warnings)
mentioned above.

Code generation for VM types (which was previously relying on this
duplication of expressions) is now dealt with entirely in

    void CodeGenFunction::EmitVariablyModifiedType(QualType type);

Besides the issue mentioned above, the patch is sometime able to get rid
of redundant loads (see the FIXME removed from test/CodeGen/vla.c).

Moreover, it should be now able to correctly handle contrieved cases
where the typeof_expr node was not occurring as the top-most base node
(see new test test/CodeGen/vla-4.c), which were previously handled
incorrectly as far as we can tell.

Enea.

vla.patch (5.46 KB)

I believe that the patch does the right thing with TypeOfExpr and indeed
fix the wrong code generation (while it fixes also the reported
duplicated presence of expr in AST and the bogus warning).

If there are no objections I will commit it.

Looks good to me. I applied the change to Clang 2.9 and it appears to resolve the problem there as well. The patch did not apply cleanly against 2.9 due to changes in VLA handling following the 2.9 release. If anyone is interested in the changes for Clang 2.9, let me know and I'll make them available.

Tom.

In r147730.