NCGenerics: New Inverse Mangling 3DS XL #71878

kavon · 2024-02-26T04:18:18Z

Initial implementation of mangling for inverse requirements for NoncopyableGenerics. There are some known issues:

(1) Protocols with ~Copyable have bogus default witness tables. Entries for a default implementation of a method requirement gives this:

sil_default_witness_table NoncopyableProto {
  no_default
}

In general, protocols aren't well tested or fully covered by this PR. As discussion below reveals, we may need an updated mangling scheme to handle associated types.

(2) a number of tests now regress when NoncopyableGenerics is enabled in the compiler:

Failed Tests (4):
  Swift(macosx-x86_64) :: IRGen/existential_shape_metadata.swift
  Swift(macosx-x86_64) :: SILGen/existential_member_accesses_self_assoctype.swift
  Swift(macosx-x86_64) :: SILGen/tuple_attribute_reabstraction.swift
  Swift(macosx-x86_64) :: SILGen/variadic-generic-reabstract-tuple-result.swift

These have been marked XFAIL: noncopyable_generics since they're most likely issues that have been uncovered now that we're mangling inverses.

(3) type aliases were left basically as a TODO. I think since they can appear in an extension, and have their own generic signature, we need to be careful about how we mangle them. I have no tests for them yet either.

kavon · 2024-02-26T04:18:27Z

@swift-ci smoke test

kavon · 2024-02-26T04:45:07Z

preset=ncgenerics,smoketest=macosx
@swift-ci Please test with preset macOS Platform

include/swift/AST/ASTMangler.h

lib/AST/ASTMangler.cpp

lib/AST/Requirement.cpp

include/swift/AST/GenericSignature.h

kavon · 2024-02-27T06:13:09Z

@swift-ci test

kavon · 2024-02-28T03:54:58Z

@swift-ci test

kavon · 2024-03-02T05:03:54Z

@swift-ci smoke test

kavon · 2024-03-02T08:45:12Z

@swift-ci smoke test

docs/ABI/Mangling.rst

DougGregor · 2024-03-04T18:44:13Z

docs/ABI/Mangling.rst

@@ -925,13 +1012,19 @@ now codified into the ABI; the index 0 is therefore reserved.

  generic-param-pack-marker ::= 'Rv' GENERIC_PARAM-INDEX   // generic parameter pack marker

+  INVERTIBLE-KIND ::= 'c'  // Copyable
+  INVERTIBLE-KIND ::= 'e'  // Escapable


I don't think we should bake the invertible protocols we know about into the mangling scheme. Rather, I think we should mangle an "inverted conformance requirement" and add standard substitutions for Copyable and Escapable to keep it smaller.

I considered that approach and would support it. I just went with this current approach as it was simple to sketch out.

DougGregor · 2024-03-04T18:45:50Z

docs/ABI/Mangling.rst

  GENERIC-PARAM-COUNT ::= 'z'                // zero parameters
  GENERIC-PARAM-COUNT ::= INDEX              // N+1 parameters

  requirement ::= protocol 'R' GENERIC-PARAM-INDEX                  // protocol requirement
  requirement ::= protocol assoc-type-name 'Rp' GENERIC-PARAM-INDEX // protocol requirement on associated type
  requirement ::= protocol assoc-type-list 'RP' GENERIC-PARAM-INDEX // protocol requirement on associated type at depth
  requirement ::= protocol substitution 'RQ'                        // protocol requirement with substitution
+#if SWIFT_RUNTIME_VERSION >= 6.0
+  requirement ::= 'Ri' INVERTIBLE-KIND GENERIC-PARAM-INDEX          // inverse requirement


We'll also need a version that works with associated types, I presume?

Yes I believe we'll need them.

@slavapestov seems to think we won’t need them

Currently the only place in the language where you can state T: ~Copyable with T a dependent member type is inside of a protocol, eg

protocol P { associatedtype A: ~Copyable }

I was thinking that for symbol mangling, we don't ever need this because the requirement signature is not mangled as part of any symbol. However, now that I think about it I do believe we mangle the requirement signature in the protocol's context descriptor. So perhaps we need a mangling here after all.

Yes, we do mangle the requirement signature as part of the protocol's context descriptor.

include/swift/AST/DeclAttr.def

lib/AST/ASTMangler.cpp

kavon · 2024-03-04T21:07:48Z

@swift-ci smoke test

kavon · 2024-03-05T07:12:04Z

@swift-ci smoke test

kavon · 2024-03-05T07:12:20Z

@swift-ci test

kavon · 2024-03-05T07:14:12Z

preset=ncgenerics,smoketest=macosx
@swift-ci Please test with preset macOS Platform

kavon · 2024-03-05T08:07:26Z

@swift-ci test

kavon · 2024-03-05T08:07:32Z

preset=ncgenerics,smoketest=macosx
@swift-ci Please test with preset macOS Platform

kavon · 2024-03-05T17:47:18Z

Failed Tests (2):
  Swift(macosx-x86_64) :: SILGen/mangling_inverse_generics.swift
  Swift(macosx-x86_64) :: stdlib/Observation/Observable.swift

kavon · 2024-03-05T17:55:53Z

