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

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov marked an inline comment as done.
ilya-biryukov added inline comments.


================
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:
> 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.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64762/new/

https://reviews.llvm.org/D64762