Skip to content

[5.7] Runtime Casting for Extended Existentials #59511

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jun 17, 2022

Conversation

CodaFi
Copy link
Contributor

@CodaFi CodaFi commented Jun 16, 2022

Cherry pick of #59455 and #59507

Implement the runtime entry points for casting to and from constrained existential types.

rdar://92197049

CodaFi added 7 commits June 16, 2022 17:17
Implement casting to and from extended existentials. This is done by slightly generalizing the conditional conformances checking infrastructure.

Unfortunately, casts for reference types and metatypes are unsound because IRGen is peepholing all non-opaque existential conversions with a helper. I’ll disable that in a follow-up.

rdar://92197049
The fix here is two-fold:

1) Teach SILGen that it cannot use the scalar casting paths for extended existentials
2) Teach the runtime casting entrypoint to unwrap as much metatype structure as possible
   before arriving at a 'Self' type bound for the requirement checking paths.

The code here mirrors the destructuring check we're doing in remote mirrors.

rdar://95166916
Return a NULL demangle tree instead of crashing. When an unconditional
runtime cast fails, it's going to crash anyways, but this way it prints
a better description (though not great) than the unreachable's message here.
Ensure the last node after all of the destructuring really
is a dependent generic param type. If it isn't, something has
gone horribly wrong.
@CodaFi CodaFi added the r5.7 label Jun 16, 2022
@CodaFi CodaFi requested a review from a team as a code owner June 16, 2022 23:19
@CodaFi
Copy link
Contributor Author

CodaFi commented Jun 16, 2022

@swift-ci test

@CodaFi CodaFi requested a review from rjmccall June 16, 2022 23:35
Copy link
Contributor

@tbkka tbkka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't comment on the extended existential type theory, but the overall runtime dynamic casting structure looks good.

@@ -0,0 +1,114 @@
// ParameterizedExistentials.swift - Tests for class constant casts w/ Obj-C
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the summary here need to be updated?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expectEqual(a.value, b.value)
let c = GenericHolder(value: 5) as? any Holder<Int>
expectNotNil(c)
let d = GenericHolder(value: 5) as? any Holder<String>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see you've tried to exercise as, as!, and as?. Some of the other casting tests also use this function, which seems to reliably defeat compiler optimizations and forces use of the most-general casting path. It may not be needed for this case, since these casts don't seem to have an alternate runtime path at current.

fileprivate func runtimeCast<From, To> (_ x: From, to: To.Type) -> To? {
  return x as? To
}

@@ -1695,10 +1695,56 @@ tryCastUnwrappingExistentialSource(
return tryCast(destLocation, destType,
srcInnerValue, srcInnerType,
destFailureType, srcFailureType,
takeOnSuccess & (srcInnerValue == srcValue),
takeOnSuccess && (srcInnerValue == srcValue),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch!

srcFailureType = srcInnerType;
return tryCast(destLocation, destType, srcInnerValue, srcInnerType,
destFailureType, srcFailureType,
takeOnSuccess & (srcInnerValue == srcValue), mayDeferChecks);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be && here as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be! @rjmccall caught this the first time around. I'll patch this out on main.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return tryCastToObjectiveCClass;
case MetadataKind::ExistentialMetatype:
return tryCastToExistentialMetatype;
return tryCastToExistentialMetatype;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for fixing whitespace.

Copy link
Contributor

@tbkka tbkka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry! I just realized this is the cherry-pick. Looks good as-is. consider my earlier comments suggestions for a follow-up on main.

@CodaFi CodaFi merged commit 489508f into swiftlang:release/5.7 Jun 17, 2022
@CodaFi CodaFi deleted the strato-caster branch June 17, 2022 05:00
@AnthonyLatsis AnthonyLatsis added 🍒 release cherry pick Flag: Release branch cherry picks swift 5.7 labels Jan 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍒 release cherry pick Flag: Release branch cherry picks swift 5.7
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants