Serialize package and SPI doc comments in swiftsourceinfo
#65265
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hamishknight
requested review from
ahoppen,
bnbarham,
rintaro,
hborla,
slavapestov and
xedin
as code owners
bnbarham
reviewed
Apr 19, 2023
hamishknight
force-pushed
the
whats-up-doc
branch
2 times, most recently
from
5b453be to
a92da15
Compare
hamishknight
commented
Apr 19, 2023
Comment on lines
-139
to
-143
| if (Optional<std::pair<RawComment, bool>> RC = Context.getRawComment(this)) { | ||
| auto P = RC.value(); | ||
| if (!SerializedOK || P.second) | ||
| return P.first; | ||
| } |
There was a problem hiding this comment.
Heh, just noticed this predicate was wrong anyway, and allowed us to retrieve a serialized entry from the cache even if !SerializedOK.
ahoppen
approved these changes
Apr 20, 2023
hamishknight
force-pushed
the
whats-up-doc
branch
2 times, most recently
from
30b2bbf to
575ae2b
Compare
hamishknight
commented
Apr 21, 2023
hamishknight
force-pushed
the
whats-up-doc
branch
from
575ae2b to
754f032
Compare
QuietMisdreavus
approved these changes
Apr 24, 2023
hamishknight
force-pushed
the
whats-up-doc
branch
from
754f032 to
b0fd7d1
Compare
bnbarham
approved these changes
Apr 25, 2023
Comment on lines
+360
to
+362
| // Skip the decl if it does not have a comment. Note this means | ||
| // we'll only serialize "direct" brief comments, but that's okay | ||
| // because clients can compute the semantic brief comment themselves. |
There was a problem hiding this comment.
Seems weird that we output the brief comment to me at all. @nkcsgexi do you remember why this is the case?
|
oh, this is a great idea to include internal doc comment in the source info file! |
It doesn't seem like there's any client that's actually taking advantage of setting it to `false`, and its default value of `false` is more likely than not going to cause clients to accidentally miss comments that they may want. In fact, this was exactly the case for code completion's brief field. Finally, the parameter wasn't even consistently applied, as we would attempt to deserialize swiftdoc comments even if it were `false`.
Turn these ASTContext cached computations into request evaluator cached computations.
Previously we were using the same set of conditions for serializing as for swiftdoc, so excluded them. However it's reasonable to have them in the swiftsourceinfo.
Match the format of the spi and package variants of these tests.
If we have both loaded a swiftdoc, and the decl we have should have had its doc comment serialized into it, we can check it without needing to fall back to the swiftsourceinfo. This requires a couple of refactorings: - Factoring out the `shouldIncludeDecl` logic into `getDocCommentSerializationTargetFor` for determining whether a doc comment should end up in the swiftdoc or not. - Factoring out `CommentProviderFinder` for searching for the doc providing comment decl for brief comments, in order to allow us to avoid querying the raw comment when searching for it. This has the added bonus of meaning we no longer need to fall back to parsing the raw comment for the brief comment if the comment is provided by another decl in the swiftdoc. This diff is best viewed without whitespace.
Make it clear that we will walk the comment providing decls if needed.
hamishknight
force-pushed
the
whats-up-doc
branch
from
b0fd7d1 to
be31c22
Compare
|
@swift-ci please test |
|
Happy to take care of further feedback in a follow-up |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Requestify raw and brief comment computation, and allow
packageand SPI doc comments to be serialized in swiftsourceinfo. In the future we may want to emit a separate private swiftdoc for SPI doc comments, but for now we can just include them in the swiftsourceinfo to at least allow them to be picked up for local builds.