Non-copyable generics fixes #71241

slavapestov · 2024-01-30T15:43:29Z

This fixes some validation test failures when non-copyable generics are enabled. Should be NFC otherwise.

slavapestov · 2024-01-30T20:25:34Z

@swift-ci Please smoke test

slavapestov · 2024-01-31T00:22:16Z

@swift-ci Please smoke test

slavapestov · 2024-01-31T03:13:56Z

@swift-ci Please test source compatibility

kavon · 2024-01-31T21:47:41Z

lib/Sema/TypeCheckDeclPrimary.cpp

+    // Force creation of the generic signature.
+    (void) ED->getGenericSignature();
+    dumpGenericSignature(Ctx, ED);
+


I'm curious why we have to do this now.

We force certain requests in TypeCheckDeclPrimary just to ensure diagnostics are emitted before we go on to SILGen. (This isn't new, I just moved some code around so that the -debug-generic-signature behavior happens here instead of in TypeCheckGeneric.cpp.)

ahoppen

Only reviewed the code completion parts.

ahoppen · 2024-01-31T23:13:40Z

lib/IDE/CodeCompletionResultType.cpp

+        LLVM_DEBUG(llvm::dbgs() << "Adding conformed protocol "
+                                << Conformance->getProtocol()->getName()
+                                << "\n";);


Are these debug statements intentionally left?

They're only active when you run with -Xllvm -debug-only=CodeCompletionResultType

ahoppen · 2024-01-31T23:14:01Z

lib/IDE/CodeCompletionResultType.cpp

@@ -22,6 +22,9 @@ using namespace swift;
 using namespace ide;
 using TypeRelation = CodeCompletionResultTypeRelation;

+#define DEBUG_TYPE "CodeCompletionResultType"


Stupid question: What is DEBUG_TYPE used for?

ahoppen · 2024-01-31T23:17:10Z

lib/IDE/CodeCompletionResultType.cpp

  SmallVector<const USRBasedType *, 2> Supertypes;
  ;
  if (auto Nominal = Ty->getAnyNominal()) {
    if (auto *Proto = dyn_cast<ProtocolDecl>(Nominal)) {
      Proto->walkInheritedProtocols([&](ProtocolDecl *inherited) {
        if (Proto != inherited &&
-            !inherited->isSpecificProtocol(KnownProtocolKind::Sendable)) {
+            !inherited->isSpecificProtocol(KnownProtocolKind::Sendable) &&
+            !inherited->getInvertibleProtocolKind()) {


I’m not sure how the inverted protocols are modeled in the AST but shouldn’t we check if the specific conformance is inverted (ie. has ~) instead of checking whether the protocol is invertible?

We're just skipping these protocols entirely here because we get the wrong USRs for their ProtocolDecls. Once the USR mangling is fixed we can remove this hack.

ahoppen · 2024-01-31T23:17:45Z

lib/IDE/CodeCompletionResultType.cpp

+          // FIXME: Copyable and Escapable conformances are skipped because the
+          // USR mangling produces the type 'Any' for the protocol type.


Why does it produce Any for the protocol name?

Because we need to fix the mangling.

Ah, I see. Thanks for the clarification. Is the name mangling something we are planning to fix in the short term?

yes, because we need to distinguish Any (which is now formally <Self where Self: Copyable, Self: Escapable> but must mangle as before) from any ~Copyable (<Self where Self: Escapable>, which needs a new mangling invented) and any ~Escapable (<Self where Self: Copyable>, ditto). Right now we simply strip out Copyable and Escapable from existentials before mangling, which is not correct.

OK, great 👍🏽

….cpp

…lTypeDecl from a SynthesizedFileUnit

We want extensions to introduce default Copyable/Escapable just like other generic contexts, so that once Optional adopts ~Copyable, an `extension Optional` actually adds `Wrapped: Copyable` by default.

…checkConformance()

…kConformance()

…eckConformance()

slavapestov · 2024-02-01T02:59:48Z

@swift-ci Please smoke test

slavapestov force-pushed the ncgenerics-fixes branch 2 times, most recently from 369fae7 to a88a144 Compare January 30, 2024 20:24

slavapestov marked this pull request as ready for review January 30, 2024 20:26

slavapestov requested review from hborla, xedin, ahoppen, bnbarham, hamishknight, rintaro, zoecarver, hyp, egorzhdan and tshortli as code owners January 30, 2024 20:26

slavapestov requested review from kavon and removed request for xedin, rintaro, hyp, bnbarham, egorzhdan, ahoppen, hborla, zoecarver, hamishknight and tshortli January 30, 2024 20:26

slavapestov force-pushed the ncgenerics-fixes branch from a88a144 to 91adc2f Compare January 31, 2024 00:13

slavapestov requested a review from ktoso as a code owner January 31, 2024 00:13

slavapestov force-pushed the ncgenerics-fixes branch from 91adc2f to d5c6be4 Compare January 31, 2024 03:12

slavapestov requested a review from AnthonyLatsis as a code owner January 31, 2024 03:12

slavapestov force-pushed the ncgenerics-fixes branch from a7e0f9c to ccd7427 Compare January 31, 2024 20:11

kavon approved these changes Jan 31, 2024

View reviewed changes

ahoppen reviewed Jan 31, 2024

View reviewed changes

slavapestov added 4 commits January 31, 2024 18:40

AST: Introduce GenericSignature::getRequirementsWithInverses()

6ed553d

AST: Refactor ASTPrinter to use getRequirementsWithInverses()

45f31b4

AST: Refactor the mangler to use getRequirementsWithInverses()

8e7edb2

Sema: Move -debug-generic-signatures behavior to TypeCheckDeclPrimary…

2355cb4

….cpp

slavapestov force-pushed the ncgenerics-fixes branch from ccd7427 to 6a7f718 Compare January 31, 2024 23:42

slavapestov added 17 commits January 31, 2024 21:55

ASTPrinter: Refactor printing of RequirementSignatures

23b1690

SIL: Add some assertions

094328d

SILGen: Clean up substitution map construction to deal with Copyable

af3ab14

Sema: Simplify DerivedConformance::createBuiltinCall()

7ed5509

Sema: Debug messages for TypeCheckCircularity.cpp

4f2ad8f

Sema: We can end up in deriveConformanceForInvertible() with a Nomina…

7ae4cc6

…lTypeDecl from a SynthesizedFileUnit

Sema: We might be generic but not have parsed generic parameters

69e69a7

IDE: Skip inverse protocols in CodeCompletionResultType's walk

778e6d5

Distributed: Fixes for non-copyable generics

4a46717

Sema: Implement new behavior of extensions with non-copyable generics

9e24746

We want extensions to introduce default Copyable/Escapable just like other generic contexts, so that once Optional adopts ~Copyable, an `extension Optional` actually adds `Wrapped: Copyable` by default.

ClangImporter: Fixes for non-copyable generics

bbbfc62

Sema: Introduce ResolveValueWitnessesRequest

821c3a3

Sema: Refactor ResolveImplicitMemberRequest to not call TypeChecker::…

1bf5e1b

…checkConformance()

Sema: Refactor evaluateMembersRequest() to not call TypeChecker::chec…

28f2621

…kConformance()

Sema: Refactor computeLoweredProperties() to not call TypeChecker::ch…

528519c

…eckConformance()

Sema: Remove TypeChecker::checkConformance()

6951cc1

AST: Remove ProtocolDecl::getRequirementSignatureAsGenericSignature()

d8f85a3

slavapestov force-pushed the ncgenerics-fixes branch from 6a7f718 to d8f85a3 Compare February 1, 2024 02:57

slavapestov merged commit 80205ec into swiftlang:main Feb 1, 2024

vanvoorden mentioned this pull request Jun 20, 2024

Use OSSALifetimeCompletion in PredictableMemOpt #70477

Merged

		// FIXME: Copyable and Escapable conformances are skipped because the
		// USR mangling produces the type 'Any' for the protocol type.

Non-copyable generics fixes #71241

Non-copyable generics fixes #71241

Uh oh!

Conversation

slavapestov commented Jan 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

slavapestov commented Jan 30, 2024

Uh oh!

slavapestov commented Jan 31, 2024

Uh oh!

slavapestov commented Jan 31, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

slavapestov Feb 1, 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!

ahoppen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

slavapestov commented Feb 1, 2024

Uh oh!

Uh oh!

slavapestov commented Jan 30, 2024 •

edited

Loading

slavapestov Feb 1, 2024 •

edited

Loading