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

Rust: exclude extraction of code excluded by cfg #18313

Merged
merged 4 commits into from
Jan 8, 2025

Conversation

redsun82
Copy link
Contributor

This change skips all extraction of disabled #[cfg] blocks. This:

  • brings down warnings coming from those blocks
  • avoids spurious alerts coming from them
  • might also save some execution time and database size

This requires a newer version of rust-analyzer than what we have on main, reason for which this sits on top of #18295 and should be merged after that.

@github-actions github-actions bot added the Rust Pull requests that update Rust code label Dec 18, 2024
@aibaars aibaars force-pushed the aibaars/update-rust-ungram branch from 92a10b8 to a6ec51a Compare December 18, 2024 12:00
@redsun82 redsun82 force-pushed the redsun82/rust-mute-warnings-in-uncompiled-blocks branch from 7804b56 to 218bc80 Compare December 18, 2024 12:34
Base automatically changed from aibaars/update-rust-ungram to main December 18, 2024 15:01
@redsun82 redsun82 marked this pull request as ready for review December 18, 2024 15:42
@Copilot Copilot bot review requested due to automatic review settings December 18, 2024 15:42
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 3 out of 6 changed files in this pull request and generated 1 comment.

Files not reviewed (3)
  • rust/ql/integration-tests/options/cfg/functions.ql: Language not supported
  • rust/ql/integration-tests/options/features/functions.ql: Language not supported
  • rust/ql/integration-tests/options/target/functions.ql: Language not supported
Comments suppressed due to low confidence (1)

rust/ast-generator/src/main.rs:527

  • [nitpick] The variable name 'field' is too generic. Consider renaming it to 'node_field' for better clarity.
if field.name == "attrs" {

Tip: If you use Visual Studio Code, you can request a review from Copilot before you push from the "Source Control" tab. Learn more

rust/extractor/src/translate/base.rs Outdated Show resolved Hide resolved
@redsun82 redsun82 requested a review from aibaars December 20, 2024 15:38
Comment on lines 566 to 573
for attr in item.attrs() {
if let Some((name, tokens)) = attr.as_simple_call() {
if name == "cfg" && sema.check_cfg_attr(&tokens) == Some(false) {
return true;
}
}
}
false
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps this would be nicer with any and is_some_and?

Suggested change
for attr in item.attrs() {
if let Some((name, tokens)) = attr.as_simple_call() {
if name == "cfg" && sema.check_cfg_attr(&tokens) == Some(false) {
return true;
}
}
}
false
item.attrs().any(|attr| {
attr.as_simple_call().is_some_and(|(name, tokens)| {
name == "cfg" && sema.check_cfg_attr(&tokens) == Some(false)
})
})

@redsun82 redsun82 requested a review from paldepind January 7, 2025 12:05
@redsun82 redsun82 merged commit 3bf2416 into main Jan 8, 2025
14 checks passed
@redsun82 redsun82 deleted the redsun82/rust-mute-warnings-in-uncompiled-blocks branch January 8, 2025 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Rust Pull requests that update Rust code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants