Skip to content

Conversation

bnbarham
Copy link
Contributor

Use .swiftsourceinfo to include location information for results from external modules. Also add line and column so that clients don't need to map them from the offset.

@bnbarham bnbarham requested a review from benlangmuir April 29, 2021 21:31
@bnbarham
Copy link
Contributor Author

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 3715c9ddce734fa21d1f9f439c4e9206841ae2c8

@bnbarham
Copy link
Contributor Author

@swift-ci please test Linux platform

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 3715c9ddce734fa21d1f9f439c4e9206841ae2c8

@bnbarham bnbarham force-pushed the add-external-locs branch from 3715c9d to 14ca181 Compare April 30, 2021 01:20
@bnbarham
Copy link
Contributor Author

Added target triple to sourcekitd requests in the multi module test.

@swift-ci please test

@bnbarham
Copy link
Contributor Author

I forgot to check the symbol graph result in the multi-module test. Adding that now but the comments aren't showing up - so this isn't ready to merge yet!

bnbarham added 3 commits May 1, 2021 09:35
.swiftsourceinfo contains comment ranges, so parsing should attach
comments when it is being emitted. Generally this turned out to be fine
because .swiftdoc is usually output as well, but when calling
swift-frontend directly in tests that isn't necessarily the case.
.swiftsourceinfo files contain the serialized location for declarations.
Use this when outputting locations in cursor info so that clients need
not perform an extra index lookup for external modules.
Add line/column in addition to offset so clients need not map it
themselves.
@bnbarham bnbarham force-pushed the add-external-locs branch from 14ca181 to 0139fed Compare May 1, 2021 00:00
@bnbarham
Copy link
Contributor Author

bnbarham commented May 1, 2021

Turned out we weren't attaching comments when emitting only a .swiftsourceinfo. I've changed that to now attach them (in practice it doesn't really matter, usually .swiftdoc would be output which would attach comments). @nkcsgexi was not attaching them intentional?

@swift-ci please test

@bnbarham bnbarham requested a review from nkcsgexi May 1, 2021 00:06
@swift-ci
Copy link
Contributor

swift-ci commented May 1, 2021

Build failed
Swift Test Linux Platform
Git Sha - 0139fed855a706fdc14275113edd6be74c44e917

Have SourceKit return locations for symbols outside of the current
module as well. Callsites of location and comment information should
explicitly disable retrieving serialized information where performance
is a concern.

Resolves rdar://75582627
@bnbarham bnbarham force-pushed the add-external-locs branch from 0139fed to 73d9f5b Compare May 1, 2021 01:37
@bnbarham
Copy link
Contributor Author

bnbarham commented May 1, 2021

Windows failure was \ vs /.

@swift-ci please test

@swift-ci
Copy link
Contributor

swift-ci commented May 1, 2021

Build failed
Swift Test OS X Platform
Git Sha - 0139fed855a706fdc14275113edd6be74c44e917

@swift-ci
Copy link
Contributor

swift-ci commented May 1, 2021

Build failed
Swift Test OS X Platform
Git Sha - 73d9f5b

@bnbarham
Copy link
Contributor Author

bnbarham commented May 1, 2021

@swift-ci please test macOS platform

@swift-ci
Copy link
Contributor

swift-ci commented May 1, 2021

Build failed
Swift Test OS X Platform
Git Sha - 73d9f5b

@bnbarham
Copy link
Contributor Author

bnbarham commented May 1, 2021

Another spurious (but different) failure

@swift-ci please test macOS platform

@swift-ci
Copy link
Contributor

swift-ci commented May 1, 2021

Build failed
Swift Test OS X Platform
Git Sha - 73d9f5b

@bnbarham
Copy link
Contributor Author

bnbarham commented May 3, 2021

@swift-ci please test macOS platform

@bnbarham
Copy link
Contributor Author

bnbarham commented May 3, 2021

@swift-ci please test Windows platform

@bnbarham bnbarham merged commit 415452e into swiftlang:main May 3, 2021
@bnbarham bnbarham deleted the add-external-locs branch May 3, 2021 23:49
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