Skip to content

Commit 4069f01

Browse files
Gate #[test] expansion under cfg(test).
This will mean users opting to not activate `cfg(test)` will lose IDE experience on them, which is quite unfortunate, but this is unavoidable if we want to avoid false positives on e.g. diagnostics. The real fix is to provide IDE experience even for cfg'ed out code, but this is out of scope for this PR.
1 parent 42d4a17 commit 4069f01

File tree

4 files changed

+45
-6
lines changed

4 files changed

+45
-6
lines changed

crates/cfg/src/lib.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,10 @@ impl CfgOptions {
4949
cfg.fold(&|atom| self.enabled.contains(atom))
5050
}
5151

52+
pub fn check_atom(&self, cfg: &CfgAtom) -> bool {
53+
self.enabled.contains(cfg)
54+
}
55+
5256
pub fn insert_atom(&mut self, key: Symbol) {
5357
self.enabled.insert(CfgAtom::Flag(key));
5458
}

crates/hir-def/src/nameres/collector.rs

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
use std::{cmp::Ordering, iter, mem, ops::Not};
77

88
use base_db::{CrateId, CrateOrigin, Dependency, LangCrateOrigin};
9-
use cfg::{CfgExpr, CfgOptions};
9+
use cfg::{CfgAtom, CfgExpr, CfgOptions};
1010
use either::Either;
1111
use hir_expand::{
1212
attrs::{Attr, AttrId},
@@ -1307,13 +1307,23 @@ impl DefCollector<'_> {
13071307
};
13081308

13091309
// Skip #[test]/#[bench] expansion, which would merely result in more memory usage
1310-
// due to duplicating functions into macro expansions
1310+
// due to duplicating functions into macro expansions, but only if `cfg(test)` is active,
1311+
// otherwise they are expanded to nothing and this can impact e.g. diagnostics (due to things
1312+
// being cfg'ed out).
1313+
// Ideally we will just expand them to nothing here. But we are only collecting macro calls,
1314+
// not expanding them, so we have no way to do that.
13111315
if matches!(
13121316
def.kind,
13131317
MacroDefKind::BuiltInAttr(_, expander)
13141318
if expander.is_test() || expander.is_bench()
13151319
) {
1316-
return recollect_without(self);
1320+
let crate_graph = self.db.crate_graph();
1321+
let test_is_active = crate_graph[self.def_map.krate]
1322+
.cfg_options
1323+
.check_atom(&CfgAtom::Flag(sym::test.clone()));
1324+
if test_is_active {
1325+
return recollect_without(self);
1326+
}
13171327
}
13181328

13191329
let call_id = || {

crates/hir-expand/src/builtin/attr_macro.rs

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
//! Builtin attributes.
2+
use cfg::CfgAtom;
23
use intern::sym;
34
use span::{MacroCallId, Span};
45

56
use crate::{db::ExpandDatabase, name, tt, ExpandResult, MacroCallKind};
67

8+
use super::quote;
9+
710
macro_rules! register_builtin {
811
($(($name:ident, $variant:ident) => $expand:ident),* ) => {
912
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
@@ -52,15 +55,15 @@ impl BuiltinAttrExpander {
5255
}
5356

5457
register_builtin! {
55-
(bench, Bench) => dummy_attr_expand,
58+
(bench, Bench) => dummy_gate_test_expand,
5659
(cfg_accessible, CfgAccessible) => dummy_attr_expand,
5760
(cfg_eval, CfgEval) => dummy_attr_expand,
5861
(derive, Derive) => derive_expand,
5962
// derive const is equivalent to derive for our proposes.
6063
(derive_const, DeriveConst) => derive_expand,
6164
(global_allocator, GlobalAllocator) => dummy_attr_expand,
62-
(test, Test) => dummy_attr_expand,
63-
(test_case, TestCase) => dummy_attr_expand
65+
(test, Test) => dummy_gate_test_expand,
66+
(test_case, TestCase) => dummy_gate_test_expand
6467
}
6568

6669
pub fn find_builtin_attr(ident: &name::Name) -> Option<BuiltinAttrExpander> {
@@ -76,6 +79,27 @@ fn dummy_attr_expand(
7679
ExpandResult::ok(tt.clone())
7780
}
7881

82+
fn dummy_gate_test_expand(
83+
db: &dyn ExpandDatabase,
84+
id: MacroCallId,
85+
tt: &tt::Subtree,
86+
span: Span,
87+
) -> ExpandResult<tt::Subtree> {
88+
let loc = db.lookup_intern_macro_call(id);
89+
let crate_graph = db.crate_graph();
90+
let test_is_active =
91+
crate_graph[loc.krate].cfg_options.check_atom(&CfgAtom::Flag(sym::test.clone()));
92+
if test_is_active {
93+
ExpandResult::ok(tt.clone())
94+
} else {
95+
let result = quote::quote! { span=>
96+
#[cfg(test)]
97+
#tt
98+
};
99+
ExpandResult::ok(result)
100+
}
101+
}
102+
79103
/// We generate a very specific expansion here, as we do not actually expand the `#[derive]` attribute
80104
/// itself in name res, but we do want to expand it to something for the IDE layer, so that the input
81105
/// derive attributes can be downmapped, and resolved as proper paths.

crates/hir-expand/src/builtin/quote.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ macro_rules! quote_impl__ {
117117
($span:ident < ) => {$crate::builtin::quote::__quote!(@PUNCT($span) '<')};
118118
($span:ident > ) => {$crate::builtin::quote::__quote!(@PUNCT($span) '>')};
119119
($span:ident ! ) => {$crate::builtin::quote::__quote!(@PUNCT($span) '!')};
120+
($span:ident # ) => {$crate::builtin::quote::__quote!(@PUNCT($span) '#')};
120121

121122
($span:ident $first:tt $($tail:tt)+ ) => {
122123
{

0 commit comments

Comments
 (0)