@swift-ci smoke test

kavon · 2024-03-05T19:05:28Z

@swift-ci test platform linux

kavon · 2024-03-05T19:07:29Z

@swift-ci Please test Linux platform

kavon · 2024-03-05T20:56:22Z

@swift-ci smoke test

kavon · 2024-03-05T20:56:32Z

preset=ncgenerics,smoketest=macosx
@swift-ci Please test with preset macOS Platform

After implementing mangling for inverses, issues have been revealed.

kavon · 2024-03-05T22:20:48Z

@swift-ci smoke test

kavon · 2024-03-06T00:14:40Z

@swift-ci smoke test linux

slavapestov reviewed Feb 26, 2024

View reviewed changes

include/swift/AST/ASTMangler.h Outdated Show resolved Hide resolved

lib/AST/ASTMangler.cpp Outdated Show resolved Hide resolved

lib/AST/Requirement.cpp Outdated Show resolved Hide resolved

slavapestov reviewed Feb 26, 2024

View reviewed changes

include/swift/AST/GenericSignature.h Outdated Show resolved Hide resolved

kavon force-pushed the ncgenerics-mangling-2 branch 2 times, most recently from 17cb5cd to 0af627b Compare February 27, 2024 06:08

kavon force-pushed the ncgenerics-mangling-2 branch from 0af627b to a91fea5 Compare February 28, 2024 00:51

kavon force-pushed the ncgenerics-mangling-2 branch 3 times, most recently from 31146b7 to 9521098 Compare March 2, 2024 05:02

kavon added 3 commits March 2, 2024 09:57

NFC: remove dead code

87a2def

NFC: clarify comment

a754f4b

Test: more existential coverage

19a5bb0

kavon force-pushed the ncgenerics-mangling-2 branch from d94cb78 to a981a53 Compare March 2, 2024 17:59

DougGregor reviewed Mar 4, 2024

View reviewed changes

kavon force-pushed the ncgenerics-mangling-2 branch from a981a53 to a07d9eb Compare March 4, 2024 21:07

kavon force-pushed the ncgenerics-mangling-2 branch from a07d9eb to 0d19444 Compare March 5, 2024 07:05

kavon marked this pull request as ready for review March 5, 2024 07:07

kavon requested review from hborla, xedin, ahoppen, bnbarham, CodaFi, hamishknight and rintaro as code owners March 5, 2024 07:07

kavon force-pushed the ncgenerics-mangling-2 branch from 0d19444 to 3f03eef Compare March 5, 2024 08:07

kavon force-pushed the ncgenerics-mangling-2 branch from 3f03eef to 184ec68 Compare March 5, 2024 17:55

kavon force-pushed the ncgenerics-mangling-2 branch from 184ec68 to 6ee6527 Compare March 5, 2024 20:56

kavon added 2 commits March 5, 2024 14:19

Mangling: handle inverse requirements

215bd3c

XFAIL tests now failing under noncopyable_generics

2a1cf37

After implementing mangling for inverses, issues have been revealed.

kavon force-pushed the ncgenerics-mangling-2 branch from 6ee6527 to 2a1cf37 Compare March 5, 2024 22:20

kavon enabled auto-merge March 5, 2024 22:20

kavon merged commit e2d33ec into swiftlang:main Mar 6, 2024

NCGenerics: New Inverse Mangling 3DS XL #71878

NCGenerics: New Inverse Mangling 3DS XL #71878

Uh oh!

Conversation

kavon commented Feb 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kavon commented Feb 26, 2024

Uh oh!

kavon commented Feb 26, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kavon commented Feb 27, 2024

Uh oh!

kavon commented Feb 28, 2024

Uh oh!

kavon commented Mar 2, 2024

Uh oh!

kavon commented Mar 2, 2024

Uh oh!

Uh oh!

DougGregor Mar 4, 2024

Choose a reason for hiding this comment

Uh oh!

kavon Mar 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DougGregor Mar 4, 2024

Choose a reason for hiding this comment

Uh oh!

kavon Mar 4, 2024

Choose a reason for hiding this comment

Uh oh!

Azoy Mar 4, 2024

Choose a reason for hiding this comment

Uh oh!

slavapestov Mar 5, 2024

Choose a reason for hiding this comment

Uh oh!

DougGregor Mar 6, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kavon commented Mar 4, 2024

Uh oh!

kavon commented Mar 5, 2024

Uh oh!

kavon commented Mar 5, 2024

Uh oh!

kavon commented Mar 5, 2024

Uh oh!

kavon commented Mar 5, 2024

Uh oh!

kavon commented Mar 5, 2024

Uh oh!

kavon commented Mar 5, 2024

Uh oh!

kavon commented Mar 5, 2024

Uh oh!

kavon commented Mar 5, 2024

Uh oh!

kavon commented Mar 5, 2024

Uh oh!

kavon commented Mar 5, 2024

Uh oh!

kavon commented Mar 5, 2024

Uh oh!

kavon commented Mar 5, 2024

Uh oh!

kavon commented Mar 6, 2024

Uh oh!

Uh oh!

kavon commented Feb 26, 2024 •

edited

Loading

kavon Mar 4, 2024 •

edited

Loading