[cfe-commits] [PATCH] Fix pretty print when dealing with anonymous structures. PR14316.

[David Blaikie]

On Mon, Nov 12, 2012 at 12:19 AM, Florent Bruneau
<florent.bruneau at intersec.com> wrote:
> On Mon 12 Nov 2012 05:57:02 AM CET, David Blaikie wrote:
>>
>> On Sun, Nov 11, 2012 at 12:25 PM, Florent Bruneau
>> <florent.bruneau at intersec.com>  wrote:
>>>
>>> When I have an expression of the form:
>>>
>>>   struct blah {
>>>     struct {
>>>       struct {
>>>         int b;
>>>       };
>>>     };
>>>   };
>>>
>>>   int foo(const struct blah *b) {
>>>     return b->b;
>>>   }
>>>
>>> The expression 'b->b' has the following (correct) AST:
>>>
>>>   (ImplicitCastExpr 0x44bc968 'int'<LValueToRValue>
>>>     (MemberExpr 0x44bc8f0 'int' lvalue .b 0x44899b0
>>>       (MemberExpr 0x44bc8c0 'struct blah::<anonymous at foo.c:3:9>'
>>> lvalue .  0x4489a50
>>>         (MemberExpr 0x44bc890 'struct blah::<anonymous at foo.c:2:5>'
>>> lvalue ->  0x44bc2f0
>>>           (ImplicitCastExpr 0x44bc878 'union blah *'<LValueToRValue>
>>>             (DeclRefExpr 0x44bc850 'union blah *' lvalue ParmVar
>>> 0x44bc440 'b' 'union blah *'))))))
>>>
>>> But the pretty printer (and as a consequence clang -cc1 -ast-print foo.c)
>>> badly print this as 'b.b' because all the MemberExpr targeting anonymous
>>> types are simply ignored. The patch fix that behavior: the arrow/dot
>>> notation is printed if the base of the expression is not anonymous.
>>
>>
>> This looks reasonable to me, but could you include a test case (
>> http://llvm.org/docs/DeveloperPolicy.html#test-cases ). Probably the
>> easiest/best way would be to use -ast-print as you mentioned in your
>> description. If you search for "ast-print" in clang's test directory
>> you'll find a smattering of test cases that test ast print output. If
>> I had to guess where to put this one, I might consider putting it in
>> test/Sema/ast-print.c
>
>
> Thanks for the review.
>
> Here's the patch with a testcase. I hope I put it in the right folder. I
> considered putting it in either Coverage/ast-printing.c or Sema/ast-print.c
> but neither of them actually seem to check the output of the printer.

Thanks for that. I moved the test into Sema/ast-print.c and added the
requisite FileCheck usage there (we try to minimize the number of
separate test files we have to reduce overall test execution time
which can be dominated by process startup time of each compiler
process for each test file).

Committed as r167736.

- David