Skip to content

Do not expand a derive invocation when derive is not allowed #47013

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Dec 28, 2017

Conversation

topecongiro
Copy link
Contributor

Closes #46655.

The first commit is what actually closes #46655. The second one is just a refactoring I have done while waiting on a test.

1. Change the return type of `expand_invoc()` and its subroutines to
   `Option<Expansion>` from `Expansion`.
2. Return `None` when expanding a derive invocation if the item cannot
   have derive on it (in `expand_derive_invoc()`).
@rust-highfive
Copy link
Contributor

r? @petrochenkov

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

@petrochenkov
Copy link
Contributor

@bors r+
Thanks!

@bors
Copy link
Collaborator

bors commented Dec 26, 2017

📌 Commit f4a76f3 has been approved by petrochenkov

@bors
Copy link
Collaborator

bors commented Dec 27, 2017

⌛ Testing commit f4a76f3 with merge 611349a...

bors added a commit that referenced this pull request Dec 27, 2017
Do not expand a derive invocation when derive is not allowed

Closes #46655.

The first commit is what actually closes #46655. The second one is just a refactoring I have done while waiting on a test.
@bors
Copy link
Collaborator

bors commented Dec 27, 2017

💔 Test failed - status-travis

Copy link
Contributor

@estebank estebank left a comment

Choose a reason for hiding this comment

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

test/run-pass-fulldeps/proc-macro/issue-40001.rs is failing:

[01:05:24] error: Missing 'whitelited_attr' attribute
[01:05:24]   --> /checkout/src/test/run-pass-fulldeps/proc-macro/issue-40001.rs:18:1
[01:05:24]    |
[01:05:24] 18 | fn main() {}
[01:05:24]    | ^^^^^^^^^^^^
[01:05:24]    |
[01:05:24]    = note: #[deny(missing_whitelisted_attr)] on by default
[01:05:24] 
[01:05:24] error: aborting due to previous error

@@ -59,7 +60,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingWhitelistedAttrPass {
_ => cx.tcx.hir.expect_item(cx.tcx.hir.get_parent(id)),
};

if !item.attrs.iter().any(|a| a.check_name("whitelisted_attr")) {
if attr::contains_name(&item.attrs, "whitelisted_attr") {
cx.span_lint(MISSING_WHITELISTED_ATTR, span,
"Missing 'whitelited_attr' attribute");
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a typo: whitelited -> whitelisted

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I forgot !. I fixed the whitelited typo as well.

@estebank
Copy link
Contributor

@bors r=petrochenkov

@bors
Copy link
Collaborator

bors commented Dec 28, 2017

📌 Commit d882691 has been approved by petrochenkov

@bors
Copy link
Collaborator

bors commented Dec 28, 2017

⌛ Testing commit d882691 with merge 5f7aeaf...

bors added a commit that referenced this pull request Dec 28, 2017
Do not expand a derive invocation when derive is not allowed

Closes #46655.

The first commit is what actually closes #46655. The second one is just a refactoring I have done while waiting on a test.
@bors
Copy link
Collaborator

bors commented Dec 28, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: petrochenkov
Pushing 5f7aeaf to master...

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ICE: derive(Clone) on trait item or impl item
5 participants