-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Specify the behavior of file!
#134442
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
base: master
Are you sure you want to change the base?
Specify the behavior of file!
#134442
Conversation
rustbot has assigned @workingjubilee. Use |
I assume @rust-lang/libs-api would want to review the text but, from what I understand, T-lang is the ultimate decider on built-in macros. |
This documents the existing behavior, and it is hedging about potential future changes (compiler command-line options). Nonetheless, since this could be interpreted as a new guarantee, it's a potential one-way door, so: @rfcbot merge Nominating for T-lang discussion. |
Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members: Concerns:
Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns. |
@rfcbot reviewed |
1 similar comment
@rfcbot reviewed |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
@rfcbot reviewed |
@rustbot labels -I-lang-nominated We discussed this in our lang call today. It's now in FCP. |
library/core/src/macros/mod.rs
Outdated
/// The file name is derived from the root module's source path passed to the Rust compiler | ||
/// (e.g. `./src/lib.rs`) | ||
/// end the sequence the compiler takes to get from root module to the module containing `file!`. | ||
/// (e.g. inside `mod foo;` in `./src/lib.rs`, `file!` would be `./src/foo.rs`), |
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 ./src/foo/mod.rs
:) Perhaps something like:
/// (e.g. inside `mod foo;` in `./src/lib.rs`, `file!` would be `./src/foo.rs`), | |
/// (e.g. if the root file of the crate is `./src/lib.rs`, and within that | |
/// crate there is a module `foo` whose source file is `./src/foo.rs`, then | |
/// `file!` called within will return `./src/foo.rs`), |
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 there is a complete sentence before this, I think the parentheses could be removed and "e.g." replaced with "For example,".
As-is this parenthetical clause starts a sentence and then is followed by , modified by any...
, I think that trailer is meant to go with the first sentence (before this example) in either case.
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.
Agreed. The full stop ahead of the parenthetical seems an error, as the sentence does continue after the parenthetical. As you say, probably it'd be better to keep the sentence together and state the "For example" after 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.
I pulled the example out into a sentence at the end so that the longer example doesn't break the flow of the explanation.
Some further context: we discussed this in today's @rust-lang/lang meeting, and @traviscross raised the concern about this potentially limiting our ability to change |
This comment has been minimized.
This comment has been minimized.
@rfcbot resolve tension-between-uses Good. Where I had landed is that we should weaken the guarantees sufficiently that the output of |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
The final comment period, with a disposition to merge, as per the review above, is now complete. As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed. This will be merged soon. |
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 nit on word choice, take it or leave it, then r=me
This takes the current behavior of `file!` and documents it so it is safe to make assumptions about. For example, Cargo could provide a `CARGO_RUSTC_CURRENT_DIR` as a base path for `file!`. Example use cases - Being able to look up test assets relative to the current file ([example](https://github.com/rust-lang/cargo/blob/b9026bf654d7fac283465e58b8b76742244ef07d/tests/testsuite/cargo_add/add_basic/mod.rs#L34)) - Inline snapshotting libraries being able to update Rust source code ([example](https://github.com/rust-lang/cargo/blob/b9026bf654d7fac283465e58b8b76742244ef07d/tests/testsuite/alt_registry.rs#L36-L45)) T-libs-api discussed two solutions - `file_absolute!`: - Has less meaning in other build tools like buck2 - Bakes in the assumption that a full path is available (e.g. with trim-paths) - Specifying `file!`s behavior (this PR): - Leaves it to the user to deal with trim-paths - Even though `file!` is currently unspecified, changing it would likely have too large of an impact on the ecosystem at this time. A future possibility is that rustc could have a flag that controls modifies the base path used for `file!`. That seems purely additive with specifying the behavior and we do not want to block on it. It would also likely be too disruptive for Cargo users (as mentioned). However, we tried to keep this in mind when specifying the behavior.
@bors r+ rollup |
Specify the behavior of `file!` This takes the current behavior of `file!` and documents it so it is safe to make assumptions about. For example, Cargo could provide a `CARGO_RUSTC_CURRENT_DIR` as a base path for `file!`. Example use cases - Being able to look up test assets relative to the current file ([example](https://github.com/rust-lang/cargo/blob/b9026bf654d7fac283465e58b8b76742244ef07d/tests/testsuite/cargo_add/add_basic/mod.rs#L34)) - Inline snapshotting libraries being able to update Rust source code ([example](https://github.com/rust-lang/cargo/blob/b9026bf654d7fac283465e58b8b76742244ef07d/tests/testsuite/alt_registry.rs#L36-L45)) See rust-lang/cargo#3946 for more context. T-libs-api discussed two solutions in rust-lang/libs-team#478 - `file_absolute!`: - Has less meaning in other build tools like buck2 - Bakes in the assumption that a full path is available (e.g. with trim-paths) - Specifying `file!`s behavior (this PR): - Leaves it to the user to deal with trim-paths - Even though `file!` is currently unspecified, changing it would likely have too large of an impact on the ecosystem at this time. A future possibility is that rustc could have a flag that controls modifies the base path used for `file!`. That seems purely additive with specifying the behavior and we do not want to block on it. It would also likely be too disruptive for Cargo users (as mentioned). However, we tried to keep this in mind when specifying the behavior.
Rollup of 13 pull requests Successful merges: - #134442 (Specify the behavior of `file!`) - #134841 (Look at proc-macro attributes when encountering unknown attribute) - #140372 (Exhaustively handle parsed attributes in CheckAttr) - #140766 (Stabilize keylocker) - #141061 (Change __rust_no_alloc_shim_is_unstable to be a function) - #142042 (Make E0621 missing lifetime suggestion verbose) - #142101 (core::ptr: deduplicate more method docs) - #142176 (tests: Split dont-shuffle-bswaps along opt-levels and arches) - #142258 (platform-support.md: Mention specific Linux kernel version or later) - #142260 (Miri subtree update) - #142262 (Mark `core::slice::memchr` as `#[doc(hidden)]`) - #142272 (tests: Change ABIs in tests to more future-resilient ones) - #142275 (rustdoc: Refractor `clean_ty_generics`) r? `@ghost` `@rustbot` modify labels: rollup
Specify the behavior of `file!` This takes the current behavior of `file!` and documents it so it is safe to make assumptions about. For example, Cargo could provide a `CARGO_RUSTC_CURRENT_DIR` as a base path for `file!`. Example use cases - Being able to look up test assets relative to the current file ([example](https://github.com/rust-lang/cargo/blob/b9026bf654d7fac283465e58b8b76742244ef07d/tests/testsuite/cargo_add/add_basic/mod.rs#L34)) - Inline snapshotting libraries being able to update Rust source code ([example](https://github.com/rust-lang/cargo/blob/b9026bf654d7fac283465e58b8b76742244ef07d/tests/testsuite/alt_registry.rs#L36-L45)) See rust-lang/cargo#3946 for more context. T-libs-api discussed two solutions in rust-lang/libs-team#478 - `file_absolute!`: - Has less meaning in other build tools like buck2 - Bakes in the assumption that a full path is available (e.g. with trim-paths) - Specifying `file!`s behavior (this PR): - Leaves it to the user to deal with trim-paths - Even though `file!` is currently unspecified, changing it would likely have too large of an impact on the ecosystem at this time. A future possibility is that rustc could have a flag that controls modifies the base path used for `file!`. That seems purely additive with specifying the behavior and we do not want to block on it. It would also likely be too disruptive for Cargo users (as mentioned). However, we tried to keep this in mind when specifying the behavior.
Rollup of 12 pull requests Successful merges: - #134442 (Specify the behavior of `file!`) - #134841 (Look at proc-macro attributes when encountering unknown attribute) - #140372 (Exhaustively handle parsed attributes in CheckAttr) - #140766 (Stabilize keylocker) - #141061 (Change __rust_no_alloc_shim_is_unstable to be a function) - #142042 (Make E0621 missing lifetime suggestion verbose) - #142101 (core::ptr: deduplicate more method docs) - #142258 (platform-support.md: Mention specific Linux kernel version or later) - #142260 (Miri subtree update) - #142262 (Mark `core::slice::memchr` as `#[doc(hidden)]`) - #142272 (tests: Change ABIs in tests to more future-resilient ones) - #142275 (rustdoc: Refractor `clean_ty_generics`) r? `@ghost` `@rustbot` modify labels: rollup
Specify the behavior of `file!` This takes the current behavior of `file!` and documents it so it is safe to make assumptions about. For example, Cargo could provide a `CARGO_RUSTC_CURRENT_DIR` as a base path for `file!`. Example use cases - Being able to look up test assets relative to the current file ([example](https://github.com/rust-lang/cargo/blob/b9026bf654d7fac283465e58b8b76742244ef07d/tests/testsuite/cargo_add/add_basic/mod.rs#L34)) - Inline snapshotting libraries being able to update Rust source code ([example](https://github.com/rust-lang/cargo/blob/b9026bf654d7fac283465e58b8b76742244ef07d/tests/testsuite/alt_registry.rs#L36-L45)) See rust-lang/cargo#3946 for more context. T-libs-api discussed two solutions in rust-lang/libs-team#478 - `file_absolute!`: - Has less meaning in other build tools like buck2 - Bakes in the assumption that a full path is available (e.g. with trim-paths) - Specifying `file!`s behavior (this PR): - Leaves it to the user to deal with trim-paths - Even though `file!` is currently unspecified, changing it would likely have too large of an impact on the ecosystem at this time. A future possibility is that rustc could have a flag that controls modifies the base path used for `file!`. That seems purely additive with specifying the behavior and we do not want to block on it. It would also likely be too disruptive for Cargo users (as mentioned). However, we tried to keep this in mind when specifying the behavior.
Rollup of 14 pull requests Successful merges: - #134442 (Specify the behavior of `file!`) - #134841 (Look at proc-macro attributes when encountering unknown attribute) - #140372 (Exhaustively handle parsed attributes in CheckAttr) - #140766 (Stabilize keylocker) - #141642 (Note the version and PR of removed features when using it) - #141909 (Add central execution context to bootstrap) - #141992 (use `#[naked]` for `__rust_probestack`) - #142102 (docs: Small clarification on the usage of read_to_string and read_to_end trait methods) - #142124 (Allow transmute casts in pre-runtime-MIR) - #142240 (deduplicate the rest of AST walker functions) - #142258 (platform-support.md: Mention specific Linux kernel version or later) - #142262 (Mark `core::slice::memchr` as `#[doc(hidden)]`) - #142271 (compiler: fn ptrs should hit different lints based on ABI) - #142288 (const_eval: fix some outdated comments) r? `@ghost` `@rustbot` modify labels: rollup
This takes the current behavior of
file!
and documents it so it is safe to make assumptions about.For example, Cargo could provide a
CARGO_RUSTC_CURRENT_DIR
as a base path forfile!
.Example use cases
See rust-lang/cargo#3946 for more context.
T-libs-api discussed two solutions in rust-lang/libs-team#478
file_absolute!
:file!
s behavior (this PR):file!
is currently unspecified, changing it would likely have too large of an impact on the ecosystem at this time.A future possibility is that rustc could have a flag that controls modifies the base path used for
file!
.That seems purely additive with specifying the behavior and we do not want to block on it.
It would also likely be too disruptive for Cargo users (as mentioned). However, we tried to keep this in mind when specifying the behavior.