[clang] [Serialization] Fix lazy template loading (PR #133057)
Chuanqi Xu via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 28 02:53:12 PDT 2025
ChuanqiXu9 wrote:
> > > > > > While I may not able to look into them in detail recently, it may be helpful to split this into seperate patches to review and to land.
> > > > >
> > > > >
> > > > > I initially considered this, but @vgvassilev said in [root-project/root#17722 (comment)](https://github.com/root-project/root/pull/17722#issuecomment-2706555950) he prefers a single PR, also for external testing.
> > > >
> > > >
> > > > Maybe you can test it with this and land it with different patches. So that we can revert one of them if either of them are problematic but other parts are fine.
> > >
> > >
> > > This is a relatively small patch focused on reducing the round trips to modules deserialization. I see this as an atomic change that if it goes in partially would defeat its purpose. What's the goal of a partial optimization?
> >
> >
> > I think partial optimizations are optimization too. If these codes are not dependent on each other, it should be better to split them.
> > Given the scale of the patch, it may not be serious problem actually. I still think it is better to land them separately, but if you want to save some typings. I don't feel too bad.
>
> Honestly I am more concerned about the tests that @ilya-biryukov is running. As long as they are happy I do not particularly care about commit style. Although it'd be weird to land 40 line patch in many commits :)
I don't feel odd. I remember it is (or was) LLVM's policy that smaller patches are preferred : )
https://github.com/llvm/llvm-project/pull/133057
More information about the cfe-commits
mailing list