[PATCH] D129748: [Modules] Disable preferred_name attribute in C++20 Modules

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D129748#3651771 <https://reviews.llvm.org/D129748#3651771>, @erichkeane wrote:

> I guess I don't have a good idea why this attribute would cause ODR issues?  It would seem that if it appeared in 2 different TUs that we could just 'pick' whichever we wanted, right?

If the compiler finds it appeared in 2 different TUs with different definition (although the judgement is wrong), the compiler would emit an error. So it would block the uses of C++20 Modules with `preferred_name`.

> The description in the bug report of the problem isn't clear to me what the actual issue is.

Sorry. My bad. Let me try to clarify it. When we write the attribute `preferred_name(foo)` in ASTWriter, the compiler would try to write the type for the argument `foo`. Then when the compiler write the type for `foo`, the compiler find the type for `foo` is a TypeDef type. So the compiler would write the corresponding type `foo_templ<char>`. The key point here is that the AST for `foo_templ<char>` is complete now. Since the AST for `foo_templ<char>` is constructed in Sema.

But problem comes when we read it. When we read the attribute `preferred_name(foo)`, we would read the type for the argument `foo` and then we would try to read the type `foo_templ<char>` later. However, the key problem here is that when we read `foo_templ<char>`, its AST is not constructed yet! So we get a different type with the writer writes. So here is the ODR violation.

The problem is fundamental and I've spent 2 weeks on it. But I don't find any fixes for it. Then I found that, once I disabled `preferred_name`, we could go much further. So I am wondering if it would be an option to skip `preferred_name` if C++ modules is enabled. The idea may not be good in general. But I feel it might be an option in this specific case given it is hard to solve and `preferred_name` is primarily for printers.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129748/new/

https://reviews.llvm.org/D129748