ChuanqiXu9 wrote:
> > 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.
>
> I'm ok with pushing the commits one-by-one after the PR is reviewed, just let
> me know.
>
> > > Complete only needed partial specializations: It is unclear (to me) why
> > > this needs to be done "for safety", but this change significantly
> > > improves the effectiveness of lazy loading.
> >
> >
> > This comes from the logic: if we have a partial template specialization
> > `A<int, T, U>` and we need a full specialization for `A<int, double,
> > double>`, we hope the partial specialization to be loaded
>
> Sure, but in my understanding, that's not needed on the `ASTReader` side but
> is taken care of by `Sema` (?). For the following example:
>
> ```c++
> //--- partial.cppm
> export module partial;
>
> export template <typename S, typename T, typename U>
> struct Partial {
> static constexpr int Value() { return 0; }
> };
>
> export template <typename T, typename U>
> struct Partial<int, T, U> {
> static constexpr int Value() { return 1; }
> };
>
> //--- partial.cpp
> import partial;
>
> static_assert(Partial<int, double, double>::Value() == 1);
> ```
>
> (I assume that's what you have in mind?) I see two calls to
> `ASTReader::CompleteRedeclChain` (with this PR applied): The first asks for
> the full instantiation `Partial<int, double, double>` and regardless of what
> we load, the answer to the query is that it's not defined yet. The second
> asks for the partial specialization `Partial<int, T, U>` and then
> instantiation proceeds to do the right thing.
If it works, I feel good with it.
https://github.com/llvm/llvm-project/pull/133057
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
