Skip to content
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

enable PathBuf usage in proc_macro bridge, tracked::path #88909

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion compiler/rustc_expand/src/proc_macro_server.rs
Expand Up @@ -21,6 +21,7 @@ use rustc_span::{BytePos, FileName, MultiSpan, Pos, RealFileName, SourceFile, Sp
use pm::bridge::{server, TokenTree};
use pm::{Delimiter, Level, LineColumn, Spacing};
use std::ops::Bound;
use std::path;
use std::{ascii, panic};

trait FromInternal<T> {
Expand Down Expand Up @@ -406,7 +407,7 @@ impl server::FreeFunctions for Rustc<'_> {
}

fn track_path(&mut self, path: &str) {
self.sess.file_depinfo.borrow_mut().insert(Symbol::intern(path));
self.sess.file_depinfo.borrow_mut().insert(path::PathBuf::from(path));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
self.sess.file_depinfo.borrow_mut().insert(path::PathBuf::from(path));
self.sess.file_depinfo.borrow_mut().insert(PathBuf::from(path));

Nit (here and in the other files as well): types are usually imported directly, without parent module.

}
}

Expand Down
5 changes: 2 additions & 3 deletions compiler/rustc_interface/src/passes.rs
Expand Up @@ -599,9 +599,8 @@ fn write_out_deps(
// Account for explicitly marked-to-track files
// (e.g. accessed in proc macros).
let file_depinfo = sess.parse_sess.file_depinfo.borrow();
let extra_tracked_files = file_depinfo.iter().map(|path_sym| {
let path = PathBuf::from(&*path_sym.as_str());
let file = FileName::from(path);
let extra_tracked_files = file_depinfo.iter().map(|path| {
let file = FileName::from(path.clone());
escape_dep_filename(&file.prefer_local().to_string())
});
files.extend(extra_tracked_files);
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_session/src/parse.rs
Expand Up @@ -13,6 +13,7 @@ use rustc_span::hygiene::ExpnId;
use rustc_span::source_map::{FilePathMapping, SourceMap};
use rustc_span::{MultiSpan, Span, Symbol};

use std::path;
use std::str;

/// The set of keys (and, optionally, values) that define the compilation
Expand Down Expand Up @@ -134,7 +135,7 @@ pub struct ParseSess {
/// Environment variables accessed during the build and their values when they exist.
pub env_depinfo: Lock<FxHashSet<(Symbol, Option<Symbol>)>>,
/// File paths accessed during the build.
pub file_depinfo: Lock<FxHashSet<Symbol>>,
pub file_depinfo: Lock<FxHashSet<path::PathBuf>>,
/// All the type ascriptions expressions that have had a suggestion for likely path typo.
pub type_ascription_path_suggestions: Lock<FxHashSet<Span>>,
/// Whether cfg(version) should treat the current release as incomplete
Expand Down
2 changes: 2 additions & 0 deletions library/proc_macro/src/bridge/mod.rs
Expand Up @@ -15,6 +15,7 @@ use std::marker;
use std::mem;
use std::ops::Bound;
use std::panic;
use std::path;
use std::sync::atomic::AtomicUsize;
use std::sync::Once;
use std::thread;
Expand Down Expand Up @@ -361,6 +362,7 @@ mark_noop! {
LineColumn,
Spacing,
Bound<usize>,
path::PathBuf,
}

rpc_encode_decode!(
Expand Down
13 changes: 13 additions & 0 deletions library/proc_macro/src/bridge/rpc.rs
Expand Up @@ -5,6 +5,7 @@ use std::char;
use std::io::Write;
use std::num::NonZeroU32;
use std::ops::Bound;
use std::path;
use std::str;

pub(super) type Writer = super::buffer::Buffer<u8>;
Expand Down Expand Up @@ -246,6 +247,18 @@ impl<S> DecodeMut<'_, '_, S> for String {
}
}

impl<S> Encode<S> for path::PathBuf {
fn encode(self, w: &mut Writer, s: &mut S) {
self.to_str().expect("`PathBuf`s must be valid UTF-8 for now!").encode(w, s);
}
}

impl<S> DecodeMut<'_, '_, S> for path::PathBuf {
fn decode(r: &mut Reader<'_>, s: &mut S) -> Self {
path::PathBuf::from(<&str>::decode(r, s))
}
}

/// Simplified version of panic payloads, ignoring
/// types other than `&'static str` and `String`.
pub enum PanicMessage {
Expand Down
12 changes: 11 additions & 1 deletion src/test/run-make/track-path-dep-info/macro_def.rs
@@ -1,11 +1,21 @@
#![feature(track_path)]
#![feature(proc_macro_tracked_env,proc_macro_tracked_path)]
#![crate_type = "proc-macro"]

extern crate proc_macro;
use proc_macro::*;

use std::str;

#[proc_macro]
pub fn access_tracked_paths(_: TokenStream) -> TokenStream {
tracked_path::path("emojis.txt");

// currently only valid utf-8 paths are supported
let invalid = [1_u8, 2,123, 254, 0, 0, 1, 1];
let invalid: &str = unsafe {
str::from_utf8_unchecked(&invalid[..])
Copy link
Contributor

Choose a reason for hiding this comment

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

This is undefined behavior though.
I don't think UB is something that we should be testing.

Copy link
Member

Choose a reason for hiding this comment

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

In #87173 it would be possible to safely construct a PathBuf that is not a valid UTF-8 path and then pass it to libproc_macro.

Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest including the AsRef<Path> part of #87173 into this PR and using it to avoid the UB here.

};
tracked_path::path(invalid);

TokenStream::new()
}