Skip to content

Conversation

bjorn3
Copy link
Member

@bjorn3 bjorn3 commented Jun 3, 2017

Also moved check_attribute up to ease reading.

@rust-highfive
Copy link
Contributor

r? @pnkfelix

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

@Mark-Simulacrum
Copy link
Member

Looks like there are some tidy errors here:

tidy check (x86_64-unknown-linux-gnu)
[00:02:31] tidy error: /checkout/src/librustc/hir/check_attr.rs:39: trailing whitespace
[00:02:31] tidy error: /checkout/src/librustc/hir/check_attr.rs:56: trailing whitespace
[00:02:31] tidy error: /checkout/src/librustc/hir/check_attr.rs:65: trailing whitespace
[00:02:32] some tidy checks failed

@Mark-Simulacrum Mark-Simulacrum added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 4, 2017
@bjorn3
Copy link
Member Author

bjorn3 commented Jun 4, 2017

fixed tidy errors

@arielb1
Copy link
Contributor

arielb1 commented Jun 6, 2017

r? @michaelwoerister (I think)

@arielb1 arielb1 assigned michaelwoerister and unassigned pnkfelix Jun 6, 2017
Copy link
Member

@michaelwoerister michaelwoerister left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, @bjorn3! I've added a comment below.

@@ -35,11 +35,26 @@ impl Target {
}
}

/// Visitor which check if all attributes are valid.
///
/// Note: Doesn't check if attributes are existing.
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if this doc comment is useful. I would prefer a module-level comment saying something like:

//! This module implements some validity checks for attributes.
//! In particular it verifies that `#[inline]` and `#[repr]` attributes are 
//! attached to items that actually support them and if there are
//! conflicts between multiple such attributes attached to the same 
//! item.

@michaelwoerister michaelwoerister added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 7, 2017
@bjorn3
Copy link
Member Author

bjorn3 commented Jun 8, 2017

Changed it

@michaelwoerister
Copy link
Member

Thank you! There is some trailing whitespace on some lines that makes make tidy fail:
https://travis-ci.org/rust-lang/rust/jobs/240885005#L721

Looks good to me otherwise.

@bjorn3
Copy link
Member Author

bjorn3 commented Jun 11, 2017

Removed trailing whitespace

@Mark-Simulacrum
Copy link
Member

@bors r=michaelwoerister

@bors
Copy link
Collaborator

bors commented Jun 11, 2017

📌 Commit f4d89ba has been approved by michaelwoerister

@Mark-Simulacrum
Copy link
Member

Actually, could you squash these commits down to one please? Thanks!

@bors r-

@bjorn3
Copy link
Member Author

bjorn3 commented Jun 13, 2017

Squashed

@Mark-Simulacrum
Copy link
Member

Thanks!

@bors r=michaelwoerister rollup

@bors
Copy link
Collaborator

bors commented Jun 13, 2017

📌 Commit e759370 has been approved by michaelwoerister

@arielb1 arielb1 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 13, 2017
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Jun 13, 2017
Add docs to librustc/hir/check_attr.rs

Also moved `check_attribute` up to ease reading.
bors added a commit that referenced this pull request Jun 14, 2017
Rollup of 6 pull requests

- Successful merges: #42408, #42428, #42496, #42597, #42636, #42638
- Failed merges: #42612
@bors bors merged commit e759370 into rust-lang:master Jun 14, 2017
@bjorn3 bjorn3 deleted the patch-2 branch June 14, 2017 06:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

7 participants