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

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added a comment.

Thank you for the detailed explanation, @ChuanqiXu, that was very helpful.

It looks to me like the problem may be that the initial declaration of the `basic_string_view` class template is non-defining, but when serializing that declaration, we serialize a definition of the `basic_string_view<char>` specialization (note that we do not attempt to serialize a definition of the class template at this point); the AST serialization appears to be "jumping ahead". When the defining declaration is later serialized, we serialize the class template definition, but not the specialization definition (presumably because it was already serialized and/or dumped). This analysis assumes that the act of dumping the AST reflects the serialization order; that may not be a valid assumption. I didn't look into whether the serialization code does actually "jump ahead" as the dump appears to indicate. From the AST writing side, it seems to me that there should be a rule that an instantiated declaration should never be written prior to its point of instantiation. Thus, a corrected AST dump would look more like:

  |-ClassTemplateDecl 0x7913ac8 <./string_view.h:1:1, line:2:7> col:7 in A.<global> hidden basic_string_view
  | |-TemplateTypeParmDecl 0x7913950 <line:1:10, col:16> col:16 in A.<global> hidden class depth 0 index 0 _CharT
  | |-CXXRecordDecl 0x7913a18 <line:2:1, col:7> col:7 in A.<global> hidden class basic_string_view
  | `-ClassTemplateSpecializationDecl 0x7913cd0 <line:1:1, line:2:7> line:9:1 in A.<global> class basic_string_view definition
  |   `-TemplateArgument type 'char'
  |     `-BuiltinType 0x78c77b0 'char'
  |-TypedefDecl 0x7913e78 <line:4:1, col:33> col:33 in A.<global> hidden referenced string_view 'basic_string_view<char>':'string_view'
  | `-TemplateSpecializationType 0x7913df0 'basic_string_view<char>' sugar basic_string_view
  |   |-TemplateArgument type 'char'
  |   | `-BuiltinType 0x78c77b0 'char'
  |   `-RecordType 0x7913dd0 'string_view'
  |     `-ClassTemplateSpecialization 0x7913cd0 'basic_string_view'
  |-ClassTemplateDecl 0x7914040 prev 0x7913ac8 <line:6:1, line:14:1> line:9:1 in A.<global> hidden basic_string_view
  | |-TemplateTypeParmDecl 0x7913ed8 <line:6:10, col:16> col:16 in A.<global> hidden class depth 0 index 0 _CharT
  | |-CXXRecordDecl 0x7913fa8 prev 0x7913a18 <line:7:1, line:14:1> line:9:1 in A.<global> hidden class basic_string_view definition
  | | |-DefinitionData empty standard_layout trivially_copyable has_user_declared_ctor can_const_default_init
  | | | |-DefaultConstructor exists non_trivial user_provided defaulted_is_constexpr
  | | | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
  | | | |-MoveConstructor exists simple trivial needs_implicit
  | | | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
  | | | |-MoveAssignment exists simple trivial needs_implicit
  | | | `-Destructor simple irrelevant trivial constexpr needs_implicit
  | | |-PreferredNameAttr 0x7914100 <line:8:16, col:46> string_view
  | | |-CXXRecordDecl 0x7914168 <line:7:1, line:9:1> col:1 in A.<global> hidden implicit referenced class basic_string_view
  | | |-AccessSpecDecl 0x7914218 <line:10:1, col:7> col:1 in A.<global> public
  | | `-CXXConstructorDecl 0x79142d0 <line:11:5, line:13:5> line:11:5 in A.<global> hidden basic_string_view<_CharT> 'void ()'
  | |   `-CompoundStmt 0x79143a8 <line:12:5, line:13:5>
  | `-ClassTemplateSpecializationDecl 0x7913cd0 <line:1:1, line:2:7> line:9:1 in A.<global> class basic_string_view definition
  |   |-DefinitionData pass_in_registers empty standard_layout trivially_copyable has_user_declared_ctor can_const_default_init
  |   | |-DefaultConstructor exists non_trivial user_provided defaulted_is_constexpr
  |   | |-CopyConstructor simple trivial has_const_param implicit_has_const_param
  |   | |-MoveConstructor exists simple trivial
  |   | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
  |   | |-MoveAssignment exists simple trivial needs_implicit
  |   | `-Destructor simple irrelevant trivial constexpr
  |   |-TemplateArgument type 'char'
  |   | `-BuiltinType 0x78c77b0 'char'
  |   |-PreferredNameAttr 0x79143c8 <line:8:16, col:46> string_view
  |   |-CXXRecordDecl 0x7914698 <line:7:1, line:9:1> col:1 in A.<global> implicit class basic_string_view
  |   |-AccessSpecDecl 0x7914748 <line:10:1, col:7> col:1 in A.<global> public
  |   |-CXXConstructorDecl 0x7935158 <line:11:5, line:13:5> line:11:5 in A.<global> used basic_string_view 'void ()'
  |   | `-CompoundStmt 0x79143a8 <line:12:5, line:13:5>
  |   |-CXXConstructorDecl 0x7935300 <line:9:1> col:1 in A.<global> implicit constexpr basic_string_view 'void (const string_view &)' inline default trivial noexcept-unevaluated 0x7935300
  |   | `-ParmVarDecl 0x7935420 <col:1> col:1 in A.<global> 'const string_view &'
  |   |-CXXConstructorDecl 0x79354d0 <col:1> col:1 in A.<global> implicit constexpr basic_string_view 'void (string_view &&)' inline default trivial noexcept-unevaluated 0x79354d0
  |   | `-ParmVarDecl 0x79355f0 <col:1> col:1 in A.<global> 'string_view &&'
  |   `-CXXDestructorDecl 0x79357d0 <col:1> col:1 in A.<global> implicit referenced constexpr ~basic_string_view 'void () noexcept' inline default trivial

I wonder if it would be worth pursuing (unrelated to this issue) introducing an `ImplicitInstantiationDecl` or similar that would be inserted in the AST at the point of instantiation for an instantiated declaration/definition. That might be pretty noisy, but would (I think) simplify some aspects of the AST serialization.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129748