Skip to content

Do not print builtin macro definitions in ast_dump #566

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

Merged
merged 2 commits into from
Mar 9, 2017

Conversation

tsliang
Copy link

@tsliang tsliang commented Mar 8, 2017

Resolves issue #476. Moves out logic for checking whether a cursor has a filename (previously used exclusively in private function lib::filter_builtins) into the actual cursor. This code is then used to check whether a cursor is a builtin when dumping the AST.

Resolves issue rust-lang#476. Moves out logic for checking whether a cursor has a filename (previously used exclusively in private function lib::filter_builtins) into the actual cursor. This code is then used to check whether a cursor is a builtin when dumping the AST.
Copy link
Contributor

@emilio emilio left a comment

Choose a reason for hiding this comment

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

Looks great! Two minor nits.

src/clang.rs Outdated
/// Returns whether the cursor refers to a built-in definition.
pub fn is_builtin(&self) -> bool {
let (file, _, _, _) = self.location().location();
!file.name().is_some()
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: file.name().is_none() instead.

src/lib.rs Outdated
None => ctx.options().builtins,
Some(..) => true,
}
!cursor.is_builtin() || ctx.options().builtins
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I'd reverse the order here since the second is way less expensive to check.

@tsliang
Copy link
Author

tsliang commented Mar 9, 2017

@emilio thanks for the feedback. Latest commit should address those two points.

@fitzgen
Copy link
Member

fitzgen commented Mar 9, 2017

@bors-servo r=emilio,fitzgen

Thanks @tsliang !

@bors-servo
Copy link

📌 Commit b826a80 has been approved by emilio,fitzgen

@bors-servo
Copy link

⌛ Testing commit b826a80 with merge 2afecec...

bors-servo pushed a commit that referenced this pull request Mar 9, 2017
Do not print builtin macro definitions in ast_dump

Resolves issue #476. Moves out logic for checking whether a cursor has a filename (previously used exclusively in private function lib::filter_builtins) into the actual cursor. This code is then used to check whether a cursor is a builtin when dumping the AST.
@bors-servo
Copy link

☀️ Test successful - status-travis
Approved by: emilio,fitzgen
Pushing 2afecec to master...

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.

5 participants