mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #17006 - Veykril:cfg-censor-fixes, r=Veykril

Browse Source 
fix: Fix some cfg censoring bugs

Fixes https://github.com/rust-lang/rust-analyzer/issues/16947
This commit is contained in:
bors 2024-04-03 14:45:19 +00:00
commit c0eb78e839
4 changed files with 85 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
crates/hir-def/src/macro_expansion_tests/builtin_derive_macro.rs
View File

@ -610,6 +610,10 @@ struct Foo {
field1: i32, field1: i32,
#[cfg(never)] #[cfg(never)]
field2: (), field2: (),
#[cfg(feature = "never")]
field3: (),
#[cfg(not(feature = "never"))]
field4: (),
} }
#[derive(Default)] #[derive(Default)]
enum Bar { enum Bar {
@ -618,12 +622,16 @@ enum Bar {
Bar, Bar,
} }
"#, "#,
expect![[r#" expect![[r##"
#[derive(Default)] #[derive(Default)]
struct Foo { struct Foo {
field1: i32, field1: i32,
#[cfg(never)] #[cfg(never)]
field2: (), field2: (),
#[cfg(feature = "never")]
field3: (),
#[cfg(not(feature = "never"))]
field4: (),
} }
#[derive(Default)] #[derive(Default)]
enum Bar { enum Bar {
@ -635,7 +643,7 @@ enum Bar {
impl < > $crate::default::Default for Foo< > where { impl < > $crate::default::Default for Foo< > where {
fn default() -> Self { fn default() -> Self {
Foo { Foo {
field1: $crate::default::Default::default(), field1: $crate::default::Default::default(), field4: $crate::default::Default::default(),
} }
} }
} }
@ -643,6 +651,6 @@ impl < > $crate::default::Default for Bar< > where {
fn default() -> Self { fn default() -> Self {
Bar::Bar Bar::Bar
} }
}"#]], }"##]],
); );
} }

119
crates/hir-expand/src/cfg_process.rs
View File

@ -1,57 +1,59 @@
//! Processes out #[cfg] and #[cfg_attr] attributes from the input for the derive macro //! Processes out #[cfg] and #[cfg_attr] attributes from the input for the derive macro
use std::iter::Peekable; use std::iter::Peekable;
use base_db::CrateId;
use cfg::{CfgAtom, CfgExpr}; use cfg::{CfgAtom, CfgExpr};
use rustc_hash::FxHashSet; use rustc_hash::FxHashSet;
use syntax::{ use syntax::{
ast::{self, Attr, HasAttrs, Meta, VariantList}, ast::{self, Attr, HasAttrs, Meta, VariantList},
AstNode, NodeOrToken, SyntaxElement, SyntaxNode, T, AstNode, NodeOrToken, SyntaxElement, SyntaxKind, SyntaxNode, T,
}; };
use tracing::{debug, warn}; use tracing::{debug, warn};
use tt::SmolStr; use tt::SmolStr;
use crate::{db::ExpandDatabase, proc_macro::ProcMacroKind, MacroCallLoc, MacroDefKind}; use crate::{db::ExpandDatabase, proc_macro::ProcMacroKind, MacroCallLoc, MacroDefKind};
fn check_cfg_attr(attr: &Attr, loc: &MacroCallLoc, db: &dyn ExpandDatabase) -> Option<bool> { fn check_cfg(db: &dyn ExpandDatabase, attr: &Attr, krate: CrateId) -> Option<bool> {
if !attr.simple_name().as_deref().map(|v| v == "cfg")? { if !attr.simple_name().as_deref().map(|v| v == "cfg")? {
return None; return None;
} }
debug!("Evaluating cfg {}", attr);
let cfg = parse_from_attr_meta(attr.meta()?)?; let cfg = parse_from_attr_meta(attr.meta()?)?;
debug!("Checking cfg {:?}", cfg); let enabled = db.crate_graph()[krate].cfg_options.check(&cfg) != Some(false);
let enabled = db.crate_graph()[loc.krate].cfg_options.check(&cfg) != Some(false);
Some(enabled) Some(enabled)
} }
fn check_cfg_attr_attr(attr: &Attr, loc: &MacroCallLoc, db: &dyn ExpandDatabase) -> Option<bool> { fn check_cfg_attr(db: &dyn ExpandDatabase, attr: &Attr, krate: CrateId) -> Option<bool> {
if !attr.simple_name().as_deref().map(|v| v == "cfg_attr")? { if !attr.simple_name().as_deref().map(|v| v == "cfg_attr")? {
return None; return None;
} }
debug!("Evaluating cfg_attr {}", attr);
let cfg_expr = parse_from_attr_meta(attr.meta()?)?; let cfg_expr = parse_from_attr_meta(attr.meta()?)?;
debug!("Checking cfg_attr {:?}", cfg_expr); let enabled = db.crate_graph()[krate].cfg_options.check(&cfg_expr) != Some(false);
let enabled = db.crate_graph()[loc.krate].cfg_options.check(&cfg_expr) != Some(false);
Some(enabled) Some(enabled)
} }
fn process_has_attrs_with_possible_comma<I: HasAttrs>( fn process_has_attrs_with_possible_comma<I: HasAttrs>(
items: impl Iterator<Item = I>,
loc: &MacroCallLoc,
db: &dyn ExpandDatabase, db: &dyn ExpandDatabase,
items: impl Iterator<Item = I>,
krate: CrateId,
remove: &mut FxHashSet<SyntaxElement>, remove: &mut FxHashSet<SyntaxElement>,
) -> Option<()> { ) -> Option<()> {
for item in items { for item in items {
let field_attrs = item.attrs(); let field_attrs = item.attrs();
'attrs: for attr in field_attrs { 'attrs: for attr in field_attrs {
if check_cfg_attr(&attr, loc, db).map(|enabled| !enabled).unwrap_or_default() { if let Some(enabled) = check_cfg(db, &attr, krate) {
debug!("censoring type {:?}", item.syntax()); if enabled {
remove.insert(item.syntax().clone().into()); debug!("censoring {:?}", attr.syntax());
// We need to remove the , as well remove.insert(attr.syntax().clone().into());
remove_possible_comma(&item, remove); } else {
break 'attrs; debug!("censoring {:?}", item.syntax());
remove.insert(item.syntax().clone().into());
// We need to remove the , as well
remove_possible_comma(&item, remove);
break 'attrs;
}
} }
if let Some(enabled) = check_cfg_attr_attr(&attr, loc, db) { if let Some(enabled) = check_cfg_attr(db, &attr, krate) {
if enabled { if enabled {
debug!("Removing cfg_attr tokens {:?}", attr); debug!("Removing cfg_attr tokens {:?}", attr);
let meta = attr.meta()?; let meta = attr.meta()?;
@ -60,13 +62,13 @@ fn process_has_attrs_with_possible_comma<I: HasAttrs>(
} else { } else {
debug!("censoring type cfg_attr {:?}", item.syntax()); debug!("censoring type cfg_attr {:?}", item.syntax());
remove.insert(attr.syntax().clone().into()); remove.insert(attr.syntax().clone().into());
continue;
} }
} }
} }
} }
Some(()) Some(())
} }
#[derive(Debug, PartialEq, Eq, Clone, Copy)] #[derive(Debug, PartialEq, Eq, Clone, Copy)]
enum CfgExprStage { enum CfgExprStage {
/// Stripping the CFGExpr part of the attribute /// Stripping the CFGExpr part of the attribute
@ -78,6 +80,7 @@ enum CfgExprStage {
// Related Issue: https://github.com/rust-lang/rust-analyzer/issues/10110 // Related Issue: https://github.com/rust-lang/rust-analyzer/issues/10110
EverythingElse, EverythingElse,
} }
/// This function creates its own set of tokens to remove. To help prevent malformed syntax as input. /// This function creates its own set of tokens to remove. To help prevent malformed syntax as input.
fn remove_tokens_within_cfg_attr(meta: Meta) -> Option<FxHashSet<SyntaxElement>> { fn remove_tokens_within_cfg_attr(meta: Meta) -> Option<FxHashSet<SyntaxElement>> {
let mut remove: FxHashSet<SyntaxElement> = FxHashSet::default(); let mut remove: FxHashSet<SyntaxElement> = FxHashSet::default();
@ -131,23 +134,28 @@ fn remove_possible_comma(item: &impl AstNode, res: &mut FxHashSet<SyntaxElement>
} }
} }
fn process_enum( fn process_enum(
variants: VariantList,
loc: &MacroCallLoc,
db: &dyn ExpandDatabase, db: &dyn ExpandDatabase,
variants: VariantList,
krate: CrateId,
remove: &mut FxHashSet<SyntaxElement>, remove: &mut FxHashSet<SyntaxElement>,
) -> Option<()> { ) -> Option<()> {
'variant: for variant in variants.variants() { 'variant: for variant in variants.variants() {
for attr in variant.attrs() { for attr in variant.attrs() {
if check_cfg_attr(&attr, loc, db).map(|enabled| !enabled).unwrap_or_default() { if let Some(enabled) = check_cfg(db, &attr, krate) {
// Rustc does not strip the attribute if it is enabled. So we will leave it if enabled {
debug!("censoring type {:?}", variant.syntax()); debug!("censoring {:?}", attr.syntax());
remove.insert(variant.syntax().clone().into()); remove.insert(attr.syntax().clone().into());
// We need to remove the , as well } else {
remove_possible_comma(&variant, remove); // Rustc does not strip the attribute if it is enabled. So we will leave it
continue 'variant; debug!("censoring type {:?}", variant.syntax());
}; remove.insert(variant.syntax().clone().into());
// We need to remove the , as well
remove_possible_comma(&variant, remove);
continue 'variant;
}
}
if let Some(enabled) = check_cfg_attr_attr(&attr, loc, db) { if let Some(enabled) = check_cfg_attr(db, &attr, krate) {
if enabled { if enabled {
debug!("Removing cfg_attr tokens {:?}", attr); debug!("Removing cfg_attr tokens {:?}", attr);
let meta = attr.meta()?; let meta = attr.meta()?;
@ -156,17 +164,16 @@ fn process_enum(
} else { } else {
debug!("censoring type cfg_attr {:?}", variant.syntax()); debug!("censoring type cfg_attr {:?}", variant.syntax());
remove.insert(attr.syntax().clone().into()); remove.insert(attr.syntax().clone().into());
continue;
} }
} }
} }
if let Some(fields) = variant.field_list() { if let Some(fields) = variant.field_list() {
match fields { match fields {
ast::FieldList::RecordFieldList(fields) => { ast::FieldList::RecordFieldList(fields) => {
process_has_attrs_with_possible_comma(fields.fields(), loc, db, remove)?; process_has_attrs_with_possible_comma(db, fields.fields(), krate, remove)?;
} }
ast::FieldList::TupleFieldList(fields) => { ast::FieldList::TupleFieldList(fields) => {
process_has_attrs_with_possible_comma(fields.fields(), loc, db, remove)?; process_has_attrs_with_possible_comma(db, fields.fields(), krate, remove)?;
} }
} }
} }
@ -175,9 +182,9 @@ fn process_enum(
} }
pub(crate) fn process_cfg_attrs( pub(crate) fn process_cfg_attrs(
db: &dyn ExpandDatabase,
node: &SyntaxNode, node: &SyntaxNode,
loc: &MacroCallLoc, loc: &MacroCallLoc,
db: &dyn ExpandDatabase,
) -> Option<FxHashSet<SyntaxElement>> { ) -> Option<FxHashSet<SyntaxElement>> {
// FIXME: #[cfg_eval] is not implemented. But it is not stable yet // FIXME: #[cfg_eval] is not implemented. But it is not stable yet
let is_derive = match loc.def.kind { let is_derive = match loc.def.kind {
@ -193,36 +200,35 @@ pub(crate) fn process_cfg_attrs(
let item = ast::Item::cast(node.clone())?; let item = ast::Item::cast(node.clone())?;
for attr in item.attrs() { for attr in item.attrs() {
if let Some(enabled) = check_cfg_attr_attr(&attr, loc, db) { if let Some(enabled) = check_cfg_attr(db, &attr, loc.krate) {
if enabled { if enabled {
debug!("Removing cfg_attr tokens {:?}", attr); debug!("Removing cfg_attr tokens {:?}", attr);
let meta = attr.meta()?; let meta = attr.meta()?;
let removes_from_cfg_attr = remove_tokens_within_cfg_attr(meta)?; let removes_from_cfg_attr = remove_tokens_within_cfg_attr(meta)?;
remove.extend(removes_from_cfg_attr); remove.extend(removes_from_cfg_attr);
} else { } else {
debug!("censoring type cfg_attr {:?}", item.syntax()); debug!("Removing type cfg_attr {:?}", item.syntax());
remove.insert(attr.syntax().clone().into()); remove.insert(attr.syntax().clone().into());
continue;
} }
} }
} }
match item { match item {
ast::Item::Struct(it) => match it.field_list()? { ast::Item::Struct(it) => match it.field_list()? {
ast::FieldList::RecordFieldList(fields) => { ast::FieldList::RecordFieldList(fields) => {
process_has_attrs_with_possible_comma(fields.fields(), loc, db, &mut remove)?; process_has_attrs_with_possible_comma(db, fields.fields(), loc.krate, &mut remove)?;
} }
ast::FieldList::TupleFieldList(fields) => { ast::FieldList::TupleFieldList(fields) => {
process_has_attrs_with_possible_comma(fields.fields(), loc, db, &mut remove)?; process_has_attrs_with_possible_comma(db, fields.fields(), loc.krate, &mut remove)?;
} }
}, },
ast::Item::Enum(it) => { ast::Item::Enum(it) => {
process_enum(it.variant_list()?, loc, db, &mut remove)?; process_enum(db, it.variant_list()?, loc.krate, &mut remove)?;
} }
ast::Item::Union(it) => { ast::Item::Union(it) => {
process_has_attrs_with_possible_comma( process_has_attrs_with_possible_comma(
it.record_field_list()?.fields(),
loc,
db, db,
it.record_field_list()?.fields(),
loc.krate,
&mut remove, &mut remove,
)?; )?;
} }
@ -234,10 +240,22 @@ pub(crate) fn process_cfg_attrs(
/// Parses a `cfg` attribute from the meta /// Parses a `cfg` attribute from the meta
fn parse_from_attr_meta(meta: Meta) -> Option<CfgExpr> { fn parse_from_attr_meta(meta: Meta) -> Option<CfgExpr> {
let tt = meta.token_tree()?; let tt = meta.token_tree()?;
let mut iter = tt.token_trees_and_tokens().skip(1).peekable(); let mut iter = tt
.token_trees_and_tokens()
.filter(is_not_whitespace)
.skip(1)
.take_while(is_not_closing_paren)
.peekable();
next_cfg_expr_from_syntax(&mut iter) next_cfg_expr_from_syntax(&mut iter)
} }
fn is_not_closing_paren(element: &NodeOrToken<ast::TokenTree, syntax::SyntaxToken>) -> bool {
!matches!(element, NodeOrToken::Token(token) if (token.kind() == syntax::T![')']))
}
fn is_not_whitespace(element: &NodeOrToken<ast::TokenTree, syntax::SyntaxToken>) -> bool {
!matches!(element, NodeOrToken::Token(token) if (token.kind() == SyntaxKind::WHITESPACE))
}
fn next_cfg_expr_from_syntax<I>(iter: &mut Peekable<I>) -> Option<CfgExpr> fn next_cfg_expr_from_syntax<I>(iter: &mut Peekable<I>) -> Option<CfgExpr>
where where
I: Iterator<Item = NodeOrToken<ast::TokenTree, syntax::SyntaxToken>>, I: Iterator<Item = NodeOrToken<ast::TokenTree, syntax::SyntaxToken>>,
@ -256,14 +274,13 @@ fn next_cfg_expr_from_syntax<I>(iter: &mut Peekable<I>) -> Option<CfgExpr>
let Some(NodeOrToken::Node(tree)) = iter.next() else { let Some(NodeOrToken::Node(tree)) = iter.next() else {
return Some(CfgExpr::Invalid); return Some(CfgExpr::Invalid);
}; };
let mut tree_iter = tree.token_trees_and_tokens().skip(1).peekable(); let mut tree_iter = tree
while tree_iter .token_trees_and_tokens()
.peek() .filter(is_not_whitespace)
.filter( .skip(1)
|element| matches!(element, NodeOrToken::Token(token) if (token.kind() != syntax::T![')'])), .take_while(is_not_closing_paren)
) .peekable();
.is_some() while tree_iter.peek().is_some() {
{
let pred = next_cfg_expr_from_syntax(&mut tree_iter); let pred = next_cfg_expr_from_syntax(&mut tree_iter);
if let Some(pred) = pred { if let Some(pred) = pred {
preds.push(pred); preds.push(pred);

4
crates/hir-expand/src/db.rs
View File

@ -175,7 +175,7 @@ pub fn expand_speculative(
}; };
let censor_cfg = let censor_cfg =
cfg_process::process_cfg_attrs(speculative_args, &loc, db).unwrap_or_default(); cfg_process::process_cfg_attrs(db, speculative_args, &loc).unwrap_or_default();
let mut fixups = fixup::fixup_syntax(span_map, speculative_args, span); let mut fixups = fixup::fixup_syntax(span_map, speculative_args, span);
fixups.append.retain(|it, _| match it { fixups.append.retain(|it, _| match it {
syntax::NodeOrToken::Token(_) => true, syntax::NodeOrToken::Token(_) => true,
@ -462,7 +462,7 @@ fn macro_arg(db: &dyn ExpandDatabase, id: MacroCallId) -> MacroArgResult {
let (mut tt, undo_info) = { let (mut tt, undo_info) = {
let syntax = item_node.syntax(); let syntax = item_node.syntax();
let censor_cfg = cfg_process::process_cfg_attrs(syntax, &loc, db).unwrap_or_default(); let censor_cfg = cfg_process::process_cfg_attrs(db, syntax, &loc).unwrap_or_default();
let mut fixups = fixup::fixup_syntax(map.as_ref(), syntax, span); let mut fixups = fixup::fixup_syntax(map.as_ref(), syntax, span);
fixups.append.retain(|it, _| match it { fixups.append.retain(|it, _| match it {
syntax::NodeOrToken::Token(_) => true, syntax::NodeOrToken::Token(_) => true,

5
crates/ide-completion/src/completions/postfix.rs
View File

@ -9,6 +9,7 @@
ty_filter::TryEnum, ty_filter::TryEnum,
SnippetCap, SnippetCap,
}; };
use stdx::never;
use syntax::{ use syntax::{
ast::{self, make, AstNode, AstToken}, ast::{self, make, AstNode, AstToken},
SyntaxKind::{BLOCK_EXPR, EXPR_STMT, FOR_EXPR, IF_EXPR, LOOP_EXPR, STMT_LIST, WHILE_EXPR}, SyntaxKind::{BLOCK_EXPR, EXPR_STMT, FOR_EXPR, IF_EXPR, LOOP_EXPR, STMT_LIST, WHILE_EXPR},
@ -319,7 +320,9 @@ fn build_postfix_snippet_builder<'ctx>(
) -> Option<impl Fn(&str, &str, &str) -> Builder + 'ctx> { ) -> Option<impl Fn(&str, &str, &str) -> Builder + 'ctx> {
let receiver_range = ctx.sema.original_range_opt(receiver.syntax())?.range; let receiver_range = ctx.sema.original_range_opt(receiver.syntax())?.range;
if ctx.source_range().end() < receiver_range.start() { if ctx.source_range().end() < receiver_range.start() {
// This shouldn't happen, yet it does. I assume this might be due to an incorrect token mapping. // This shouldn't happen, yet it does. I assume this might be due to an incorrect token
// mapping.
never!();
return None; return None;
} }
let delete_range = TextRange::new(receiver_range.start(), ctx.source_range().end()); let delete_range = TextRange::new(receiver_range.start(), ctx.source_range().end());