Repository: rust-analyzer/rust-analyzer
Pull request #8068 (2021-03-17 17:35:46)

Correctly handle `#[cfg]` on function parameters
Fixes https://github.com/rust-analyzer/rust-analyzer/issues/5649


Patch:
diff --git a/crates/hir_def/src/data.rs b/crates/hir_def/src/data.rs
index 1a27f7bf21..e976e419eb 100644
--- a/crates/hir_def/src/data.rs
+++ b/crates/hir_def/src/data.rs
@@ -9,7 +9,7 @@ use crate::{
     attr::Attrs,
     body::Expander,
     db::DefDatabase,
-    item_tree::{AssocItem, FunctionQualifier, ItemTreeId, ModItem},
+    item_tree::{AssocItem, FunctionQualifier, ItemTreeId, ModItem, Param},
     type_ref::{TypeBound, TypeRef},
     visibility::RawVisibility,
     AssocContainerId, AssocItemId, ConstId, ConstLoc, FunctionId, FunctionLoc, HasModule, ImplId,
@@ -36,19 +36,38 @@ impl FunctionData {
     pub(crate) fn fn_data_query(db: &dyn DefDatabase, func: FunctionId) -> Arc<FunctionData> {
         let loc = func.lookup(db);
         let krate = loc.container.module(db).krate;
+        let crate_graph = db.crate_graph();
+        let cfg_options = &crate_graph[krate].cfg_options;
         let item_tree = db.item_tree(loc.id.file_id);
         let func = &item_tree[loc.id.value];
 
+        let enabled_params = func
+            .params
+            .clone()
+            .filter(|&param| item_tree.attrs(db, krate, param.into()).is_cfg_enabled(cfg_options));
+
+        // If last cfg-enabled param is a `...` param, it's a varargs function.
+        let is_varargs = enabled_params
+            .clone()
+            .next_back()
+            .map_or(false, |param| matches!(item_tree[param], Param::Varargs));
+
         Arc::new(FunctionData {
             name: func.name.clone(),
-            params: func.params.iter().map(|id| item_tree[*id].clone()).collect(),
+            params: enabled_params
+                .clone()
+                .filter_map(|id| match &item_tree[id] {
+                    Param::Normal(ty) => Some(item_tree[*ty].clone()),
+                    Param::Varargs => None,
+                })
+                .collect(),
             ret_type: item_tree[func.ret_type].clone(),
             attrs: item_tree.attrs(db, krate, ModItem::from(loc.id.value).into()),
             has_self_param: func.has_self_param,
             has_body: func.has_body,
             qualifier: func.qualifier.clone(),
             is_in_extern_block: func.is_in_extern_block,
-            is_varargs: func.is_varargs,
+            is_varargs,
             visibility: item_tree[func.visibility].clone(),
         })
     }
diff --git a/crates/hir_def/src/item_tree.rs b/crates/hir_def/src/item_tree.rs
index 7bb22c4c49..90df3d9291 100644
--- a/crates/hir_def/src/item_tree.rs
+++ b/crates/hir_def/src/item_tree.rs
@@ -134,6 +134,7 @@ impl ItemTree {
                 imports,
                 extern_crates,
                 functions,
+                params,
                 structs,
                 fields,
                 unions,
@@ -157,6 +158,7 @@ impl ItemTree {
             imports.shrink_to_fit();
             extern_crates.shrink_to_fit();
             functions.shrink_to_fit();
+            params.shrink_to_fit();
             structs.shrink_to_fit();
             fields.shrink_to_fit();
             unions.shrink_to_fit();
@@ -303,6 +305,7 @@ struct ItemTreeData {
     imports: Arena<Import>,
     extern_crates: Arena<ExternCrate>,
     functions: Arena<Function>,
+    params: Arena<Param>,
     structs: Arena<Struct>,
     fields: Arena<Field>,
     unions: Arena<Union>,
@@ -334,6 +337,7 @@ pub enum AttrOwner {
 
     Variant(Idx<Variant>),
     Field(Idx<Field>),
+    Param(Idx<Param>),
 }
 
 macro_rules! from_attrs {
@@ -348,7 +352,7 @@ macro_rules! from_attrs {
     };
 }
 
-from_attrs!(ModItem(ModItem), Variant(Idx<Variant>), Field(Idx<Field>));
+from_attrs!(ModItem(ModItem), Variant(Idx<Variant>), Field(Idx<Field>), Param(Idx<Param>));
 
 /// Trait implemented by all item nodes in the item tree.
 pub trait ItemTreeNode: Clone {
@@ -484,7 +488,7 @@ macro_rules! impl_index {
     };
 }
 
-impl_index!(fields: Field, variants: Variant);
+impl_index!(fields: Field, variants: Variant, params: Param);
 
 impl Index<RawVisibilityId> for ItemTree {
     type Output = RawVisibility;
@@ -560,12 +564,17 @@ pub struct Function {
     /// Whether the function is located in an `extern` block (*not* whether it is an
     /// `extern "abi" fn`).
     pub is_in_extern_block: bool,
-    pub params: Box<[Idx<TypeRef>]>,
-    pub is_varargs: bool,
+    pub params: IdRange<Param>,
     pub ret_type: Idx<TypeRef>,
     pub ast_id: FileAstId<ast::Fn>,
 }
 
+#[derive(Debug, Clone, Eq, PartialEq)]
+pub enum Param {
+    Normal(Idx<TypeRef>),
+    Varargs,
+}
+
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct FunctionQualifier {
     pub is_default: bool,
@@ -796,6 +805,7 @@ pub struct Variant {
     pub fields: Fields,
 }
 
+/// A range of densely allocated ItemTree IDs.
 pub struct IdRange<T> {
     range: Range<u32>,
     _p: PhantomData<T>,
@@ -814,6 +824,12 @@ impl<T> Iterator for IdRange<T> {
     }
 }
 
+impl<T> DoubleEndedIterator for IdRange<T> {
+    fn next_back(&mut self) -> Option<Self::Item> {
+        self.range.next_back().map(|raw| Idx::from_raw(raw.into()))
+    }
+}
+
 impl<T> fmt::Debug for IdRange<T> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         f.debug_tuple(&format!("IdRange::<{}>", type_name::<T>())).field(&self.range).finish()
diff --git a/crates/hir_def/src/item_tree/lower.rs b/crates/hir_def/src/item_tree/lower.rs
index 7e91b991d2..3f558edd81 100644
--- a/crates/hir_def/src/item_tree/lower.rs
+++ b/crates/hir_def/src/item_tree/lower.rs
@@ -333,8 +333,8 @@ impl Ctx {
         let visibility = self.lower_visibility(func);
         let name = func.name()?.as_name();
 
-        let mut params = Vec::new();
         let mut has_self_param = false;
+        let start_param = self.next_param_idx();
         if let Some(param_list) = func.param_list() {
             if let Some(self_param) = param_list.self_param() {
                 let self_type = match self_param.ty() {
@@ -356,22 +356,25 @@ impl Ctx {
                         }
                     }
                 };
-                params.push(self_type);
+                let ty = self.data().type_refs.intern(self_type);
+                let idx = self.data().params.alloc(Param::Normal(ty));
+                self.add_attrs(idx.into(), RawAttrs::new(&self_param, &self.hygiene));
                 has_self_param = true;
             }
             for param in param_list.params() {
-                let type_ref = TypeRef::from_ast_opt(&self.body_ctx, param.ty());
-                params.push(type_ref);
-            }
-        }
-        let params = params.into_iter().map(|param| self.data().type_refs.intern(param)).collect();
-
-        let mut is_varargs = false;
-        if let Some(params) = func.param_list() {
-            if let Some(last) = params.params().last() {
-                is_varargs = last.dotdotdot_token().is_some();
+                let idx = match param.dotdotdot_token() {
+                    Some(_) => self.data().params.alloc(Param::Varargs),
+                    None => {
+                        let type_ref = TypeRef::from_ast_opt(&self.body_ctx, param.ty());
+                        let ty = self.data().type_refs.intern(type_ref);
+                        self.data().params.alloc(Param::Normal(ty))
+                    }
+                };
+                self.add_attrs(idx.into(), RawAttrs::new(&param, &self.hygiene));
             }
         }
+        let end_param = self.next_param_idx();
+        let params = IdRange::new(start_param..end_param);
 
         let ret_type = match func.ret_type().and_then(|rt| rt.ty()) {
             Some(type_ref) => TypeRef::from_ast(&self.body_ctx, type_ref),
@@ -419,7 +422,6 @@ impl Ctx {
             qualifier,
             is_in_extern_block: false,
             params,
-            is_varargs,
             ret_type,
             ast_id,
         };
@@ -682,9 +684,11 @@ impl Ctx {
             GenericsOwner::Function(func) => {
                 generics.fill(&self.body_ctx, sm, node);
                 // lower `impl Trait` in arguments
-                for param in &*func.params {
-                    let param = self.data().type_refs.lookup(*param);
-                    generics.fill_implicit_impl_trait_args(param);
+                for id in func.params.clone() {
+                    if let Param::Normal(ty) = self.data().params[id] {
+                        let ty = self.data().type_refs.lookup(ty);
+                        generics.fill_implicit_impl_trait_args(ty);
+                    }
                 }
             }
             GenericsOwner::Struct
@@ -769,6 +773,11 @@ impl Ctx {
             self.tree.data.as_ref().map_or(0, |data| data.variants.len() as u32),
         ))
     }
+    fn next_param_idx(&self) -> Idx<Param> {
+        Idx::from_raw(RawIdx::from(
+            self.tree.data.as_ref().map_or(0, |data| data.params.len() as u32),
+        ))
+    }
 }
 
 fn desugar_future_path(orig: TypeRef) -> Path {
diff --git a/crates/hir_ty/src/diagnostics/expr.rs b/crates/hir_ty/src/diagnostics/expr.rs
index 50dc40335a..3909ad3545 100644
--- a/crates/hir_ty/src/diagnostics/expr.rs
+++ b/crates/hir_ty/src/diagnostics/expr.rs
@@ -713,4 +713,38 @@ fn main() {
             "#,
         );
     }
+
+    #[test]
+    fn cfgd_out_fn_params() {
+        check_diagnostics(
+            r#"
+fn foo(#[cfg(NEVER)] x: ()) {}
+
+struct S;
+
+impl S {
+    fn method(#[cfg(NEVER)] self) {}
+    fn method2(#[cfg(NEVER)] self, arg: u8) {}
+    fn method3(self, #[cfg(NEVER)] arg: u8) {}
+}
+
+extern "C" {
+    fn fixed(fixed: u8, #[cfg(NEVER)] ...);
+    fn varargs(#[cfg(not(NEVER))] ...);
+}
+
+fn main() {
+    foo();
+    S::method();
+    S::method2(0);
+    S::method3(S);
+    S.method3();
+    unsafe {
+        fixed(0);
+        varargs(1, 2, 3);
+    }
+}
+            "#,
+        )
+    }
 }
diff --git a/crates/parser/src/grammar/params.rs b/crates/parser/src/grammar/params.rs
index e313f6fb7f..9e2f02d435 100644
--- a/crates/parser/src/grammar/params.rs
+++ b/crates/parser/src/grammar/params.rs
@@ -41,22 +41,32 @@ fn list_(p: &mut Parser, flavor: Flavor) {
         FnDef | FnTrait | FnPointer => (T!['('], T![')']),
     };
 
-    let m = p.start();
+    let list_marker = p.start();
     p.bump(bra);
 
+    let mut param_marker = None;
     if let FnDef = flavor {
         // test self_param_outer_attr
         // fn f(#[must_use] self) {}
         let m = p.start();
         attributes::outer_attrs(p);
-        opt_self_param(p, m);
+        match opt_self_param(p, m) {
+            Ok(()) => {}
+            Err(m) => param_marker = Some(m),
+        }
     }
 
     while !p.at(EOF) && !p.at(ket) {
         // test param_outer_arg
         // fn f(#[attr1] pat: Type) {}
-        let m = p.start();
-        attributes::outer_attrs(p);
+        let m = match param_marker.take() {
+            Some(m) => m,
+            None => {
+                let m = p.start();
+                attributes::outer_attrs(p);
+                m
+            }
+        };
 
         if !p.at_ts(PARAM_FIRST) {
             p.error("expected value parameter");
@@ -72,8 +82,12 @@ fn list_(p: &mut Parser, flavor: Flavor) {
         }
     }
 
+    if let Some(m) = param_marker {
+        m.abandon(p);
+    }
+
     p.expect(ket);
-    m.complete(p, PARAM_LIST);
+    list_marker.complete(p, PARAM_LIST);
 }
 
 const PARAM_FIRST: TokenSet = patterns::PATTERN_FIRST.union(types::TYPE_FIRST);
@@ -153,7 +167,7 @@ fn variadic_param(p: &mut Parser) -> bool {
 //     fn d(&'a mut self, x: i32) {}
 //     fn e(mut self) {}
 // }
-fn opt_self_param(p: &mut Parser, m: Marker) {
+fn opt_self_param(p: &mut Parser, m: Marker) -> Result<(), Marker> {
     if p.at(T![self]) || p.at(T![mut]) && p.nth(1) == T![self] {
         p.eat(T![mut]);
         self_as_name(p);
@@ -176,7 +190,7 @@ fn opt_self_param(p: &mut Parser, m: Marker) {
                 | (T![&], LIFETIME_IDENT, T![self], _)
                 | (T![&], LIFETIME_IDENT, T![mut], T![self])
         ) {
-            return m.abandon(p);
+            return Err(m);
         }
         p.bump(T![&]);
         if p.at(LIFETIME_IDENT) {
@@ -189,6 +203,7 @@ fn opt_self_param(p: &mut Parser, m: Marker) {
     if !p.at(T![')']) {
         p.expect(T![,]);
     }
+    Ok(())
 }
 
 fn self_as_name(p: &mut Parser) {


Test patch:
diff --git a/crates/syntax/test_data/parser/inline/ok/0139_param_outer_arg.rast b/crates/syntax/test_data/parser/inline/ok/0139_param_outer_arg.rast
index 495e4c51b5..a84088bf30 100644
--- a/crates/syntax/test_data/parser/inline/ok/0139_param_outer_arg.rast
+++ b/crates/syntax/test_data/parser/inline/ok/0139_param_outer_arg.rast
@@ -6,16 +6,16 @@ SOURCE_FILE@0..28
       IDENT@3..4 "f"
     PARAM_LIST@4..24
       L_PAREN@4..5 "("
-      ATTR@5..13
-        POUND@5..6 "#"
-        L_BRACK@6..7 "["
-        PATH@7..12
-          PATH_SEGMENT@7..12
-            NAME_REF@7..12
-              IDENT@7..12 "attr1"
-        R_BRACK@12..13 "]"
-      WHITESPACE@13..14 " "
-      PARAM@14..23
+      PARAM@5..23
+        ATTR@5..13
+          POUND@5..6 "#"
+          L_BRACK@6..7 "["
+          PATH@7..12
+            PATH_SEGMENT@7..12
+              NAME_REF@7..12
+                IDENT@7..12 "attr1"
+          R_BRACK@12..13 "]"
+        WHITESPACE@13..14 " "
         IDENT_PAT@14..17
           NAME@14..17
             IDENT@14..17 "pat"
diff --git a/crates/syntax/test_data/parser/ok/0051_parameter_attrs.rast b/crates/syntax/test_data/parser/ok/0051_parameter_attrs.rast
index e10521d853..88470c41c0 100644
--- a/crates/syntax/test_data/parser/ok/0051_parameter_attrs.rast
+++ b/crates/syntax/test_data/parser/ok/0051_parameter_attrs.rast
@@ -6,25 +6,25 @@ SOURCE_FILE@0..519
       IDENT@3..5 "g1"
     PARAM_LIST@5..34
       L_PAREN@5..6 "("
-      ATTR@6..14
-        POUND@6..7 "#"
-        L_BRACK@7..8 "["
-        PATH@8..13
-          PATH_SEGMENT@8..13
-            NAME_REF@8..13
-              IDENT@8..13 "attr1"
-        R_BRACK@13..14 "]"
-      WHITESPACE@14..15 " "
-      ATTR@15..23
-        POUND@15..16 "#"
-        L_BRACK@16..17 "["
-        PATH@17..22
-          PATH_SEGMENT@17..22
-            NAME_REF@17..22
-              IDENT@17..22 "attr2"
-        R_BRACK@22..23 "]"
-      WHITESPACE@23..24 " "
-      PARAM@24..33
+      PARAM@6..33
+        ATTR@6..14
+          POUND@6..7 "#"
+          L_BRACK@7..8 "["
+          PATH@8..13
+            PATH_SEGMENT@8..13
+              NAME_REF@8..13
+                IDENT@8..13 "attr1"
+          R_BRACK@13..14 "]"
+        WHITESPACE@14..15 " "
+        ATTR@15..23
+          POUND@15..16 "#"
+          L_BRACK@16..17 "["
+          PATH@17..22
+            PATH_SEGMENT@17..22
+              NAME_REF@17..22
+                IDENT@17..22 "attr2"
+          R_BRACK@22..23 "]"
+        WHITESPACE@23..24 " "
         IDENT_PAT@24..27
           NAME@24..27
             IDENT@24..27 "pat"
@@ -48,16 +48,16 @@ SOURCE_FILE@0..519
       IDENT@41..43 "g2"
     PARAM_LIST@43..59
       L_PAREN@43..44 "("
-      ATTR@44..52
-        POUND@44..45 "#"
-        L_BRACK@45..46 "["
-        PATH@46..51
-          PATH_SEGMENT@46..51
-            NAME_REF@46..51
-              IDENT@46..51 "attr1"
-        R_BRACK@51..52 "]"
-      WHITESPACE@52..53 " "
-      PARAM@53..58
+      PARAM@44..58
+        ATTR@44..52
+          POUND@44..45 "#"
+          L_BRACK@45..46 "["
+          PATH@46..51
+            PATH_SEGMENT@46..51
+              NAME_REF@46..51
+                IDENT@46..51 "attr1"
+          R_BRACK@51..52 "]"
+        WHITESPACE@52..53 " "
         IDENT_PAT@53..54
           NAME@53..54
             IDENT@53..54 "x"
@@ -203,16 +203,16 @@ SOURCE_FILE@0..519
           IDENT@193..196 "bar"
         PARAM_LIST@196..233
           L_PAREN@196..197 "("
-          ATTR@197..204
-            POUND@197..198 "#"
-            L_BRACK@198..199 "["
-            PATH@199..203
-              PATH_SEGMENT@199..203
-                NAME_REF@199..203
-                  IDENT@199..203 "attr"
-            R_BRACK@203..204 "]"
-          WHITESPACE@204..205 " "
-          PARAM@205..211
+          PARAM@197..211
+            ATTR@197..204
+              POUND@197..198 "#"
+              L_BRACK@198..199 "["
+              PATH@199..203
+                PATH_SEGMENT@199..203
+                  NAME_REF@199..203
+                    IDENT@199..203 "attr"
+              R_BRACK@203..204 "]"
+            WHITESPACE@204..205 " "
             WILDCARD_PAT@205..206
               UNDERSCORE@205..206 "_"
             COLON@206..207 ":"