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

Fri Jul 15 06:35:32 PDT 2022

aaron.ballman added a comment.

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

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

I'm less convinced that's reasonable, because that root cause seems like it will impact several other attributes as well. For example, I would be `IBOutletCollection`, `OwnerAttr`, `PointerAttr`, and `TypeTagForDatatypeAttr` all behave the same way as they all take a type argument. I don't think we want to selectively disable so many attributes (for example, disallowing owner and pointer attributes means we lose out on C++ Core Guideline features that people will likely expect to be able to use with modules. However, I agree that being able to modularize the STL is an important use case.

I'd like to understand better what the root cause is. The attribute requires a resolved type name, which means we must have seen the type before the attribute when writing the AST out. When reading the AST back in, why is the type not visible before the attribute? That sounds like we're writing the AST out in the wrong order somehow, or something odd like that.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129748