carlosgalvezp wrote: Being totally unfamiliar with the code, I have a feeling that we shouldn't be fixing in the `analyze` function. `analyze` has 1 single responsibility: wrap `analyzeImpl` in order to add cacheability for performance reasons, and it does so simply and well, and it does exactly what one would expect - no more no less.
However, the real problem is that `analyzeImpl` ultimately might end up calling itself, via `analyzeUnresolvedOrDefaulted -> analyzeRecord -> analyze -> analyzeImpl` (IIRC). This is not obvious due to how the code is written, and this patch would make it even less obvious IMO. In that sense I believe it would make sense to ensure no recursion happens somewhere in the chain I described above, not in `analyze`. WDYT? https://github.com/llvm/llvm-project/pull/66810 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
