[cfe-commits] [cfe-dev] [review request] Removing redundant implicit casts in the AST, take 2

John McCall rjmccall at apple.com
Thu Mar 8 00:29:04 PST 2012

On Mar 7, 2012, at 11:51 PM, Enea Zaffanella wrote:
> On 03/08/2012 03:52 AM, John McCall wrote:
>> On Feb 23, 2012, at 6:42 PM, David Blaikie wrote:
>>> Sorry, I'll provide some more context - AnalyzeImplicitConversions
>>> itself does "IgnoreParensImpCasts" as its first thing - but not before
>>> recording the QualType of the outer Expr (lines 4202, 4203). Many of
>>> the diagnostics work simply by comparing the type of the outer
>>> expression with the type of the inner one - thus they fired /all/ if
>>> you didn't have that IgnoreParensImpCasts before the
>>> AnalyzeImplicitConversions call - because they looked just like
>>> implicit conversions (they /were/ in the AST). Now that explicit
>>> conversions aren't implemented as implicit ones this problem doesn't
>>> exist.
>>> In fact when I made this change I was hoping there were cases where an
>>> explicit conversion could contain an implicit one as you showed - but
>>> further, that such cases might have real warnings. Turned out removing
>>> that outer IgnoreParensImpCasts didn't cause any clang tests to change
>>> at all... nice, even if it didn't allow us to catch new cases.
>>> In any case I suspect it's the right thing - there's no reason for us
>>> to be ignoring those implicit conversions except that we were doing it
>>> to support our not-so-great AST representation of explicit
>>> conversions.
>> There are a few situations where we implement a cast with multiple
>> implicit conversions.  One example would be something like
>>   (_Complex float) 0
>> which is an IntegralToFloating conversion followed by a
>> FloatingRealToComplex.
>> John.
> One option coming to mind (it could be the case that it was already proposed on this list some time ago) would be to change the representation of casts in the AST so that a single (implicit or explicit) cast node records the whole chain of conversions (that is, the sequence of cast kinds) needed to transform the source type into the destination type. This would save some memory and maybe it could also simplify the logic in some AST visitors. (That said, it would probably require some big effort to adapt all of the existing visitors.)

I can't imagine how this would simplify the logic in any AST visitors.  Visitors that don't care about the semantics of the cast will still see an ICE node that they have to look through (except in the relatively uncommon case of an explicit cast).  Visitors that do care about those semantics will suddenly have to scan through a list of operations for every node, and they'll have to re-derive all the intermediate types.


More information about the cfe-commits mailing list