Page MenuHomePhabricator

[AST] Add a isActuallyImplicitCast() helper to the CastExpr class.
AbandonedPublic

Authored by lebedev.ri on Jul 26 2018, 4:14 AM.

Details

Summary

This is mostly factored out of D48958.
As of D49838, ImplicitCastExpr can tell whether it is a part of ExplicitCastExpr group.
But given just the CastExpr, one still has to check that it is not ExplicitCastExpr itself,
before using ImplicitCastExpr::getIsPartOfExplicitCast().
Thus, it makes sense to factor out it into a helper function in CastExpr baseclass.

This indeed does not have tests. Would be happy to add those, if needed and told how to write those.

Diff Detail

Repository
rC Clang

Event Timeline

lebedev.ri created this revision.Jul 26 2018, 4:14 AM

I'm not sure that this logic requires a separate function. Since you've fixed the getIsPartOfExplicitCast logic correctly, it is pretty simple...

include/clang/AST/Expr.h
2856

These two sentences are both sentence fragments and should likely be joined.

A suggested alternative?
"An explicit cast corresponds to a cast written in the source code, while an implicit cast is materialized for the purposes of automatic conversions.

3022

Are you asking or telling? I'd prefer this comment not be here, or explain the logic.

lebedev.ri marked 2 inline comments as done.

Comment fine-tuning.

I'm not sure that this logic requires a separate function. Since you've fixed the getIsPartOfExplicitCast logic correctly, it is pretty simple...

Certainly. I can totally inline it into the caller,
but i *think* @rsmith suggested that such functionality should be present, so i'll wait for him to comment.

erichkeane accepted this revision.Jul 26 2018, 7:47 AM

If @rsmith requested this, I'm fine with it then.

This revision is now accepted and ready to land.Jul 26 2018, 7:47 AM
rsmith requested changes to this revision.Jul 26 2018, 1:23 PM

What I requested was that either we make CastExpr::isPartOfExplicitCast() return true for CastExprs that are not ImplicitCastExprs, or that we move isPartOfExplicitCast do the ImplicitCastExpr derived class. We don't need to do both, and since we're doing the latter in D49838, we don't need to do anything else. (Essentially, my request was to fix the bug that CastExpr::isPartOfExplicitCast() incorrectly returned false when called on an explicit cast expression, and I don't mind which way we fix it.)

This revision now requires changes to proceed.Jul 26 2018, 1:23 PM
lebedev.ri abandoned this revision.Jul 26 2018, 1:28 PM

What I requested was that either we make CastExpr::isPartOfExplicitCast() return true for CastExprs that are not ImplicitCastExprs, or that we move isPartOfExplicitCast do the ImplicitCastExpr derived class. We don't need to do both, and since we're doing the latter in D49838, we don't need to do anything else. (Essentially, my request was to fix the bug that CastExpr::isPartOfExplicitCast() incorrectly returned false when called on an explicit cast expression, and I don't mind which way we fix it.)

Ok, no problem at atll, so i have read too much into it :)
I will then simply inline this into the only caller.