[PATCH] D57267: [AST] Factor out the logic of the various Expr::Ignore*
Reid Kleckner via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 8 12:34:25 PST 2019
rnk added a comment.
In D57267#1390484 <https://reviews.llvm.org/D57267#1390484>, @riccibruno wrote:
> @void @rnk Perhaps you can comment on this: currently `Expr::IgnoreImpCasts` skips `FullExpr`s, but `Expr::IgnoreParenImpCasts` only skips (via `IgnoreParens`) `ConstantExpr`s. Is there any reason for this inconsistency ? I would like to add `FullExpr` to the nodes skipped by `IgnoreParenImpCasts` for consistency but I am worried about unexpected issues even though all tests pass.
Yes, @rsmith asked me to skip all FullExprs, but that change did not pass tests, so I only made IgnoreParens ignore ConstantExpr to preserve existing behavior. There is no good design reason for it to be that way, and if you can adjust the callers to account for the new behavior, I think making them consistent would be good.
CHANGES SINCE LAST ACTION
More information about the cfe-commits