[Sema] SR-15807: Associated Type Inference fails across module boundaries #42293
Conversation
…nformance instead of trailingWhereClause
lib/AST/NameLookup.cpp
Outdated
| SelfBounds SelfBoundsFromGenericSignatureRequest::evaluate( | ||
| Evaluator &evaluator, const ExtensionDecl *extDecl) const { | ||
| SelfBounds result; | ||
| if (!extDecl->hasComputedGenericSignature()) |
There was a problem hiding this comment.
This is brittle because it assumes someone else forced the computation by calling getGenericSignature() first. We want to trigger request computation lazily and not write code that depends on other requests having been executed first. You can just remove this conditional.
| @@ -220,8 +220,8 @@ AssociatedTypeInference::inferTypeWitnessesViaValueWitnesses( | |||
| if (!checkConformance(proto)) | |||
| return false; | |||
|
|
|||
| // Now check any additional bounds on 'Self' from the where clause. | |||
| auto bounds = getSelfBoundsFromWhereClause(extension); | |||
There was a problem hiding this comment.
If the extension came from a source file, then it might still have a trailing where clause, so we still want to prefer to do it the old way since it avoids computing a generic signature and therefore it is faster. You can check if 'extension->getParentSourceFile() != nullptr' to determine which method to use.
test/Sema/sr15807.swift
Outdated
| @@ -0,0 +1,9 @@ | |||
| // RUN: %empty-directory(%t) | |||
There was a problem hiding this comment.
A more descriptive name for the test, or a comment in the source file that explains this is about where clause Self bounds across module boundaries, might be good.
…elf bounds in source file.
|
Thank you for the review and detailed explanation, I really appreciate it! Thanks for your code review! |
| @@ -68,6 +68,9 @@ SWIFT_REQUEST(NameLookup, SelfBoundsFromWhereClauseRequest, | |||
| SelfBounds(llvm::PointerUnion<const TypeDecl *, | |||
| const ExtensionDecl *>), | |||
| Uncached, NoLocationInfo) | |||
| SWIFT_REQUEST(NameLookup, SelfBoundsFromGenericSignatureRequest, | |||
| SelfBounds(const ExtensionDecl * extDecl), | |||
| Uncached, NoLocationInfo) | |||
There was a problem hiding this comment.
Can you try making this request and SelfBoundsFromWhereClauseRequest Cached? (In a separate PR, if you like)
There was a problem hiding this comment.
It is my pleasure! I will do it!
|
@swift-ci Please smoke test |
|
@swift-ci Please test source compatibility |
|
Linux smoke test fails because of |
| @@ -0,0 +1,16 @@ | |||
| // RUN: %empty-directory(%t) | |||
| // RUN: %target-swift-frontend -emit-module -o %t/ModuleA.swiftmodule %S/Inputs/where_clause_across_module_boundaries_module.swift | |||
| // RUN: %target-swift-frontend -I %t -emit-sil -verify %s | |||
There was a problem hiding this comment.
Although there is no formal convention, it is preferable to align the compilation mode with the scope of the test, in this case -typecheck (or %target-typecheck-verify-swift) rather than -emit-sil.
There was a problem hiding this comment.
Got it! Thanks for the review!
|
@swift-ci smoke test |
Summary
This PR fixes: SR-15807
When
ExtensionDeclis in the module file(created bydeserializeExtension) usinggetSelfBoundsFromWhereClausewill always get default value. BecauseextensionDecl->getTrailingWhereClausewill always benullptr.Solution
This PR tries to create another function
getSelfBoundsFromGenericSignatureto resolve this issue.getSelfBoundsFromGenericSignaturewill getSelfBoundsusing generic signature instead of trailingWhereClause.Thanks
This is my first contribution, please feel free to critique, and thanks for your code review 😄 .