-
Notifications
You must be signed in to change notification settings - Fork 13.7k
print raw lifetime idents with r# #144897
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
print raw lifetime idents with r# #144897
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
This PR modifies |
compiler/rustc_span/src/symbol.rs
Outdated
if let Some(ident) = self.as_raw_lifetime_name_without_apostrophe() { | ||
write!(f, "'{}", IdentPrinter::new(ident.name, true, None)) | ||
} else { | ||
fmt::Display::fmt(&IdentPrinter::new(self.name, self.is_raw_guess(), None), f) | ||
} |
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 is this not a change in IdentPrinter
instead?
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.
How do you propose us to make the check in IdentPrinter
? It doesn't know about editions because it only accepts the symbol. is_raw_guess
and as_raw_lifetime_name_without_apostrophe
both need the span edition to know whether some identifier is treated as a keyword or not.
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 could accept edition as a parameter? otherwise I'm not sure what's even the point of it
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.
otherwise I'm not sure what's even the point of it
Just looked at the code again. IdentPrinter is used from two general places. One where one has the token data (sorta at the parsing/ast/proc macro stage) in which rawness of idents are known and do not need to be guessed, and one at a later stage where they do. that's why rawness was originally passed as a bool. To improve the design as you highlighted I chose to change the original bool into an enum, but we still have to guess whether a raw lifetime should be printed here and not inside IdentPrinter.
This comment has been minimized.
This comment has been minimized.
5805f0a
to
9705706
Compare
9705706
to
b74a97e
Compare
This comment was marked as resolved.
This comment was marked as resolved.
b74a97e
to
4d45896
Compare
This comment was marked as resolved.
This comment was marked as resolved.
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.
Thanks! r=me with nits addressed in one way or another
@@ -3090,7 +3090,13 @@ impl<'a> Parser<'a> { | |||
if let Some((ident, is_raw)) = self.token.lifetime() { | |||
// Disallow `'fn`, but with a better error message than `expect_lifetime`. | |||
if matches!(is_raw, IdentIsRaw::No) && ident.without_first_quote().is_reserved() { | |||
self.dcx().emit_err(errors::InvalidLabel { span: ident.span, name: ident.name }); | |||
self.dcx().emit_err(errors::InvalidLabel { |
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.
alternatively, we could just mimic the diagnostic emitted by expect_lifetime
("lifetimes cannot use keyword names") and just say "labels cannot use keyword names"
compiler/rustc_span/src/symbol.rs
Outdated
// this should be kept consistent with `Parser::expect_lifetime` found under | ||
// compiler/rustc_parse/src/parser/ty.rs | ||
let name_without_apostrophe = self.without_first_quote(); | ||
name_without_apostrophe.name != self.name | ||
&& ![kw::UnderscoreLifetime, kw::StaticLifetime].contains(&self.name) | ||
&& name_without_apostrophe.is_raw_guess() |
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.
Since rustc_span
actually defines Ident
and is_reserved
, you could reasonably also define a canonical "is_reserved_lifetime
" which you can then reuse.
4d45896
to
f3ff3d3
Compare
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
This comment has been minimized.
This comment has been minimized.
f3ff3d3
to
4970127
Compare
@bors r=fmease |
…inting, r=fmease print raw lifetime idents with r# This replaces rust-lang#143185 and fixes rust-lang#143150 cc `@fmease`
Rollup of 20 pull requests Successful merges: - #132087 (Fix overly restrictive lifetime in `core::panic::Location::file` return type) - #137396 (Recover `param: Ty = EXPR`) - #142185 (Convert moves of references to copies in ReferencePropagation) - #144443 (Make target pointer width in target json an integer) - #144648 (Implementation: `#[feature(nonpoison_rwlock)]`) - #144897 (print raw lifetime idents with r#) - #145218 ([Debuginfo] improve enum value formatting in LLDB for better readability) - #145380 (Add codegen-llvm regression tests) - #145573 (Add an experimental unsafe(force_target_feature) attribute.) - #145597 (resolve: Remove `ScopeSet::Late`) - #145641 (On E0277, point at type that doesn't implement bound) - #145669 (rustdoc-search: GUI tests check for `//` in URL) - #145695 (Introduce ProjectionElem::try_map.) - #145710 (Fix the ABI parameter inconsistency issue in debug.rs for LoongArch64) - #145726 (Experiment: Reborrow trait) - #145731 (Make raw pointers work in type-based search) - #145736 (triagebot: Update style team reviewers) - #145738 (Uplift rustc_mir_transform::coverage::counters::union_find to rustc_data_structures.) - #145743 (doc: fix some typos in comment) - #145745 (tests: Ignore basic-stepping.rs on LoongArch) Failed merges: - #145670 (port `sanitize` attribute to the new parsing infrastructure) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 28 pull requests Successful merges: - #132087 (Fix overly restrictive lifetime in `core::panic::Location::file` return type) - #137396 (Recover `param: Ty = EXPR`) - #137457 (Fix host code appearing in Wasm binaries) - #142185 (Convert moves of references to copies in ReferencePropagation) - #144648 (Implementation: `#[feature(nonpoison_rwlock)]`) - #144897 (print raw lifetime idents with r#) - #145218 ([Debuginfo] improve enum value formatting in LLDB for better readability) - #145380 (Add codegen-llvm regression tests) - #145573 (Add an experimental unsafe(force_target_feature) attribute.) - #145597 (resolve: Remove `ScopeSet::Late`) - #145633 (Fix some typos in LocalKey documentation) - #145641 (On E0277, point at type that doesn't implement bound) - #145669 (rustdoc-search: GUI tests check for `//` in URL) - #145695 (Introduce ProjectionElem::try_map.) - #145710 (Fix the ABI parameter inconsistency issue in debug.rs for LoongArch64) - #145726 (Experiment: Reborrow trait) - #145731 (Make raw pointers work in type-based search) - #145736 (triagebot: Update style team reviewers) - #145738 (Uplift rustc_mir_transform::coverage::counters::union_find to rustc_data_structures.) - #145742 (rustdoc js: Even more typechecking improvments ) - #145743 (doc: fix some typos in comment) - #145745 (tests: Ignore basic-stepping.rs on LoongArch) - #145747 (Refactor lint buffering to avoid requiring a giant enum) - #145751 (fix(lexer): Allow '-' in the frontmatter infostring continue set) - #145761 (Add aarch64_be-unknown-hermit target) - #145762 (convert strings to symbols in attr diagnostics) - #145763 (Ship LLVM tools for the correct target when cross-compiling) - #145765 (Revert suggestions for missing methods in tuples) r? `@ghost` `@rustbot` modify labels: rollup
This replaces #143185 and fixes #143150
cc @fmease