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.
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
RFC: proc macro
include!
#3200base: master
Are you sure you want to change the base?
RFC: proc macro
include!
#3200Changes from all commits
7445c70
1d641b3
b4aa253
3029688
f1f4aca
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Would it make sense for
include_bytes
to returnLiteral
as well, or would that not be possible?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 think it should work because
Literal
can be a byte string.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.
Actually, yeah, I overlooked that possibility.
The main limitation is that the only current interface for getting the contents out of a
Literal
is toToString
it.syn
does have a.value()
forLitByteStr
as well asLitStr
, though, so I guess it's workable.It's probably not good to short term require debug escaping a binary file to reparse the byte string literal if a proc macro is going to post process the file... but if it's just including the literal, it can put the
Literal
in the token stream, and we can offer ways to extract (byte) string literals without printing the string literal in the future.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.
The one limitation which needs to be solved is how do spans work. Do we just say that the byte string literal contains the raw bytes of the file (even though that would be illegal in a normal byte string, and invalid UTF-8), maybe as a new "kind" of byte string, so span offsets are mapped directly with the source file? Or are there multiple span positions (representing a
\xNN
in the byte string) which map to a single byte in the source file?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.
Hmm, what bytes are not allowed in byte string literals? Does the literal itself have to be valid UTF-8?
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.
A Rust source file must be valid UTF-8. Thus, the contents of a byte string literal in the source must be valid UTF-8.
Bytes that are not < 0x80 thus must be escaped to appear in a byte string literal.
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.
And then another question that's worth making explicit: what does it even mean for rustc to report a span into a binary file?
I think binary includes are better served by a different API that lets rustc point into generated code, rather than trying to point into an opaque binary file.
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.
One way to support both options would be to take a
Span
that the path is relative to. Then it would make multi-level includes easier (the macro includes a path relative to the Rust source file, then the included file references another relative file so that needs to be included based on theSpan
from the firstproc_macro::include_str
call).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.
What would
Span::mixed_site
be relative to?Also, that would kinda soft-block the feature onthough I suppose requiring a span would be strictly more powerful thanSpan::def_site
, while the RFC is currently written such that additional unstable features (such as span subslicing) are incremental improvements not required for the functionality to be useful...include!
-style base path, so that fits into the same category.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 suppose it should just behave the exact same as a
include_str!("..")
macro invocation whose tokens carry a mixed_site span.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.
Somewhat surprisingly, this looks for a file called
"a"
relative to the file in whichx!()
is invoked, not relative to the file that contains the definition above.