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

Harden param_attrs test wrt. usage of a proc macro #[attr] #64031

Merged
merged 1 commit into from
Sep 5, 2019

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Aug 31, 2019

The param-attrs-builtin-attrs.rs test file uses the #[test] attribute which should cover this but #[test] isn't a proc macro attribute so we add another test to be on the safe side. This intends to address #64010 (comment).

r? @nikomatsakis

cc @c410-f3r @petrochenkov
cc #60406

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 31, 2019
@Centril Centril added the F-param_attrs `#![feature(param_attrs)]` label Aug 31, 2019
use proc_macro::TokenStream;

#[proc_macro_attribute]
pub fn id(_: TokenStream, input: TokenStream) -> TokenStream { input }
Copy link
Contributor

Choose a reason for hiding this comment

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

We have auxiliary/test-macros.rs that defines all the utility macros like this, you can use #[identity_attr] from there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd have to move the test, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, ok then.
(We need a way to use a crate from the root auxiliary directory in all subdirectories to avoid duplication.)

@petrochenkov
Copy link
Contributor

(#[test] is a proc macro attribute though. Yes, there are some differences with #[id] in whether expansion goes directly into AST or through tokens to AST, but their name resolution behavior should be identical.)

@Centril
Copy link
Contributor Author

Centril commented Aug 31, 2019

(Yeah, fair point. I suppose the test can serve as accident prevention for possibly weird changes but those would be unlikely. I'll leave it to Niko to decide whether they feel this would be beneficial. I think it need not block stabilization in either case.)

@nikomatsakis
Copy link
Contributor

@bors rollup r+

@bors
Copy link
Contributor

bors commented Sep 4, 2019

📌 Commit 5187a3e has been approved by nikomatsakis

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 4, 2019
@nikomatsakis
Copy link
Contributor

Seems good to test a "user-provided" proc macro regardless

Centril added a commit to Centril/rust that referenced this pull request Sep 5, 2019
… r=nikomatsakis

Harden `param_attrs` test wrt. usage of a proc macro `#[attr]`

The `param-attrs-builtin-attrs.rs` test file uses the `#[test]` attribute which should cover this but `#[test]` isn't a proc macro attribute so we add another test to be on the safe side. This intends to address rust-lang#64010 (comment).

r? @nikomatsakis

cc @c410-f3r @petrochenkov
cc rust-lang#60406
Centril added a commit to Centril/rust that referenced this pull request Sep 5, 2019
… r=nikomatsakis

Harden `param_attrs` test wrt. usage of a proc macro `#[attr]`

The `param-attrs-builtin-attrs.rs` test file uses the `#[test]` attribute which should cover this but `#[test]` isn't a proc macro attribute so we add another test to be on the safe side. This intends to address rust-lang#64010 (comment).

r? @nikomatsakis

cc @c410-f3r @petrochenkov
cc rust-lang#60406
bors added a commit that referenced this pull request Sep 5, 2019
Rollup of 15 pull requests

Successful merges:

 - #62860 (Stabilize checked_duration_since for 1.38.0)
 - #63549 (Rev::rposition counts from the wrong end)
 - #63985 (Stabilize pin_into_inner in 1.39.0)
 - #64005 (Add a `Place::is_indirect` method to determine whether a `Place` contains a `Deref` projection)
 - #64031 (Harden `param_attrs` test wrt. usage of a proc macro `#[attr]`)
 - #64038 (Check impl trait substs when checking for recursive types)
 - #64043 (Add some more tests for underscore imports)
 - #64092 (Update xLTO compatibility table in rustc book.)
 - #64110 (Refer to "`self` type" instead of "receiver type")
 - #64120 (Move path parsing earlier)
 - #64123 (Added warning around code with reference to uninit bytes)
 - #64128 (unused_parens: account for or-patterns and `&(mut x)`)
 - #64141 (Minimize uses of `LocalInternedString`)
 - #64142 (Fix doc links in `std::cmp` module)
 - #64148 (fix a few typos in comments)

Failed merges:

r? @ghost
@bors bors merged commit 5187a3e into rust-lang:master Sep 5, 2019
@Centril Centril deleted the param-attrs-no-macros-test branch September 5, 2019 06:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F-param_attrs `#![feature(param_attrs)]` S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants