[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)

David Blaikie via cfe-commits cfe-commits at lists.llvm.org
Fri Jul 12 09:48:51 PDT 2024


dwblaikie wrote:

> I think the appropriate tag for such commits would be NFCI, and should still require PR and review.

Clarifying a couple of things here
1) I tend to agree the patch might've been a bit subtle and maybe split up into smaller, independent parts (usual rules: [be an isolated change. Independent changes should be submitted as separate patches as this makes reviewing easier](https://llvm.org/docs/Contributing.html#how-to-submit-a-patch)
2) I really don't think anyone should read into a distinction between NFC and NFCI, I don't think it's helpful to try to create a distinction here - NFC is already a statement of intent, as much as any patch description is always a statement of intent. We all make mistakes, but I don't think adding "intended" to the acronym is really helpful
3) Non-NFC changes don't necessarily require PR/review - LLVM allows for, especially code owners, to commit without precommit review if the change is sufficiently obvious. So some simple cleanups, clear bug fixes, direction already has buy-in, etc, are often committed without precommit review.

But, yeah, bit more caution in the future, and probably smaller/more isolated patches would be good in any case.

https://github.com/llvm/llvm-project/pull/75912


More information about the cfe-commits mailing list