[PATCH] D133052: [clang] Avoid crash when expanding conversion templates in concepts.

[Luke Nihlen via Phabricator via cfe-commits]

luken-google added a comment.

In D133052#3765753 <https://reviews.llvm.org/D133052#3765753>, @ychen wrote:

> Oh, one more thing, we probably need to handle nested levels too, for example, `foo(a);` might be triggered by a template which may be in turn triggered by the concept check. So only checking `S.CodeSynthesisContexts.back()` seems not enough. We need to check if we're in concept check context regardless of instantiation levels.

I'll defer to your expertise on this one, but wanted to raise two concerns about this approach before proceeding:

a) The documentation around CodeSynthesisContexts asserts it should be treated as a stack (see https://github.com/llvm/llvm-project/blob/a5880b5f9c4a7ee543fbc38d528d2830fc27753e/clang/include/clang/Sema/Sema.h#L9131-L9132)

b) Given this is a heuristic fix, does it make sense to keep it as narrow as possible, to avoid unintended consequences? I share your confidence we will encounter other infinite template expansion bugs, but if we're not going to solve the general problem I would think that each individual fix should be conservative in its coverage.

Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133052