[PATCH] D64762: [AST] Treat semantic form of InitListExpr as implicit code in traversals

[Dmitri Gribenko via Phabricator via cfe-commits]

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