-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Add check for invalid #[macro_export] arguments #107911
Conversation
r? @TaKO8Ki (rustbot has picked a reviewer for you, use r? to override) |
|
two things about the title - 1. it doesn't need the issue number in the title, and 2. can you make it more "functional"? like, what does it do? e.g. "add a check for invalid macro_export arguments" or something. |
This comment has been minimized.
This comment has been minimized.
/// | ||
pub INVALID_MACRO_EXPORT_ARGUMENTS, | ||
Deny, | ||
"There aren't any attributes named \"invalid_parameters\"", |
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 does this message reference "invalid_parameters"
?
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.
Fixed
attr.span, | ||
errors::MacroExport::Normal, | ||
); | ||
} else if let Some(meta_item_list) = attr.meta_item_list() { |
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 if it's a #[macro_export = ".."]
?
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.
When testing this:
error: malformed `macro_export` attribute input
--> fake-test-src-base/invalid_macro_export_argument.rs:31:1
|
LL | #[macro_export = "Not a valid value"] //~ WARN `not_local_inner_macros` isn't a valid `#[macro_export]` argument
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
It seems like Rust already gives an error in that case.When testing this:
error: malformed `macro_export` attribute input
--> fake-test-src-base/invalid_macro_export_argument.rs:31:1
|
LL | #[macro_export = "Not a valid value"] //~ WARN `not_local_inner_macros` isn't a valid `#[macro_export]` argument
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
It seems like Rust already gives an error in that case.
/// | ||
/// | ||
pub INVALID_MACRO_EXPORT_ARGUMENTS, | ||
Deny, |
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.
This needs to start out at a warn level, not Deny, or else it'll break a bunch of crates that are using this incorrectly currently.
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.
Ok
Does this need a future incompatible warn?
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.
That can be added in a follow-up
The code is all fixed, but testing gives an error and I don't know why. The stderr doesn't show any diff and cannot see what I'm doing wrong. With these input: Output
The lint level has been changed to a Is there something that I'm doing wrong? |
If you make it a warning, then you need to mark the test with |
@rustbot label: -S-waiting-on-author +S-waiting-on-review |
Can you squash these into one commit? I think this is ready to go after that. |
Whoops, sorry, forgot to approve. r? @compiler-errors @bors r+ |
…er-errors Add check for invalid #[macro_export] arguments Resolves rust-lang#107231 Sorry if I made something wrong, this is my first contribution to the repo.
@rustbot label: -T-bootstrap |
Some changes occurred in src/tools/cargo cc @ehuss |
@rustbot label: -T-bootstrap |
@blyxyas, I can push to your branch and rebase this if you're having trouble. |
I'd appreciate that a lot. Thanks |
I'll wait for CI to check this PR, then I can re-approve. In the future, |
@bors r+ |
…er-errors Add check for invalid #[macro_export] arguments Resolves rust-lang#107231 Sorry if I made something wrong, this is my first contribution to the repo.
@bors rollup |
…mpiler-errors Rollup of 7 pull requests Successful merges: - rust-lang#105736 (Test that the compiler/library builds with validate-mir) - rust-lang#107291 ([breaking change] Remove a rustdoc back compat warning) - rust-lang#107675 (Implement -Zlink-directives=yes/no) - rust-lang#107848 (Split `x setup` sub-actions to CLI arguments) - rust-lang#107911 (Add check for invalid #[macro_export] arguments) - rust-lang#108229 ([107049] Recognise top level keys in config.toml.example) - rust-lang#108333 (Make object bound candidates sound in the new trait solver) Failed merges: - rust-lang#108337 (hir-analysis: make a helpful note) r? `@ghost` `@rustbot` modify labels: rollup
This fixes build failure with Rust 1.69+. According to [^1], "the only valid argument is `#[macro_export(local_inner_macros)]` or no argument (`#[macro_export]`)". The check for invalid `#[macro_export]` arguments was added in rust-lang/rust#107911 and first released in Rust 1.69.0. [^1]: https://github.com/rust-lang/rust/blob/1.69.0/compiler/rustc_lint_defs/src/builtin.rs#L4129 Signed-off-by: Ruoyu Zhong <[email protected]>
This fixes build failure with Rust 1.69+. According to [^1], "the only valid argument is `#[macro_export(local_inner_macros)]` or no argument (`#[macro_export]`)". The check for invalid `#[macro_export]` arguments was added in rust-lang/rust#107911 and first released in Rust 1.69.0. [^1]: https://github.com/rust-lang/rust/blob/1.69.0/compiler/rustc_lint_defs/src/builtin.rs#L4129 Signed-off-by: Ruoyu Zhong <[email protected]>
This fixes build failure with Rust 1.69+. According to [^1], "the only valid argument is `#[macro_export(local_inner_macros)]` or no argument (`#[macro_export]`)". The check for invalid `#[macro_export]` arguments was added in rust-lang/rust#107911 and first released in Rust 1.69.0. [^1]: https://github.com/rust-lang/rust/blob/1.69.0/compiler/rustc_lint_defs/src/builtin.rs#L4129 Signed-off-by: Ruoyu Zhong <[email protected]>
This fixes build failure with Rust 1.69+. According to [^1], "the only valid argument is `#[macro_export(local_inner_macros)]` or no argument (`#[macro_export]`)". The check for invalid `#[macro_export]` arguments was added in rust-lang/rust#107911 and first released in Rust 1.69.0. [^1]: https://github.com/rust-lang/rust/blob/1.69.0/compiler/rustc_lint_defs/src/builtin.rs#L4129 Signed-off-by: Ruoyu Zhong <[email protected]> Signed-off-by: fn500i <[email protected]>
Resolves #107231
Sorry if I made something wrong, this is my first contribution to the repo.