Skip to content

[NFC][SILGen]: remove dead code in SILGenExpr#88429

Merged
jamieQ merged 1 commit intoswiftlang:mainfrom
jamieQ:silgen-expr-dead-code-or-bug
Apr 14, 2026
Merged

[NFC][SILGen]: remove dead code in SILGenExpr#88429
jamieQ merged 1 commit intoswiftlang:mainfrom
jamieQ:silgen-expr-dead-code-or-bug

Conversation

@jamieQ
Copy link
Copy Markdown
Contributor

@jamieQ jamieQ commented Apr 12, 2026

Removes the following two bits of code in SILGenExpr that were either dead or superfluous:

  1. Some pattern matching logic to dig out nested function conversions and explicitly apply a sendable conversion. Per the comments this was nominally done to avoid emitting multiple conversion thunks. However, I was unable to find a case that produced such thunks any longer, and additionally, the condition gating the logic always evaluated to false, so it's been removed.
  2. Removed a superfluous conditional branch in RValueEmitter::visitConsumeExpr.

  • Explanation: Removes some unnecessary code.
  • Scope: Removes two sections of logic in SILGenExpr that were either dead or superfluous.
  • Issues: N/A
  • Risk: Minimal risk. The change should not alter existing behavior.
  • Testing: Ran existing tests.
  • Reviewers: @slavapestov

@jamieQ
Copy link
Copy Markdown
Contributor Author

jamieQ commented Apr 12, 2026

@swift-ci smoke test

Copy link
Copy Markdown
Contributor Author

@jamieQ jamieQ left a comment

Choose a reason for hiding this comment

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

@gottesmm this is probably very low priority, but I'd like your opinion on whether these deletions are the right fixes or if there are more appropriate changes that should be made. Thanks!

return RValue(SGF, {value}, subType.getASTType());
ManagedValue value =
SGF.B.createLoadTake(E, optTemp->getManagedAddress());
return RValue(SGF, {value}, subType.getASTType());
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Want to confirm that deleting the condition here is the correct fix, and there's not something else we should be emitting in one of the branches. In the case below (~L7300-7312) the codegen looks more complex in the nontrivial branch.

Comment on lines -2034 to -2035
// The reason why we need to evaluate this especially is that otherwise we
// generate multiple conversion thunks.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I tried to figure out if this was still happening, but I couldn't find a case that reproduced this behavior. Maybe it was fixed in #86729?

Comment on lines -2043 to -2046
if (subExprType->hasExtInfo() && subExprType->getExtInfo().isSendable() &&
subSubExprType->hasExtInfo() &&
!subExprType->getExtInfo().isSendable() &&
subExprType->withSendable(true) == subSubExprType) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is the reason why this code is effectively dead – the conjunction contains both subExprType->getExtInfo().isSendable() and !subExprType->getExtInfo().isSendable() (presumably this was a typo and the second one was supposed to be subSubExprType).

Does removal seem like the most appropriate change here, or would fixing the logic error be better?

@jamieQ jamieQ marked this pull request as ready for review April 14, 2026 12:24
@jamieQ jamieQ requested a review from a team as a code owner April 14, 2026 12:24
@jamieQ jamieQ requested a review from gottesmm April 14, 2026 12:24
Copy link
Copy Markdown
Contributor

@slavapestov slavapestov left a comment

Choose a reason for hiding this comment

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

I'm fine with deleting dead code that is not covered by tests.

@jamieQ jamieQ merged commit a8d11f5 into swiftlang:main Apr 14, 2026
3 checks passed
@jamieQ jamieQ deleted the silgen-expr-dead-code-or-bug branch April 14, 2026 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants