ChuanqiXu9 wrote:

> > > > > > > > > [do not merge] [runtime-cxxmodules] Rework our lazy template 
> > > > > > > > > specialization deserialization mechanism 
> > > > > > > > > [root-project/root#14495](https://github.com/root-project/root/pull/14495)
> > > > > > > > 
> > > > > > > > 
> > > > > > > > From 
> > > > > > > > [root-project/root#14495](https://github.com/root-project/root/pull/14495),
> > > > > > > >  I see there is new reply saying the testing is actually fine. 
> > > > > > > > Do you think we still need to split the patch?
> > > > > > > 
> > > > > > > 
> > > > > > > That comment was concerning the version of the patch that had the 
> > > > > > > lazy template deserialization turned off by default. Yes, I still 
> > > > > > > think that this patch should implement tha on-disk hash table on 
> > > > > > > top of D41416
> > > > > > 
> > > > > > 
> > > > > > OK. And would you like to send a PR for D41416? I've already fixed 
> > > > > > the issue mentioned in the review page. Then I'd like to send small 
> > > > > > and incremental patches on that.
> > > > > 
> > > > > 
> > > > > Do you mean that I should open a PR for D41416 and you will apply 
> > > > > your patch there? I have no problem if we do everything here as part 
> > > > > of this PR. This way we will have the full history of how this was 
> > > > > born in one place ;)
> > > > 
> > > > 
> > > > Yeah, and please create a branch under llvm/llvm-project directly. Then 
> > > > I can perform stacked PR on that.
> > > 
> > > 
> > > There it is: 
> > > https://github.com/llvm/llvm-project/tree/users/vgvassilev/D41416_D153003
> > 
> > 
> > Is there a PR? So that we can comment on that. For example, is D153003 
> > necessary? I remeber in the review process, we think it may not be wanted.
> 
> I can create a PR. My understanding is that D153003 because of the 
> intricacies of the ODR hashing approach. If we create an alternative we can 
> take it out. For now, we can keep it and later we can drop it.

I feel better to drop D153003 if it is not a blocking issue. I feel it make 
things more complicated...

https://github.com/llvm/llvm-project/pull/76774
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to