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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
