Skip to content

[4.2][CodeComplete] Strip out try and optional eval expr in operator completion #18245

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

rintaro
Copy link
Member

@rintaro rintaro commented Jul 26, 2018

Cherry-pick of #18209 reviewed by @benlangmuir

  • Explaination: foldSequence() may hoist up try expression and optional-evaluation-expression by mutating their sub expression. When completing operators, this behavior ruins reusability of operand, and used to cause excessively nested expression which leads to stack overflow. Since these expression doesn't affect completion, strip them out beforehand so we can reuse operands. Also, this change contains fix for a regression introduced in earlier change where infix operator were disappeared from completion results when leading sequence contains AssignExpr.
  • Scope: Affect code-completion regarding optional evaluation expression.
  • Issue: rdar://problem/42452085
  • Risk: Low.
  • Testing: Added regression test cases.
  • Reviewed By: Ben Langmuir ([CodeComplete] Strip out try and optional eval expo in operator completion #18209)

@rintaro
Copy link
Member Author

rintaro commented Jul 26, 2018

@swift-ci Please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - a6beda4bf71c1a58ce51d59b505ffec1dede2235

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - a6beda4bf71c1a58ce51d59b505ffec1dede2235

rintaro added 2 commits July 26, 2018 13:25
…lation

foldSequence() may hoist these expression up by mutating their sub
expression. When completing operators, this behavior ruins reusability
of operand. Since these expression doesn't affect completion, we can
strip them out.

rdar://problem/42452085
Follow-up to f577578. The same
treatment for ternary expression (IfExpr). Plus, fix a regression
introduced in f577578 where infix operators were disappeard from
results.
@rintaro rintaro force-pushed the 4.2-ide-compilation-rdar42452085 branch from a6beda4 to df5c4b5 Compare July 26, 2018 04:26
@rintaro
Copy link
Member Author

rintaro commented Jul 26, 2018

@swift-ci Please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - a6beda4bf71c1a58ce51d59b505ffec1dede2235

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - a6beda4bf71c1a58ce51d59b505ffec1dede2235

@rintaro
Copy link
Member Author

rintaro commented Jul 26, 2018

@swift-ci Please nominate

@akyrtzi akyrtzi merged commit 3990932 into swiftlang:swift-4.2-branch Jul 26, 2018
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