https://github.com/Sirraide requested changes to this pull request.
I’ve taken another look at this since it’s been a while, and you seem to be making good progress, from what I can tell; there are a few overarching things I’d like to point out here. - The way effects are being cached seems a bit too convoluted right now; I think you should be able to just use a `FoldingSet` for that like we do for types, but also, I don’t think the caching is really necessary (see my follow-up comments below). - I haven’t taken a look at the code in `AnalysisBasedWarnings.cpp` both since that’s a part of the code base that I’m not too familiar with and also because there are already enough things to comment on in all the other files, and there’s no point in making one single humongous review imo. - We might want to look into TableGen’ing quite a bit of the effects implementation like we do for attributes—especially if we’re planning to add more in the future; that said, I don’t believe we should do that as part of this pr (both because it’d be quite a bit of extra work and because we can’t really start generating code if we don’t yet know what we even want to generate), but it’s something that we should keep in mind as a follow up on this, so I just wanted to point that out. https://github.com/llvm/llvm-project/pull/84983 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
