hahnjo 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.

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

Reply via email to