[cfe-commits] Patches for clang -ast-print

[Eli Friedman]

On Mon, Oct 29, 2012 at 5:29 PM, Jordan Rose <jordan_rose at apple.com> wrote:
>
> On Oct 29, 2012, at 17:23 , Eli Friedman <eli.friedman at gmail.com> wrote:
>
>> On Mon, Oct 29, 2012 at 5:00 PM, Jordan Rose <jordan_rose at apple.com> wrote:
>>>
>>> On Oct 29, 2012, at 13:25 , Eli Friedman <eli.friedman at gmail.com> wrote:
>>>
>>>> On Sun, Oct 28, 2012 at 1:52 PM, Grzegorz Jablonski <grzejabl at gmail.com> wrote:
>>>>> On 10/28/2012 09:04 PM, Eli Friedman wrote:
>>>>>>
>>>>>> On Sun, Oct 28, 2012 at 3:41 AM, Grzegorz Jablonski <grzejabl at gmail.com>
>>>>>> wrote:
>>>>>>>
>>>>>>> On 10/28/2012 03:45 AM, Eli Friedman wrote:
>>>>>>>>
>>>>>>>> We shouldn't be printing "operator const char*" in the first place.
>>>>>>>
>>>>>>> OK, revised patch.
>>>>>>
>>>>>> Please use CastExpr::getSubExprAsWritten.
>>>>>>
>>>>> Like that?
>>>>
>>>> (Adding cfe-commits to the CC list.)
>>>>
>>>> It's a bit scary that this affects CFG dumping. Can someone who knows
>>>> that code take a look?
>>>
>>> Yeah, this is definitely not desireable, even if it is just a debugging feature. PrinterHelpers allow us to do something special when printing a certain expression, and the helper might want to see one ImplicitCastExpr inside another.
>>>
>>> I'm not sure what the correct answer is now. Maybe the CFG printer is using the wrong PrintingPolicy anyway, but just skipping over implicit nodes seems scary.
>>
>> Hmm, yes... if we really need to provide the PrinterHelper a chance to
>> react at every node, then this patch won't work as-is.  Unfortunately,
>> there isn't any way to tell at the moment whether a CXXMemberCallExpr
>> is implicit (except maybe getRParenLoc().isValid()?); similarly for
>> MemberExpr.  Given that, this would become rather messy.
>>
>> That said, I think the CFG printer probably shouldn't be
>> pretty-printing expressions in the first place.  It certainly
>> shouldn't be depending on the StmtPrinter to reliably hand every
>> sub-expression back, if only because that could obscure the actual CFG
>> datastructure.
>
>
> Since the CFG is a linearized structure now, it's not printing CFG structure. Instead, it's trying to show the statement associated with a single CFGElement, such as a cast, in a way that you could match it up with the source but still see the evaluation pattern.

So we're printing what basically amounts to a derived analysis for all
users?  I guess that makes sense, to some extent... but is there no
way to formally query the CFGElements a given CFGElement depends on?

> It is deliberately trying to print the syntactic ("pretty") form of the statement, but with placeholders for subexpressions that appear earlier in the CFG. Maybe these two goals are incompatible, but I can say that what we have has really helped us on the analyzer side.

The issue comes down to the fact that you're trying to "pretty-print"
things which aren't actually written in the source.  Maybe we need a
PrintingPolicy bit for not-so-pretty-printing?

-Eli