-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
Strip comments from inlinable text when printing in swiftinterface
files
#69358
Conversation
swiftinterface
files
@swift-ci please smoke test |
swiftinterface
filesswiftinterface
files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’ve got another fun counterexample for you. The following is valid but if you remove the block comment, it becomes invalid.
let x = 1 /*
*/ let b = 2
@rintaro Could you also check this, in particular if you have any performance concerns? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
For full context, this code: @inlinable
public func hasComments() -> Bool {
/* comment! */ // comment!
#if NOT_PROVIDED
// comment!
return true
#endif
print(/*
comment!
*/"this should show up")
print(
// comment! don't mess up indentation!
"""
""")
#if compiler(>=5.3) // comment!
print(/*comment!*/"")
#endif
let x = 1/*
*/let y = 2
let a = 3
/* test */let b = 2
#sourceLocation(file: "if-configs.swift", line: 200)
#if !NOT_PROVIDED
// comment!
return/* comment! */true/* comment! */
#endif
} Will be written to a swiftinterface as: @inlinable public func hasComments() -> Bool {
print(
"this should show up")
print(
"""
""")
#if compiler(>=5.3)
print( "")
#endif
let x = 1
let y = 2
let a = 3
let b = 2
return true
} And bodies without comments pass through unchanged. |
28f8d0c
to
aa2b41d
Compare
@swift-ci please smoke test |
Unrelated to this PR, but I realized current |
@rintaro Uhhh, that's not great! We didn't support them when this code was originally written so it definitely needs to be updated. That's a secrecy concern. |
@harlanhaskins The problem is that AST doesn't have those information. We don't model |
I would be totally happy with |
This adds support in
InlinableText.cpp
for ignoring the ranges of comment tokens and stripping comments from swiftinterface files. Ideally we'd be able to use SwiftSyntax to read in these source ranges and just reprint them while skipping comment trivia, but that infrastructure isn't quite there yet.Resolves rdar://44318472