-
Notifications
You must be signed in to change notification settings - Fork 470
Respect #sourceLocation directives in SourceLocationConverter
#1827
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
Respect #sourceLocation directives in SourceLocationConverter
#1827
Conversation
|
@swift-ci Please test |
| /// is returned. If position is negative the location for start of file is | ||
| /// returned. |
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.
Why do we allow negative AbsolutePosition?
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 don’t think there’s a real use case. Would you prefer to assert or precondition? 🤔
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.
precondition seems fine to me
| presumedLine: Int? = nil, | ||
| presumedFile: String? = nil |
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.
Add to debug description?
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 am not super happy with the debug description as-is anyway because it also doesn’t include the file name and have been tempted to just remove it. WDYT?
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.
Or just extend it to include file as well 🤷
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.
Decided to remove debugDescription
The debug description was lossy because it didn’t print file and offset. If we include that information, it’s so verbose that the debug description doesn’t really provide any benefit over the default debug description. Let’s just remove it.
ca31078 to
282ba8d
Compare
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
282ba8d to
4ea691c
Compare
|
@swift-ci Please test |
Add a `presumedFile` and `presumedLine` property to `SourceLocation` that contains the file and line of the location while taking `#sourceLocation` directives into account. The terms “presumed file” and “presumed line” have been taken from LLVM and the Swift compiler. rdar://99187174
The debug description was lossy because it didn’t print file and offset. If we include that information, it’s so verbose that the debug description doesn’t really provide any benefit over the default debug description. Let’s just remove it.
4ea691c to
9c8e011
Compare
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
Add a
presumedFileandpresumedLineproperty toSourceLocationthat contains the file and line of the location while taking#sourceLocationdirectives into account.The terms “presumed file” and “presumed line” have been taken from LLVM and the Swift compiler.
rdar://99187174