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

Feature gate macro arguments #20190

Merged
merged 1 commit into from
Jan 1, 2015
Merged

Feature gate macro arguments #20190

merged 1 commit into from
Jan 1, 2015

Conversation

emberian
Copy link
Member

Uses the same approach as #17286 (and
subsequent changes making it more correct), where the visitor will skip any
pieces of the AST that are from "foreign code", where the spans don't line up,
indicating that that piece of code is due to a macro expansion.

If this breaks your code, read the error message to determine which feature
gate you should add to your crate.

Closes #18102

[breaking-change]

@rust-highfive
Copy link
Collaborator

r? @huonw

(rust_highfive has picked a reviewer for you, use r? to override)

$bl
}
);
);
Copy link
Member

Choose a reason for hiding this comment

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

Instead of being a macro like this (and requiring it to be before all checks), could this just move into the gate_feature function?

@alexcrichton
Copy link
Member

Thanks! Could you add some more tests as well for things like using a feature in a macro, but not having it as part of an argument?

Also, to close #18102 I think this needs test cases along the lines of:

  • Macro definitions that use gated features should be gated when used
  • Macros across crates which use features should be gated when used

@emberian
Copy link
Member Author

@alexcrichton macro definitions are part of #12122 which I planned on fixing in a follow-up PR. This PR doesn't do anything for macros which internally used gated features.

@emberian
Copy link
Member Author

(And I plan on fixing #12122 by erroring on macro definition, rather than unsuspecting user code)

@alexcrichton
Copy link
Member

It looks like #18102 is on the 1.0 milestone but #12122 is not, but I think I'll just nominate #12122 for being so now that our plans have changed.

Checking the macro definition may be tricky because we don't know what we're expanding to (e.g. an item, block, or pattern), but it may be possible!

self.gate_feature("default_type_params", ty.span,
"default type parameters are \
experimental and possibly buggy");
}
experimental and possibly buggy"),
Copy link
Member

Choose a reason for hiding this comment

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

This change may be ok to undo, we tend to prefer braces around multi-line arms.

@alexcrichton
Copy link
Member

Awesome, thanks @cmr! Just a few minor nits and otherwise r=me

Uses the same approach as #17286 (and
subsequent changes making it more correct), where the visitor will skip any
pieces of the AST that are from "foreign code", where the spans don't line up,
indicating that that piece of code is due to a macro expansion.

If this breaks your code, read the error message to determine which feature
gate you should add to your crate, and bask in the knowledge that your code
won't mysteriously break should you try to use the 1.0 release.

Closes #18102

[breaking-change]
bors added a commit that referenced this pull request Jan 1, 2015
Uses the same approach as #17286 (and
subsequent changes making it more correct), where the visitor will skip any
pieces of the AST that are from "foreign code", where the spans don't line up,
indicating that that piece of code is due to a macro expansion.

If this breaks your code, read the error message to determine which feature
gate you should add to your crate.

Closes #18102

[breaking-change]
@bors bors merged commit 41da99d into rust-lang:master Jan 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Many feature gates are not checked in macros
5 participants