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

[David Blaikie]

On Thu, Feb 23, 2012 at 1:01 PM, Nicola Gigante
<nicola.gigante at gmail.com> wrote:
>
> Il giorno 16/feb/2012, alle ore 21:21, Nicola Gigante ha scritto:
>
>>
>> Il giorno 13/feb/2012, alle ore 15:42, Nicola Gigante ha scritto:
>>
>>>
>>> Il giorno 09/feb/2012, alle ore 13:36, Nicola Gigante ha scritto:
>>>>
>>>> Forget what I said on the last mail. I was wrong.
>>>> Now I've fixed the patch. It passes all the tests an applies to
>>>> the latest revision (r150076).
>>>> If it's ok, I'd commit it :)
>>>
>>> Ping?
>>> Can I commit the patch?
>>
>> Ping^2?
>> Is anybody reviewing my patch?
>
> Ping^3?
>
> I've attached an updated patch that applies to the current
> revision, just in case.

For what it's worth, this looks like good stuff from an AST fidelity
perspective (I'm not sure I've given the actual implementation enough
review to have an opinion on whether it's the right approach, etc).
Here's one minor cleanup that we can do because of this change:


diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index a89e813..fe3a8a6 100644
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -4234,7 +4234,7 @@ void AnalyzeImplicitConversions(Sema &S, Expr
*OrigE, SourceLocation CC) {

   // Skip past explicit casts.
   if (isa<ExplicitCastExpr>(E)) {
-    E = cast<ExplicitCastExpr>(E)->getSubExpr()->IgnoreParenImpCasts();
+    E = cast<ExplicitCastExpr>(E)->getSubExpr();
     return AnalyzeImplicitConversions(S, E, CC);
   }

(this came to my attention since I've been dabbling in this code to
try to fix the warn_impcast_null_pointer_to_integer warning & thus
playing around in lots of this SemaChecking.cpp code)

No doubt there's other places that have to explicitly walk through the
implicit casts inside an explicit cast to perform similar work. Also,
I wonder, is it ever possible for an explicit cast to cause/have
implicit casts within it? Previously we wouldn't've been able to
understand the difference there (because all the implicit casts under
the noop explicit cast would be indistinguishable from one another).

- David