Skip to content
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

Create a new module and SourceFile for REPL completion #29664

Merged
merged 3 commits into from Feb 6, 2020

Conversation

hamishknight
Copy link
Collaborator

Rather than attempting to temporarily insert decls into the last source file, just create a new module and source file and carry across the imports from the last module. This matches how the REPL deals with new lines of input.

Resolves SR-12076.
Resolves rdar://problem/58860034.

We were previously doing this for the REPL, but
not for swift-ide-test. Move the assignment into
the frontend to make sure its always applied, and
inline `createREPLFile` while we're here.
Rather than attempting to temporarily insert decls
into the last source file, just create a new module
and source file and carry across the imports from
the last module. This matches how the REPL deals
with new lines of input.
Now that REPL code completion no longer injects
temporary decls into source files, this parameter
is no longer needed.
@hamishknight
Copy link
Collaborator Author

@swift-ci please test

@hamishknight
Copy link
Collaborator Author

apple/llvm-project#733

@swift-ci please test

Copy link
Member

@CodaFi CodaFi left a comment

Choose a reason for hiding this comment

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

:shipit:

@hamishknight hamishknight merged commit 2b85835 into apple:master Feb 6, 2020
@hamishknight hamishknight deleted the inout-decls branch February 6, 2020 21:43
hamishknight added a commit to apple/llvm-project that referenced this pull request Feb 6, 2020
hamishknight added a commit to apple/llvm-project that referenced this pull request Feb 6, 2020
hamishknight added a commit to apple/llvm-project that referenced this pull request Feb 6, 2020
marcrasi pushed a commit to marcrasi/swift that referenced this pull request Feb 6, 2020
Create a new module and SourceFile for REPL completion
marcrasi pushed a commit to marcrasi/swift that referenced this pull request Feb 7, 2020
Create a new module and SourceFile for REPL completion
marcrasi pushed a commit to apple/llvm-project that referenced this pull request Feb 7, 2020
…fix-to-tensorflow

This is the main PR fixing https://bugs.swift.org/browse/SR-12076 in swift-jupyter. There are three parts to the fix:
* A PR into apple/swift cherry-picking fixes apple/swift#29619 and apple/swift#29664 from master: apple/swift#29707.
* This PR cherry-picks the #712 fix from apple:swift/master llvm-project.
* This PR also makes additional fixes in SwiftCodeCompletion.cpp, which is a custom thing in the `tensorflow` branch of llvm-project.

**Background on SwiftCodeCompletion.cpp**

The existing REPL code completion implementation ([REPLCodeCompletion.cpp](https://github.com/apple/swift/blob/167fea2c78ba334c8248dc05d32e50ccb848122f/lib/IDE/REPLCodeCompletion.cpp)) had some problems making it unsuitable for implementing code completion in swift-jupyter:
* It didn't handle redeclarations correctly.
* It didn't have an API that was callable from Python.

So I duplicated it into `SwiftCodeCompletion.cpp` and made improvements.

I'm planning to upstream these improvements soon so that we don't have this duplicate implementation, but for now we have it.

Whenever someone makes an important change in `REPLCodeCompletion.cpp`, we need to make the corresponding change in `SwiftCodeCompletion.cpp`.

**This PR's fixes to SwiftCodeCompletion.cpp**

apple/swift#29664 fixes https://bugs.swift.org/browse/SR-12076 for `REPLCodeCompletion.cpp` by changing it from reusing an existing source file to creating a new source file for every completion request.

So this PR makes a corresponding change in `SwiftCodeCompletion.cpp`, making it create new source files for every request. This actually simplifies the logic in `SwiftCodeCompletion.cpp` quite a lot, because we don't have to deal with existing state in the files.

Also, while making this change, I noticed that `SwiftCodeCompletion.cpp` wastefully parsed and typechecked a file merely to calculate the completion offset (see explanation in doc comment `FindCompletionPrefixOffset` for explanation of what completion offset it). So I switched it to calculate the completion offset by looking at the characters, inspired by logic in [sourcekit-lsp](https://github.com/apple/sourcekit-lsp/blob/9b8e4c714651ac56dea4c268650b7e68fb918814/Sources/SourceKit/sourcekitd/SwiftLanguageServer.swift#L537).
marcrasi pushed a commit to marcrasi/llvm-project that referenced this pull request Feb 7, 2020
…lete-fix-to-tensorflow

This is the main PR fixing https://bugs.swift.org/browse/SR-12076 in swift-jupyter. There are three parts to the fix:
* A PR into apple/swift cherry-picking fixes apple/swift#29619 and apple/swift#29664 from master: apple/swift#29707.
* This PR cherry-picks the apple#712 fix from apple:swift/master llvm-project.
* This PR also makes additional fixes in SwiftCodeCompletion.cpp, which is a custom thing in the `tensorflow` branch of llvm-project.

**Background on SwiftCodeCompletion.cpp**

The existing REPL code completion implementation ([REPLCodeCompletion.cpp](https://github.com/apple/swift/blob/167fea2c78ba334c8248dc05d32e50ccb848122f/lib/IDE/REPLCodeCompletion.cpp)) had some problems making it unsuitable for implementing code completion in swift-jupyter:
* It didn't handle redeclarations correctly.
* It didn't have an API that was callable from Python.

So I duplicated it into `SwiftCodeCompletion.cpp` and made improvements.

I'm planning to upstream these improvements soon so that we don't have this duplicate implementation, but for now we have it.

Whenever someone makes an important change in `REPLCodeCompletion.cpp`, we need to make the corresponding change in `SwiftCodeCompletion.cpp`.

**This PR's fixes to SwiftCodeCompletion.cpp**

apple/swift#29664 fixes https://bugs.swift.org/browse/SR-12076 for `REPLCodeCompletion.cpp` by changing it from reusing an existing source file to creating a new source file for every completion request.

So this PR makes a corresponding change in `SwiftCodeCompletion.cpp`, making it create new source files for every request. This actually simplifies the logic in `SwiftCodeCompletion.cpp` quite a lot, because we don't have to deal with existing state in the files.

Also, while making this change, I noticed that `SwiftCodeCompletion.cpp` wastefully parsed and typechecked a file merely to calculate the completion offset (see explanation in doc comment `FindCompletionPrefixOffset` for explanation of what completion offset it). So I switched it to calculate the completion offset by looking at the characters, inspired by logic in [sourcekit-lsp](https://github.com/apple/sourcekit-lsp/blob/9b8e4c714651ac56dea4c268650b7e68fb918814/Sources/SourceKit/sourcekitd/SwiftLanguageServer.swift#L537).
marcrasi pushed a commit to marcrasi/swift that referenced this pull request Feb 7, 2020
Create a new module and SourceFile for REPL completion
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.

None yet

3 participants