[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
Tue Jul 16 15:49:01 PDT 2024


dwblaikie wrote:

> > That's a pretty substantial policy change to propose, and this probably isn't the place to propose/discuss it. If that's your intent, probably best to take that up on discord.
> 
> I am not proposing a policy change. I believe the current policy is aimed at giving an escape hatch for projects which there is basically one or two active developers. I am pointing out that I believe we don't need for that escape hatch to apply to any parts of clang currently.

With a fair bit of experience on this project and the norms - I can say pretty confidently that was/is not the intent of the current policy. Especially for code owners, there's an expectation they probably know better than anyone else & can make summary judgments on direction in many cases. Every now and then they'll ask for second opinions on uncertain directions, but can probably pretty well determine what's acceptable for most work without a second opinion.

(honestly, moving to all changes being reviewed might be good for the project - though I think we'd need to work more on ownership and reviewer availability/authority/process, but it's not where we are today in either case (policy or socially))

The dev policy says "Smaller patches (or patches where the developer owns the component) that meet likely-community-consensus requirements (as apply to all patch approvals) can be committed prior to an explicit review. " (in this case @ChuanqiXu9 is the owner of clang's modules support)

> > (FWIW, check some of the recent modules changes @ChuanqiXu9 has been working on to see that reviewer bandwidth here is pretty thin (& my experience in LLVM in general, including clang, is that reviewer bandwidth is pretty thin - though it is something we should address & I do think it might be time to change LLVM's post-commit review policy, but I think it'll be a substantial amount of work))
> 
> If you feel bandwidth for modules is pretty thin, I put myself available as a reviewer, so feel free to ping me. I may not have a lot of time available for fully reviewing big patches, but I can certainly help with the smaller patches such as this one.

While I appreciate the offer and would encourage you to review these changes - I'll point out that one of the reasons these reviews haven't been forthcoming is that the feature area is fairly complicated and nuanced.

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


More information about the cfe-commits mailing list