Skip to content

Add new Literal::str_value, Literal::cstr_value and Literal::byte_str_value methods #504

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
+119 −0
Closed

Add new Literal::str_value, Literal::cstr_value and Literal::byte_str_value methods #504

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ features = ["span-locations"]
[dependencies]
unicode-ident = "1.0"

[target.'cfg(procmacro2_semver_exempt)'.dependencies]
rustc-literal-escaper = "0.0.5"
Copy link
Owner

@dtolnay dtolnay Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This dependency's compiler version support is not compatible with making it a dependency of proc-macro2. Also it has unreasonably many publishers for something that would become a widely used dependency. Neither of these factors is necessarily immediately disqualifying while the dependency is gated by our semver exempt cfg, but they mean that this implementation is not going to work once it comes time to stabilize the new methods. I would prefer going directly to a different implementation that does not involve adding this dependency.


[dev-dependencies]
flate2 = "1.0"
quote = { version = "1.0", default-features = false }
Expand Down
116 changes: 116 additions & 0 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@
#![cfg_attr(any(proc_macro_span, super_unstable), feature(proc_macro_span))]
#![cfg_attr(super_unstable, feature(proc_macro_def_site))]
#![cfg_attr(docsrs, feature(doc_cfg))]
#![cfg_attr(
all(procmacro2_semver_exempt, feature = "proc-macro"),
feature(proc_macro_value)
Copy link
Owner

@dtolnay dtolnay Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not worth introducing the use of an unstable feature for this API. That makes sense for things like Span::def_site and Literal::subspan where a stable implementation is impossible. But for the methods in this PR they should just follow the stable "fallback" implementation in both cases, without being coupled to the API in nightly.

)]
#![deny(unsafe_op_in_unsafe_fn)]
#![allow(
clippy::cast_lossless,
Expand Down Expand Up @@ -178,6 +182,11 @@ use std::ffi::CStr;
#[cfg(span_locations)]
use std::path::PathBuf;

#[cfg(all(procmacro2_semver_exempt, feature = "proc-macro"))]
use proc_macro::ConversionErrorKind;
#[cfg(all(procmacro2_semver_exempt, feature = "proc-macro"))]
use rustc_literal_escaper::MixedUnit;

#[cfg(span_locations)]
#[cfg_attr(docsrs, doc(cfg(feature = "span-locations")))]
pub use crate::location::LineColumn;
Expand Down Expand Up @@ -1271,6 +1280,113 @@ impl Literal {
pub unsafe fn from_str_unchecked(repr: &str) -> Self {
Literal::_new(unsafe { imp::Literal::from_str_unchecked(repr) })
}

/// Returns the unescaped string value if the current literal is a string or a string literal.
Copy link
Owner

@dtolnay dtolnay Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not clear to me what distinction this is making between a literal that is a string vs a string literal.

#[cfg(all(procmacro2_semver_exempt, feature = "proc-macro"))]
Copy link
Owner

@dtolnay dtolnay Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This cannot be conditional on feature = "proc-macro". We need to provide methods that work identically whether or not proc-macro is being used.

pub fn str_value(&self) -> Result<String, ConversionErrorKind> {
match self.inner {
imp::Literal::Compiler(ref compiler_lit) => compiler_lit.str_value(),
imp::Literal::Fallback(ref fallback) => {
if !fallback.repr.starts_with('"') {
return Err(ConversionErrorKind::InvalidLiteralKind);
Comment on lines +1290 to +1291
Copy link
Owner

@dtolnay dtolnay Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the intended behavior on raw string literals?

}
let mut error = None;
let mut buf = String::with_capacity(fallback.repr.len());
rustc_literal_escaper::unescape_str(&fallback.repr, |_, res| match res {
Copy link
Owner

@dtolnay dtolnay Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the documentation of this function it "takes the contents of a string literal (without quotes)".

Please add a test that would catch this.

Ok(c) => buf.push(c),
Err(err) => {
if err.is_fatal() {
// `proc_macro::EscapeError` is the reexport of
// `rustc_literal_escaper::EscapeError` so we safely transmute between
// the two.
error = Some(ConversionErrorKind::FailedToUnescape(unsafe {
std::mem::transmute(err)
Comment on lines +1302 to +1303
Copy link
Owner

@dtolnay dtolnay Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This unsafe transmute is unsound. In general the version of rustc-literal-escaper in the proc-macro2 user's dependency graph is going to be different than the version of rustc-literal-escaper compiled into the standard library.

}));
}
}
});
if let Some(error) = error {
Err(error)
} else {
Ok(buf)
}
}
}
}

/// Returns the unescaped string value if the current literal is a c-string or a c-string
/// literal.
#[cfg(all(procmacro2_semver_exempt, feature = "proc-macro"))]
pub fn cstr_value(&self) -> Result<Vec<u8>, ConversionErrorKind> {
match self.inner {
imp::Literal::Compiler(ref compiler_lit) => compiler_lit.cstr_value(),
imp::Literal::Fallback(ref fallback) => {
if !fallback.repr.starts_with('c') {
return Err(ConversionErrorKind::InvalidLiteralKind);
}
let mut error = None;
let mut buf = Vec::with_capacity(fallback.repr.len());

rustc_literal_escaper::unescape_c_str(&fallback.repr, |_span, res| match res {
Ok(MixedUnit::Char(c)) => {
buf.extend_from_slice(c.get().encode_utf8(&mut [0; 4]).as_bytes())
}
Ok(MixedUnit::HighByte(b)) => buf.push(b.get()),
Err(err) => {
if err.is_fatal() {
// `proc_macro::EscapeError` is the reexport of
// `rustc_literal_escaper::EscapeError` so we safely transmute between
// the two.
error = Some(ConversionErrorKind::FailedToUnescape(unsafe {
std::mem::transmute(err)
}));
}
}
});
if let Some(error) = error {
Err(error)
} else {
buf.push(0);
Ok(buf)
}
}
}
}

/// Returns the unescaped string value if the current literal is a byte string or a byte string
/// literal.
#[cfg(all(procmacro2_semver_exempt, feature = "proc-macro"))]
pub fn byte_str_value(&self) -> Result<Vec<u8>, ConversionErrorKind> {
match self.inner {
imp::Literal::Compiler(ref compiler_lit) => compiler_lit.byte_str_value(),
imp::Literal::Fallback(ref fallback) => {
if !fallback.repr.starts_with('c') {
Copy link
Owner

@dtolnay dtolnay Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong prefix for a byte string.

Please add a test that would catch this.

return Err(ConversionErrorKind::InvalidLiteralKind);
}
let mut error = None;
let mut buf = Vec::with_capacity(fallback.repr.len());

rustc_literal_escaper::unescape_byte_str(&fallback.repr, |_span, res| match res {
Ok(c) => buf.push(c),
Err(err) => {
if err.is_fatal() {
// `proc_macro::EscapeError` is the reexport of
// `rustc_literal_escaper::EscapeError` so we safely transmute between
// the two.
error = Some(ConversionErrorKind::FailedToUnescape(unsafe {
std::mem::transmute(err)
}));
}
}
});
if let Some(error) = error {
Err(error)
} else {
Ok(buf)
}
}
}
}
}

impl FromStr for Literal {
Expand Down
Loading