Skip to content

[5.8] SILCombine: fix a crash when optimizing keypath-offset-of #62832

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

Conversation

eeckstein
Copy link
Contributor

@eeckstein eeckstein commented Jan 4, 2023

Need to use the lowered type instead of the original type.

rdar://103834325

Cherry-picked from #62862

@eeckstein eeckstein added the r5.8 label Jan 4, 2023
@eeckstein
Copy link
Contributor Author

@swift-ci test

@eeckstein eeckstein requested a review from jckarter January 4, 2023 16:47
// TODO: support lowering of the rootTy if it's not a legal SIL type in the
// first place, e.g. if it contains an AnyFunctionType.
if (!rootTy->isLegalSILType())
return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of skipping out on these cases, it's not that hard to handle them correctly. The lowered type of the ends of a keypath projection are always going to be the maximally-abstracted form of the formal type, so you can do

CanType rootTy = AI->getFunction()->getLoweredType(AbstractionPattern::getOpaque(),
    pattern->getRootType().subst(patternSubs)->getCanonicalType());

And that should work for all root types. Even if the formal type happens to be a valid lowered SIL type, it is still a category error to use a formal type without lowering, and there could remain problems from doing so.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, thanks!
-> #62862

Need to use the lowered type instead of the original type.

rdar://103834325
@eeckstein eeckstein force-pushed the fix-keypath-opt-crash-5.8 branch from f927a07 to 12e9aed Compare January 5, 2023 16:54
@eeckstein
Copy link
Contributor Author

@swift-ci test

@eeckstein eeckstein requested a review from jckarter January 5, 2023 16:57
@eeckstein eeckstein merged commit 5a0eb50 into swiftlang:release/5.8 Jan 6, 2023
@eeckstein eeckstein deleted the fix-keypath-opt-crash-5.8 branch January 6, 2023 16:13
@AnthonyLatsis AnthonyLatsis added 🍒 release cherry pick Flag: Release branch cherry picks swift 5.8 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants