gribozavr accepted this revision.
gribozavr added inline comments.
This revision is now accepted and ready to land.
================
Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:2332
S->isSemanticForm() ? S->getSyntacticForm() : S, Queue));
TRY_TO(TraverseSynOrSemInitListExpr(
S->isSemanticForm() ? S : S->getSemanticForm(), Queue));
----------------
ilya-biryukov wrote:
> ilya-biryukov wrote:
> > gribozavr wrote:
> > > Instead of adding a whole new if statement, could you wrap the second
> > > existing TRY_TO in `if(shouldVisitImplicitCode())` ?
> > Despite looking very similar, that would **not** be equivalent to the
> > current version.
> >
> > For most init lists (that do not have alternative "form"), the following
> > invariants hold:
> > ```
> > InitList* E = ...;
> > assert(E->isSemanticForm());
> > assert(E->isSyntacticForm());
> > assert(E->getSynacticForm() == nullptr);
> > ```
> >
> > This subtle fact means the current code does not traversed the list twice
> > if they do not have an alternative form (either semantic or syntactic).
> >
> > Now, if we only run the first statement, we will call
> > `TraverseSynOrSemInitListExpr(S->getSyntacticForm())` and
> > `S->getSyntacticForm()` returns `null`. So we don't traverse anything.
> >
> > I tried various ways to keep both calls, but they all ended up being too
> > complicated, hence the final version. Let me know if you see a better way
> > to address this.
> To make the last sentence less confusing:
> I tried various ways to keep **only two** calls, but they were too
> complicated and I ended up introducing an extra call to `TraverseSyn...`
> instead.
>
> assert(E->getSynacticForm() == nullptr);
That's... a really nice API.
What do you think about the following:
```
if (S->isSyntacticForm() && S->isSemanticForm()) {
// `S` does not have alternative forms, traverse the only form that's
available.
TRY_TO(TraverseSynOrSemInitListExpr(S, Queue));
return true;
}
TRY_TO(TraverseSynOrSemInitListExpr(
S->isSemanticForm() ? S->getSyntacticForm() : S, Queue));
if (getDerived().shouldVisitImplicitCode()) {
TRY_TO(TraverseSynOrSemInitListExpr(
S->isSyntacticForm() ? S->getSemanticForm() : S, Queue));
}
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64762/new/
https://reviews.llvm.org/D64762
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
