erichkeane added a comment. In D129748#3653897 <https://reviews.llvm.org/D129748#3653897>, @ChuanqiXu wrote:
> 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. Hmm... interesting. I wish I had a better understanding of the ASTWriter to help with this, but given: 1- Getting modules compiled is important 2- this attribute is 'ignore-able', and is for diagnostics only 3- the ASTWriter/ASTReader seems to be messing this up. I think there _IS_ perhaps an acceptability to ignoring this attribute when it would cause a problem. However, I think doing it as you're doing it now is wrong. IF we are going to solve the symptom here, I think we should use a much more precise cut, and make either ASTReader not emit the attribute, or ASTWriter just 'ignore' it when reading. WDYT? @aaron.ballman as well... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129748/new/ https://reviews.llvm.org/D129748 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
