6324: Improve #[cfg] diagnostics r=jonas-schievink a=jonas-schievink

Unfortunately I ran into https://github.com/rust-analyzer/rust-analyzer/issues/4058 while testing this on https://github.com/nrf-rs/nrf-hal/, so I didn't see much of it in action yet, but it does seem to work.

Co-authored-by: Jonas Schievink <jonas.schievink@ferrous-systems.com>
Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>
This commit is contained in:
bors[bot] 2020-10-23 10:38:30 +00:00 committed by GitHub
commit 81609960fa
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 816 additions and 140 deletions

1
Cargo.lock generated
View file

@ -162,6 +162,7 @@ checksum = "ed67cbde08356238e75fc4656be4749481eeffb09e19f320a25237d5221c985d"
name = "cfg"
version = "0.0.0"
dependencies = [
"expect-test",
"mbe",
"rustc-hash",
"syntax",

View file

@ -17,3 +17,4 @@ tt = { path = "../tt", version = "0.0.0" }
[dev-dependencies]
mbe = { path = "../mbe" }
syntax = { path = "../syntax" }
expect-test = "1.0"

View file

@ -2,30 +2,77 @@
//!
//! See: https://doc.rust-lang.org/reference/conditional-compilation.html#conditional-compilation
use std::slice::Iter as SliceIter;
use std::{fmt, slice::Iter as SliceIter};
use tt::SmolStr;
/// A simple configuration value passed in from the outside.
#[derive(Debug, Clone, PartialEq, Eq, Hash, Ord, PartialOrd)]
pub enum CfgAtom {
/// eg. `#[cfg(test)]`
Flag(SmolStr),
/// eg. `#[cfg(target_os = "linux")]`
///
/// Note that a key can have multiple values that are all considered "active" at the same time.
/// For example, `#[cfg(target_feature = "sse")]` and `#[cfg(target_feature = "sse2")]`.
KeyValue { key: SmolStr, value: SmolStr },
}
impl CfgAtom {
/// Returns `true` when the atom comes from the target specification.
///
/// If this returns `true`, then changing this atom requires changing the compilation target. If
/// it returns `false`, the atom might come from a build script or the build system.
pub fn is_target_defined(&self) -> bool {
match self {
CfgAtom::Flag(flag) => matches!(&**flag, "unix" | "windows"),
CfgAtom::KeyValue { key, value: _ } => matches!(
&**key,
"target_arch"
| "target_os"
| "target_env"
| "target_family"
| "target_endian"
| "target_pointer_width"
| "target_vendor" // NOTE: `target_feature` is left out since it can be configured via `-Ctarget-feature`
),
}
}
}
impl fmt::Display for CfgAtom {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
CfgAtom::Flag(name) => write!(f, "{}", name),
CfgAtom::KeyValue { key, value } => write!(f, "{} = {:?}", key, value),
}
}
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum CfgExpr {
Invalid,
Atom(SmolStr),
KeyValue { key: SmolStr, value: SmolStr },
Atom(CfgAtom),
All(Vec<CfgExpr>),
Any(Vec<CfgExpr>),
Not(Box<CfgExpr>),
}
impl From<CfgAtom> for CfgExpr {
fn from(atom: CfgAtom) -> Self {
CfgExpr::Atom(atom)
}
}
impl CfgExpr {
pub fn parse(tt: &tt::Subtree) -> CfgExpr {
next_cfg_expr(&mut tt.token_trees.iter()).unwrap_or(CfgExpr::Invalid)
}
/// Fold the cfg by querying all basic `Atom` and `KeyValue` predicates.
pub fn fold(&self, query: &dyn Fn(&SmolStr, Option<&SmolStr>) -> bool) -> Option<bool> {
pub fn fold(&self, query: &dyn Fn(&CfgAtom) -> bool) -> Option<bool> {
match self {
CfgExpr::Invalid => None,
CfgExpr::Atom(name) => Some(query(name, None)),
CfgExpr::KeyValue { key, value } => Some(query(key, Some(value))),
CfgExpr::Atom(atom) => Some(query(atom)),
CfgExpr::All(preds) => {
preds.iter().try_fold(true, |s, pred| Some(s && pred.fold(query)?))
}
@ -54,7 +101,7 @@ fn next_cfg_expr(it: &mut SliceIter<tt::TokenTree>) -> Option<CfgExpr> {
// FIXME: escape? raw string?
let value =
SmolStr::new(literal.text.trim_start_matches('"').trim_end_matches('"'));
CfgExpr::KeyValue { key: name, value }
CfgAtom::KeyValue { key: name, value }.into()
}
_ => return Some(CfgExpr::Invalid),
}
@ -70,7 +117,7 @@ fn next_cfg_expr(it: &mut SliceIter<tt::TokenTree>) -> Option<CfgExpr> {
_ => CfgExpr::Invalid,
}
}
_ => CfgExpr::Atom(name),
_ => CfgAtom::Flag(name).into(),
};
// Eat comma separator
@ -81,53 +128,3 @@ fn next_cfg_expr(it: &mut SliceIter<tt::TokenTree>) -> Option<CfgExpr> {
}
Some(ret)
}
#[cfg(test)]
mod tests {
use super::*;
use mbe::ast_to_token_tree;
use syntax::ast::{self, AstNode};
fn assert_parse_result(input: &str, expected: CfgExpr) {
let (tt, _) = {
let source_file = ast::SourceFile::parse(input).ok().unwrap();
let tt = source_file.syntax().descendants().find_map(ast::TokenTree::cast).unwrap();
ast_to_token_tree(&tt).unwrap()
};
let cfg = CfgExpr::parse(&tt);
assert_eq!(cfg, expected);
}
#[test]
fn test_cfg_expr_parser() {
assert_parse_result("#![cfg(foo)]", CfgExpr::Atom("foo".into()));
assert_parse_result("#![cfg(foo,)]", CfgExpr::Atom("foo".into()));
assert_parse_result(
"#![cfg(not(foo))]",
CfgExpr::Not(Box::new(CfgExpr::Atom("foo".into()))),
);
assert_parse_result("#![cfg(foo(bar))]", CfgExpr::Invalid);
// Only take the first
assert_parse_result(r#"#![cfg(foo, bar = "baz")]"#, CfgExpr::Atom("foo".into()));
assert_parse_result(
r#"#![cfg(all(foo, bar = "baz"))]"#,
CfgExpr::All(vec![
CfgExpr::Atom("foo".into()),
CfgExpr::KeyValue { key: "bar".into(), value: "baz".into() },
]),
);
assert_parse_result(
r#"#![cfg(any(not(), all(), , bar = "baz",))]"#,
CfgExpr::Any(vec![
CfgExpr::Not(Box::new(CfgExpr::Invalid)),
CfgExpr::All(vec![]),
CfgExpr::Invalid,
CfgExpr::KeyValue { key: "bar".into(), value: "baz".into() },
]),
);
}
}

320
crates/cfg/src/dnf.rs Normal file
View file

@ -0,0 +1,320 @@
//! Disjunctive Normal Form construction.
//!
//! Algorithm from <https://www.cs.drexel.edu/~jjohnson/2015-16/fall/CS270/Lectures/3/dnf.pdf>,
//! which would have been much easier to read if it used pattern matching. It's also missing the
//! entire "distribute ANDs over ORs" part, which is not trivial. Oh well.
//!
//! This is currently both messy and inefficient. Feel free to improve, there are unit tests.
use std::fmt;
use rustc_hash::FxHashSet;
use crate::{CfgAtom, CfgDiff, CfgExpr, CfgOptions, InactiveReason};
/// A `#[cfg]` directive in Disjunctive Normal Form (DNF).
pub struct DnfExpr {
conjunctions: Vec<Conjunction>,
}
struct Conjunction {
literals: Vec<Literal>,
}
struct Literal {
negate: bool,
var: Option<CfgAtom>, // None = Invalid
}
impl DnfExpr {
pub fn new(expr: CfgExpr) -> Self {
let builder = Builder { expr: DnfExpr { conjunctions: Vec::new() } };
builder.lower(expr.clone())
}
/// Computes a list of present or absent atoms in `opts` that cause this expression to evaluate
/// to `false`.
///
/// Note that flipping a subset of these atoms might be sufficient to make the whole expression
/// evaluate to `true`. For that, see `compute_enable_hints`.
///
/// Returns `None` when `self` is already true, or contains errors.
pub fn why_inactive(&self, opts: &CfgOptions) -> Option<InactiveReason> {
let mut res = InactiveReason { enabled: Vec::new(), disabled: Vec::new() };
for conj in &self.conjunctions {
let mut conj_is_true = true;
for lit in &conj.literals {
let atom = lit.var.as_ref()?;
let enabled = opts.enabled.contains(atom);
if lit.negate == enabled {
// Literal is false, but needs to be true for this conjunction.
conj_is_true = false;
if enabled {
res.enabled.push(atom.clone());
} else {
res.disabled.push(atom.clone());
}
}
}
if conj_is_true {
// This expression is not actually inactive.
return None;
}
}
res.enabled.sort_unstable();
res.enabled.dedup();
res.disabled.sort_unstable();
res.disabled.dedup();
Some(res)
}
/// Returns `CfgDiff` objects that would enable this directive if applied to `opts`.
pub fn compute_enable_hints<'a>(
&'a self,
opts: &'a CfgOptions,
) -> impl Iterator<Item = CfgDiff> + 'a {
// A cfg is enabled if any of `self.conjunctions` evaluate to `true`.
self.conjunctions.iter().filter_map(move |conj| {
let mut enable = FxHashSet::default();
let mut disable = FxHashSet::default();
for lit in &conj.literals {
let atom = lit.var.as_ref()?;
let enabled = opts.enabled.contains(atom);
if lit.negate && enabled {
disable.insert(atom.clone());
}
if !lit.negate && !enabled {
enable.insert(atom.clone());
}
}
// Check that this actually makes `conj` true.
for lit in &conj.literals {
let atom = lit.var.as_ref()?;
let enabled = enable.contains(atom)
|| (opts.enabled.contains(atom) && !disable.contains(atom));
if enabled == lit.negate {
return None;
}
}
if enable.is_empty() && disable.is_empty() {
return None;
}
let mut diff = CfgDiff {
enable: enable.into_iter().collect(),
disable: disable.into_iter().collect(),
};
// Undo the FxHashMap randomization for consistent output.
diff.enable.sort_unstable();
diff.disable.sort_unstable();
Some(diff)
})
}
}
impl fmt::Display for DnfExpr {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
if self.conjunctions.len() != 1 {
write!(f, "any(")?;
}
for (i, conj) in self.conjunctions.iter().enumerate() {
if i != 0 {
f.write_str(", ")?;
}
write!(f, "{}", conj)?;
}
if self.conjunctions.len() != 1 {
write!(f, ")")?;
}
Ok(())
}
}
impl Conjunction {
fn new(parts: Vec<CfgExpr>) -> Self {
let mut literals = Vec::new();
for part in parts {
match part {
CfgExpr::Invalid | CfgExpr::Atom(_) | CfgExpr::Not(_) => {
literals.push(Literal::new(part));
}
CfgExpr::All(conj) => {
// Flatten.
literals.extend(Conjunction::new(conj).literals);
}
CfgExpr::Any(_) => unreachable!("disjunction in conjunction"),
}
}
Self { literals }
}
}
impl fmt::Display for Conjunction {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
if self.literals.len() != 1 {
write!(f, "all(")?;
}
for (i, lit) in self.literals.iter().enumerate() {
if i != 0 {
f.write_str(", ")?;
}
write!(f, "{}", lit)?;
}
if self.literals.len() != 1 {
write!(f, ")")?;
}
Ok(())
}
}
impl Literal {
fn new(expr: CfgExpr) -> Self {
match expr {
CfgExpr::Invalid => Self { negate: false, var: None },
CfgExpr::Atom(atom) => Self { negate: false, var: Some(atom) },
CfgExpr::Not(expr) => match *expr {
CfgExpr::Invalid => Self { negate: true, var: None },
CfgExpr::Atom(atom) => Self { negate: true, var: Some(atom) },
_ => unreachable!("non-atom {:?}", expr),
},
CfgExpr::Any(_) | CfgExpr::All(_) => unreachable!("non-literal {:?}", expr),
}
}
}
impl fmt::Display for Literal {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
if self.negate {
write!(f, "not(")?;
}
match &self.var {
Some(var) => write!(f, "{}", var)?,
None => f.write_str("<invalid>")?,
}
if self.negate {
write!(f, ")")?;
}
Ok(())
}
}
struct Builder {
expr: DnfExpr,
}
impl Builder {
fn lower(mut self, expr: CfgExpr) -> DnfExpr {
let expr = make_nnf(expr);
let expr = make_dnf(expr);
match expr {
CfgExpr::Invalid | CfgExpr::Atom(_) | CfgExpr::Not(_) => {
self.expr.conjunctions.push(Conjunction::new(vec![expr]));
}
CfgExpr::All(conj) => {
self.expr.conjunctions.push(Conjunction::new(conj));
}
CfgExpr::Any(mut disj) => {
disj.reverse();
while let Some(conj) = disj.pop() {
match conj {
CfgExpr::Invalid | CfgExpr::Atom(_) | CfgExpr::All(_) | CfgExpr::Not(_) => {
self.expr.conjunctions.push(Conjunction::new(vec![conj]));
}
CfgExpr::Any(inner_disj) => {
// Flatten.
disj.extend(inner_disj.into_iter().rev());
}
}
}
}
}
self.expr
}
}
fn make_dnf(expr: CfgExpr) -> CfgExpr {
match expr {
CfgExpr::Invalid | CfgExpr::Atom(_) | CfgExpr::Not(_) => expr,
CfgExpr::Any(e) => CfgExpr::Any(e.into_iter().map(|expr| make_dnf(expr)).collect()),
CfgExpr::All(e) => {
let e = e.into_iter().map(|expr| make_nnf(expr)).collect::<Vec<_>>();
CfgExpr::Any(distribute_conj(&e))
}
}
}
/// Turns a conjunction of expressions into a disjunction of expressions.
fn distribute_conj(conj: &[CfgExpr]) -> Vec<CfgExpr> {
fn go(out: &mut Vec<CfgExpr>, with: &mut Vec<CfgExpr>, rest: &[CfgExpr]) {
match rest {
[head, tail @ ..] => match head {
CfgExpr::Any(disj) => {
for part in disj {
with.push(part.clone());
go(out, with, tail);
with.pop();
}
}
_ => {
with.push(head.clone());
go(out, with, tail);
with.pop();
}
},
_ => {
// Turn accumulated parts into a new conjunction.
out.push(CfgExpr::All(with.clone()));
}
}
}
let mut out = Vec::new();
let mut with = Vec::new();
go(&mut out, &mut with, conj);
out
}
fn make_nnf(expr: CfgExpr) -> CfgExpr {
match expr {
CfgExpr::Invalid | CfgExpr::Atom(_) => expr,
CfgExpr::Any(expr) => CfgExpr::Any(expr.into_iter().map(|expr| make_nnf(expr)).collect()),
CfgExpr::All(expr) => CfgExpr::All(expr.into_iter().map(|expr| make_nnf(expr)).collect()),
CfgExpr::Not(operand) => match *operand {
CfgExpr::Invalid | CfgExpr::Atom(_) => CfgExpr::Not(operand.clone()), // Original negated expr
CfgExpr::Not(expr) => {
// Remove double negation.
make_nnf(*expr)
}
// Convert negated conjunction/disjunction using DeMorgan's Law.
CfgExpr::Any(inner) => CfgExpr::All(
inner.into_iter().map(|expr| make_nnf(CfgExpr::Not(Box::new(expr)))).collect(),
),
CfgExpr::All(inner) => CfgExpr::Any(
inner.into_iter().map(|expr| make_nnf(CfgExpr::Not(Box::new(expr)))).collect(),
),
},
}
}

View file

@ -1,11 +1,17 @@
//! cfg defines conditional compiling options, `cfg` attibute parser and evaluator
mod cfg_expr;
mod dnf;
#[cfg(test)]
mod tests;
use std::fmt;
use rustc_hash::FxHashSet;
use tt::SmolStr;
pub use cfg_expr::CfgExpr;
pub use cfg_expr::{CfgAtom, CfgExpr};
pub use dnf::DnfExpr;
/// Configuration options used for conditional compilition on items with `cfg` attributes.
/// We have two kind of options in different namespaces: atomic options like `unix`, and
@ -19,33 +25,131 @@ pub use cfg_expr::CfgExpr;
/// See: https://doc.rust-lang.org/reference/conditional-compilation.html#set-configuration-options
#[derive(Debug, Clone, PartialEq, Eq, Default)]
pub struct CfgOptions {
atoms: FxHashSet<SmolStr>,
key_values: FxHashSet<(SmolStr, SmolStr)>,
enabled: FxHashSet<CfgAtom>,
}
impl CfgOptions {
pub fn check(&self, cfg: &CfgExpr) -> Option<bool> {
cfg.fold(&|key, value| match value {
None => self.atoms.contains(key),
Some(value) => self.key_values.contains(&(key.clone(), value.clone())),
})
cfg.fold(&|atom| self.enabled.contains(atom))
}
pub fn insert_atom(&mut self, key: SmolStr) {
self.atoms.insert(key);
self.enabled.insert(CfgAtom::Flag(key));
}
pub fn insert_key_value(&mut self, key: SmolStr, value: SmolStr) {
self.key_values.insert((key, value));
self.enabled.insert(CfgAtom::KeyValue { key, value });
}
pub fn append(&mut self, other: &CfgOptions) {
for atom in &other.atoms {
self.atoms.insert(atom.clone());
for atom in &other.enabled {
self.enabled.insert(atom.clone());
}
}
pub fn apply_diff(&mut self, diff: CfgDiff) {
for atom in diff.enable {
self.enabled.insert(atom);
}
for (key, value) in &other.key_values {
self.key_values.insert((key.clone(), value.clone()));
for atom in diff.disable {
self.enabled.remove(&atom);
}
}
}
pub struct CfgDiff {
// Invariants: No duplicates, no atom that's both in `enable` and `disable`.
enable: Vec<CfgAtom>,
disable: Vec<CfgAtom>,
}
impl CfgDiff {
/// Returns the total number of atoms changed by this diff.
pub fn len(&self) -> usize {
self.enable.len() + self.disable.len()
}
}
impl fmt::Display for CfgDiff {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
if !self.enable.is_empty() {
f.write_str("enable ")?;
for (i, atom) in self.enable.iter().enumerate() {
let sep = match i {
0 => "",
_ if i == self.enable.len() - 1 => " and ",
_ => ", ",
};
f.write_str(sep)?;
write!(f, "{}", atom)?;
}
if !self.disable.is_empty() {
f.write_str("; ")?;
}
}
if !self.disable.is_empty() {
f.write_str("disable ")?;
for (i, atom) in self.disable.iter().enumerate() {
let sep = match i {
0 => "",
_ if i == self.enable.len() - 1 => " and ",
_ => ", ",
};
f.write_str(sep)?;
write!(f, "{}", atom)?;
}
}
Ok(())
}
}
pub struct InactiveReason {
enabled: Vec<CfgAtom>,
disabled: Vec<CfgAtom>,
}
impl fmt::Display for InactiveReason {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
if !self.enabled.is_empty() {
for (i, atom) in self.enabled.iter().enumerate() {
let sep = match i {
0 => "",
_ if i == self.enabled.len() - 1 => " and ",
_ => ", ",
};
f.write_str(sep)?;
write!(f, "{}", atom)?;
}
let is_are = if self.enabled.len() == 1 { "is" } else { "are" };
write!(f, " {} enabled", is_are)?;
if !self.disabled.is_empty() {
f.write_str(" and ")?;
}
}
if !self.disabled.is_empty() {
for (i, atom) in self.disabled.iter().enumerate() {
let sep = match i {
0 => "",
_ if i == self.disabled.len() - 1 => " and ",
_ => ", ",
};
f.write_str(sep)?;
write!(f, "{}", atom)?;
}
let is_are = if self.disabled.len() == 1 { "is" } else { "are" };
write!(f, " {} disabled", is_are)?;
}
Ok(())
}
}

193
crates/cfg/src/tests.rs Normal file
View file

@ -0,0 +1,193 @@
use expect_test::{expect, Expect};
use mbe::ast_to_token_tree;
use syntax::{ast, AstNode};
use crate::{CfgAtom, CfgExpr, CfgOptions, DnfExpr};
fn assert_parse_result(input: &str, expected: CfgExpr) {
let (tt, _) = {
let source_file = ast::SourceFile::parse(input).ok().unwrap();
let tt = source_file.syntax().descendants().find_map(ast::TokenTree::cast).unwrap();
ast_to_token_tree(&tt).unwrap()
};
let cfg = CfgExpr::parse(&tt);
assert_eq!(cfg, expected);
}
fn check_dnf(input: &str, expect: Expect) {
let (tt, _) = {
let source_file = ast::SourceFile::parse(input).ok().unwrap();
let tt = source_file.syntax().descendants().find_map(ast::TokenTree::cast).unwrap();
ast_to_token_tree(&tt).unwrap()
};
let cfg = CfgExpr::parse(&tt);
let actual = format!("#![cfg({})]", DnfExpr::new(cfg));
expect.assert_eq(&actual);
}
fn check_why_inactive(input: &str, opts: &CfgOptions, expect: Expect) {
let (tt, _) = {
let source_file = ast::SourceFile::parse(input).ok().unwrap();
let tt = source_file.syntax().descendants().find_map(ast::TokenTree::cast).unwrap();
ast_to_token_tree(&tt).unwrap()
};
let cfg = CfgExpr::parse(&tt);
let dnf = DnfExpr::new(cfg);
let why_inactive = dnf.why_inactive(opts).unwrap().to_string();
expect.assert_eq(&why_inactive);
}
#[track_caller]
fn check_enable_hints(input: &str, opts: &CfgOptions, expected_hints: &[&str]) {
let (tt, _) = {
let source_file = ast::SourceFile::parse(input).ok().unwrap();
let tt = source_file.syntax().descendants().find_map(ast::TokenTree::cast).unwrap();
ast_to_token_tree(&tt).unwrap()
};
let cfg = CfgExpr::parse(&tt);
let dnf = DnfExpr::new(cfg);
let hints = dnf.compute_enable_hints(opts).map(|diff| diff.to_string()).collect::<Vec<_>>();
assert_eq!(hints, expected_hints);
}
#[test]
fn test_cfg_expr_parser() {
assert_parse_result("#![cfg(foo)]", CfgAtom::Flag("foo".into()).into());
assert_parse_result("#![cfg(foo,)]", CfgAtom::Flag("foo".into()).into());
assert_parse_result(
"#![cfg(not(foo))]",
CfgExpr::Not(Box::new(CfgAtom::Flag("foo".into()).into())),
);
assert_parse_result("#![cfg(foo(bar))]", CfgExpr::Invalid);
// Only take the first
assert_parse_result(r#"#![cfg(foo, bar = "baz")]"#, CfgAtom::Flag("foo".into()).into());
assert_parse_result(
r#"#![cfg(all(foo, bar = "baz"))]"#,
CfgExpr::All(vec![
CfgAtom::Flag("foo".into()).into(),
CfgAtom::KeyValue { key: "bar".into(), value: "baz".into() }.into(),
]),
);
assert_parse_result(
r#"#![cfg(any(not(), all(), , bar = "baz",))]"#,
CfgExpr::Any(vec![
CfgExpr::Not(Box::new(CfgExpr::Invalid)),
CfgExpr::All(vec![]),
CfgExpr::Invalid,
CfgAtom::KeyValue { key: "bar".into(), value: "baz".into() }.into(),
]),
);
}
#[test]
fn smoke() {
check_dnf("#![cfg(test)]", expect![[r#"#![cfg(test)]"#]]);
check_dnf("#![cfg(not(test))]", expect![[r#"#![cfg(not(test))]"#]]);
check_dnf("#![cfg(not(not(test)))]", expect![[r#"#![cfg(test)]"#]]);
check_dnf("#![cfg(all(a, b))]", expect![[r#"#![cfg(all(a, b))]"#]]);
check_dnf("#![cfg(any(a, b))]", expect![[r#"#![cfg(any(a, b))]"#]]);
check_dnf("#![cfg(not(a))]", expect![[r#"#![cfg(not(a))]"#]]);
}
#[test]
fn distribute() {
check_dnf("#![cfg(all(any(a, b), c))]", expect![[r#"#![cfg(any(all(a, c), all(b, c)))]"#]]);
check_dnf("#![cfg(all(c, any(a, b)))]", expect![[r#"#![cfg(any(all(c, a), all(c, b)))]"#]]);
check_dnf(
"#![cfg(all(any(a, b), any(c, d)))]",
expect![[r#"#![cfg(any(all(a, c), all(a, d), all(b, c), all(b, d)))]"#]],
);
check_dnf(
"#![cfg(all(any(a, b, c), any(d, e, f), g))]",
expect![[
r#"#![cfg(any(all(a, d, g), all(a, e, g), all(a, f, g), all(b, d, g), all(b, e, g), all(b, f, g), all(c, d, g), all(c, e, g), all(c, f, g)))]"#
]],
);
}
#[test]
fn demorgan() {
check_dnf("#![cfg(not(all(a, b)))]", expect![[r#"#![cfg(any(not(a), not(b)))]"#]]);
check_dnf("#![cfg(not(any(a, b)))]", expect![[r#"#![cfg(all(not(a), not(b)))]"#]]);
check_dnf("#![cfg(not(all(not(a), b)))]", expect![[r#"#![cfg(any(a, not(b)))]"#]]);
check_dnf("#![cfg(not(any(a, not(b))))]", expect![[r#"#![cfg(all(not(a), b))]"#]]);
}
#[test]
fn nested() {
check_dnf("#![cfg(all(any(a), not(all(any(b)))))]", expect![[r#"#![cfg(all(a, not(b)))]"#]]);
check_dnf("#![cfg(any(any(a, b)))]", expect![[r#"#![cfg(any(a, b))]"#]]);
check_dnf("#![cfg(not(any(any(a, b))))]", expect![[r#"#![cfg(all(not(a), not(b)))]"#]]);
check_dnf("#![cfg(all(all(a, b)))]", expect![[r#"#![cfg(all(a, b))]"#]]);
check_dnf("#![cfg(not(all(all(a, b))))]", expect![[r#"#![cfg(any(not(a), not(b)))]"#]]);
}
#[test]
fn hints() {
let mut opts = CfgOptions::default();
check_enable_hints("#![cfg(test)]", &opts, &["enable test"]);
check_enable_hints("#![cfg(not(test))]", &opts, &[]);
check_enable_hints("#![cfg(any(a, b))]", &opts, &["enable a", "enable b"]);
check_enable_hints("#![cfg(any(b, a))]", &opts, &["enable b", "enable a"]);
check_enable_hints("#![cfg(all(a, b))]", &opts, &["enable a and b"]);
opts.insert_atom("test".into());
check_enable_hints("#![cfg(test)]", &opts, &[]);
check_enable_hints("#![cfg(not(test))]", &opts, &["disable test"]);
}
/// Tests that we don't suggest hints for cfgs that express an inconsistent formula.
#[test]
fn hints_impossible() {
let mut opts = CfgOptions::default();
check_enable_hints("#![cfg(all(test, not(test)))]", &opts, &[]);
opts.insert_atom("test".into());
check_enable_hints("#![cfg(all(test, not(test)))]", &opts, &[]);
}
#[test]
fn why_inactive() {
let mut opts = CfgOptions::default();
opts.insert_atom("test".into());
opts.insert_atom("test2".into());
check_why_inactive("#![cfg(a)]", &opts, expect![["a is disabled"]]);
check_why_inactive("#![cfg(not(test))]", &opts, expect![["test is enabled"]]);
check_why_inactive(
"#![cfg(all(not(test), not(test2)))]",
&opts,
expect![["test and test2 are enabled"]],
);
check_why_inactive("#![cfg(all(a, b))]", &opts, expect![["a and b are disabled"]]);
check_why_inactive(
"#![cfg(all(not(test), a))]",
&opts,
expect![["test is enabled and a is disabled"]],
);
check_why_inactive(
"#![cfg(all(not(test), test2, a))]",
&opts,
expect![["test is enabled and a is disabled"]],
);
check_why_inactive(
"#![cfg(all(not(test), not(test2), a))]",
&opts,
expect![["test and test2 are enabled and a is disabled"]],
);
}

View file

@ -125,12 +125,20 @@ impl Attrs {
AttrQuery { attrs: self, key }
}
pub fn cfg(&self) -> impl Iterator<Item = CfgExpr> + '_ {
pub fn cfg(&self) -> Option<CfgExpr> {
// FIXME: handle cfg_attr :-)
self.by_key("cfg").tt_values().map(CfgExpr::parse)
let mut cfgs = self.by_key("cfg").tt_values().map(CfgExpr::parse).collect::<Vec<_>>();
match cfgs.len() {
0 => None,
1 => Some(cfgs.pop().unwrap()),
_ => Some(CfgExpr::All(cfgs)),
}
}
pub(crate) fn is_cfg_enabled(&self, cfg_options: &CfgOptions) -> bool {
self.cfg().all(|cfg| cfg_options.check(&cfg) != Some(false))
match self.cfg() {
None => true,
Some(cfg) => cfg_options.check(&cfg) != Some(false),
}
}
}

View file

@ -1,11 +1,12 @@
//! Diagnostics produced by `hir_def`.
use std::any::Any;
use stdx::format_to;
use cfg::{CfgExpr, CfgOptions, DnfExpr};
use hir_expand::diagnostics::{Diagnostic, DiagnosticCode};
use syntax::{ast, AstPtr, SyntaxNodePtr};
use hir_expand::{HirFileId, InFile};
use syntax::{ast, AstPtr, SyntaxNodePtr};
// Diagnostic: unresolved-module
//
@ -94,6 +95,8 @@ impl Diagnostic for UnresolvedImport {
pub struct InactiveCode {
pub file: HirFileId,
pub node: SyntaxNodePtr,
pub cfg: CfgExpr,
pub opts: CfgOptions,
}
impl Diagnostic for InactiveCode {
@ -101,8 +104,14 @@ impl Diagnostic for InactiveCode {
DiagnosticCode("inactive-code")
}
fn message(&self) -> String {
// FIXME: say *why* it is configured out
"code is inactive due to #[cfg] directives".to_string()
let inactive = DnfExpr::new(self.cfg.clone()).why_inactive(&self.opts);
let mut buf = "code is inactive due to #[cfg] directives".to_string();
if let Some(inactive) = inactive {
format_to!(buf, ": {}", inactive);
}
buf
}
fn display_source(&self) -> InFile<SyntaxNodePtr> {
InFile::new(self.file, self.node.clone())

View file

@ -283,6 +283,7 @@ pub enum ModuleSource {
}
mod diagnostics {
use cfg::{CfgExpr, CfgOptions};
use hir_expand::diagnostics::DiagnosticSink;
use hir_expand::hygiene::Hygiene;
use hir_expand::InFile;
@ -299,7 +300,7 @@ mod diagnostics {
UnresolvedImport { ast: AstId<ast::Use>, index: usize },
UnconfiguredCode { ast: InFile<SyntaxNodePtr> },
UnconfiguredCode { ast: InFile<SyntaxNodePtr>, cfg: CfgExpr, opts: CfgOptions },
}
#[derive(Debug, PartialEq, Eq)]
@ -341,8 +342,10 @@ mod diagnostics {
pub(super) fn unconfigured_code(
container: LocalModuleId,
ast: InFile<SyntaxNodePtr>,
cfg: CfgExpr,
opts: CfgOptions,
) -> Self {
Self { in_module: container, kind: DiagnosticKind::UnconfiguredCode { ast } }
Self { in_module: container, kind: DiagnosticKind::UnconfiguredCode { ast, cfg, opts } }
}
pub(super) fn add_to(
@ -395,8 +398,13 @@ mod diagnostics {
}
}
DiagnosticKind::UnconfiguredCode { ast } => {
sink.push(InactiveCode { file: ast.file_id, node: ast.value.clone() });
DiagnosticKind::UnconfiguredCode { ast, cfg, opts } => {
sink.push(InactiveCode {
file: ast.file_id,
node: ast.value.clone(),
cfg: cfg.clone(),
opts: opts.clone(),
});
}
}
}

View file

@ -6,7 +6,7 @@
use std::iter;
use base_db::{CrateId, FileId, ProcMacroId};
use cfg::CfgOptions;
use cfg::{CfgExpr, CfgOptions};
use hir_expand::InFile;
use hir_expand::{
ast_id_map::FileAstId,
@ -900,7 +900,8 @@ impl ModCollector<'_, '_> {
// `#[macro_use] extern crate` is hoisted to imports macros before collecting
// any other items.
for item in items {
if self.is_cfg_enabled(self.item_tree.attrs((*item).into())) {
let attrs = self.item_tree.attrs((*item).into());
if attrs.cfg().map_or(true, |cfg| self.is_cfg_enabled(&cfg)) {
if let ModItem::ExternCrate(id) = item {
let import = self.item_tree[*id].clone();
if import.is_macro_use {
@ -912,9 +913,11 @@ impl ModCollector<'_, '_> {
for &item in items {
let attrs = self.item_tree.attrs(item.into());
if !self.is_cfg_enabled(attrs) {
self.emit_unconfigured_diagnostic(item);
continue;
if let Some(cfg) = attrs.cfg() {
if !self.is_cfg_enabled(&cfg) {
self.emit_unconfigured_diagnostic(item, &cfg);
continue;
}
}
let module =
ModuleId { krate: self.def_collector.def_map.krate, local_id: self.module_id };
@ -1321,20 +1324,22 @@ impl ModCollector<'_, '_> {
}
}
fn is_cfg_enabled(&self, attrs: &Attrs) -> bool {
attrs.is_cfg_enabled(self.def_collector.cfg_options)
fn is_cfg_enabled(&self, cfg: &CfgExpr) -> bool {
self.def_collector.cfg_options.check(cfg) != Some(false)
}
fn emit_unconfigured_diagnostic(&mut self, item: ModItem) {
fn emit_unconfigured_diagnostic(&mut self, item: ModItem, cfg: &CfgExpr) {
let ast_id = item.ast_id(self.item_tree);
let id_map = self.def_collector.db.ast_id_map(self.file_id);
let syntax_ptr = id_map.get(ast_id).syntax_node_ptr();
let ast_node = InFile::new(self.file_id, syntax_ptr);
self.def_collector
.def_map
.diagnostics
.push(DefDiagnostic::unconfigured_code(self.module_id, ast_node));
self.def_collector.def_map.diagnostics.push(DefDiagnostic::unconfigured_code(
self.module_id,
ast_node,
cfg.clone(),
self.def_collector.cfg_options.clone(),
));
}
}

View file

@ -129,3 +129,25 @@ fn unresolved_module() {
",
);
}
#[test]
fn inactive_item() {
// Additional tests in `cfg` crate. This only tests disabled cfgs.
check_diagnostics(
r#"
//- /lib.rs
#[cfg(no)] pub fn f() {}
//^^^^^^^^^^^^^^^^^^^^^^^^ code is inactive due to #[cfg] directives: no is disabled
#[cfg(no)] #[cfg(no2)] mod m;
//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ code is inactive due to #[cfg] directives: no and no2 are disabled
#[cfg(all(not(a), b))] enum E {}
//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ code is inactive due to #[cfg] directives: b is disabled
#[cfg(feature = "std")] use std;
//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ code is inactive due to #[cfg] directives: feature = "std" is disabled
"#,
);
}

View file

@ -2128,7 +2128,7 @@ fn foo_<|>test() {}
ignore: false,
},
},
cfg_exprs: [],
cfg: None,
},
),
]
@ -2166,7 +2166,7 @@ mod tests<|> {
kind: TestMod {
path: "tests",
},
cfg_exprs: [],
cfg: None,
},
),
]

View file

@ -15,7 +15,7 @@ use crate::{display::ToNav, FileId, NavigationTarget};
pub struct Runnable {
pub nav: NavigationTarget,
pub kind: RunnableKind,
pub cfg_exprs: Vec<CfgExpr>,
pub cfg: Option<CfgExpr>,
}
#[derive(Debug, Clone)]
@ -168,7 +168,7 @@ fn runnable_fn(
};
let attrs = Attrs::from_attrs_owner(sema.db, InFile::new(HirFileId::from(file_id), &fn_def));
let cfg_exprs = attrs.cfg().collect();
let cfg = attrs.cfg();
let nav = if let RunnableKind::DocTest { .. } = kind {
NavigationTarget::from_doc_commented(
@ -179,7 +179,7 @@ fn runnable_fn(
} else {
NavigationTarget::from_named(sema.db, InFile::new(file_id.into(), &fn_def))
};
Some(Runnable { nav, kind, cfg_exprs })
Some(Runnable { nav, kind, cfg })
}
#[derive(Debug, Copy, Clone)]
@ -255,9 +255,9 @@ fn runnable_mod(
.join("::");
let attrs = Attrs::from_attrs_owner(sema.db, InFile::new(HirFileId::from(file_id), &module));
let cfg_exprs = attrs.cfg().collect();
let cfg = attrs.cfg();
let nav = module_def.to_nav(sema.db);
Some(Runnable { nav, kind: RunnableKind::TestMod { path }, cfg_exprs })
Some(Runnable { nav, kind: RunnableKind::TestMod { path }, cfg })
}
// We could create runnables for modules with number_of_test_submodules > 0,
@ -348,7 +348,7 @@ fn bench() {}
docs: None,
},
kind: Bin,
cfg_exprs: [],
cfg: None,
},
Runnable {
nav: NavigationTarget {
@ -373,7 +373,7 @@ fn bench() {}
ignore: false,
},
},
cfg_exprs: [],
cfg: None,
},
Runnable {
nav: NavigationTarget {
@ -398,7 +398,7 @@ fn bench() {}
ignore: true,
},
},
cfg_exprs: [],
cfg: None,
},
Runnable {
nav: NavigationTarget {
@ -420,7 +420,7 @@ fn bench() {}
"bench",
),
},
cfg_exprs: [],
cfg: None,
},
]
"#]],
@ -507,7 +507,7 @@ fn should_have_no_runnable_6() {}
docs: None,
},
kind: Bin,
cfg_exprs: [],
cfg: None,
},
Runnable {
nav: NavigationTarget {
@ -527,7 +527,7 @@ fn should_have_no_runnable_6() {}
"should_have_runnable",
),
},
cfg_exprs: [],
cfg: None,
},
Runnable {
nav: NavigationTarget {
@ -547,7 +547,7 @@ fn should_have_no_runnable_6() {}
"should_have_runnable_1",
),
},
cfg_exprs: [],
cfg: None,
},
Runnable {
nav: NavigationTarget {
@ -567,7 +567,7 @@ fn should_have_no_runnable_6() {}
"should_have_runnable_2",
),
},
cfg_exprs: [],
cfg: None,
},
]
"#]],
@ -609,7 +609,7 @@ impl Data {
docs: None,
},
kind: Bin,
cfg_exprs: [],
cfg: None,
},
Runnable {
nav: NavigationTarget {
@ -629,7 +629,7 @@ impl Data {
"Data::foo",
),
},
cfg_exprs: [],
cfg: None,
},
]
"#]],
@ -668,7 +668,7 @@ mod test_mod {
kind: TestMod {
path: "test_mod",
},
cfg_exprs: [],
cfg: None,
},
Runnable {
nav: NavigationTarget {
@ -693,7 +693,7 @@ mod test_mod {
ignore: false,
},
},
cfg_exprs: [],
cfg: None,
},
]
"#]],
@ -748,7 +748,7 @@ mod root_tests {
kind: TestMod {
path: "root_tests::nested_tests_0",
},
cfg_exprs: [],
cfg: None,
},
Runnable {
nav: NavigationTarget {
@ -768,7 +768,7 @@ mod root_tests {
kind: TestMod {
path: "root_tests::nested_tests_0::nested_tests_1",
},
cfg_exprs: [],
cfg: None,
},
Runnable {
nav: NavigationTarget {
@ -793,7 +793,7 @@ mod root_tests {
ignore: false,
},
},
cfg_exprs: [],
cfg: None,
},
Runnable {
nav: NavigationTarget {
@ -818,7 +818,7 @@ mod root_tests {
ignore: false,
},
},
cfg_exprs: [],
cfg: None,
},
Runnable {
nav: NavigationTarget {
@ -838,7 +838,7 @@ mod root_tests {
kind: TestMod {
path: "root_tests::nested_tests_0::nested_tests_2",
},
cfg_exprs: [],
cfg: None,
},
Runnable {
nav: NavigationTarget {
@ -863,7 +863,7 @@ mod root_tests {
ignore: false,
},
},
cfg_exprs: [],
cfg: None,
},
]
"#]],
@ -906,12 +906,14 @@ fn test_foo1() {}
ignore: false,
},
},
cfg_exprs: [
KeyValue {
key: "feature",
value: "foo",
},
],
cfg: Some(
Atom(
KeyValue {
key: "feature",
value: "foo",
},
),
),
},
]
"#]],
@ -954,20 +956,24 @@ fn test_foo1() {}
ignore: false,
},
},
cfg_exprs: [
cfg: Some(
All(
[
KeyValue {
key: "feature",
value: "foo",
},
KeyValue {
key: "feature",
value: "bar",
},
Atom(
KeyValue {
key: "feature",
value: "foo",
},
),
Atom(
KeyValue {
key: "feature",
value: "bar",
},
),
],
),
],
),
},
]
"#]],

View file

@ -1,6 +1,6 @@
//! See `CargoTargetSpec`
use cfg::CfgExpr;
use cfg::{CfgAtom, CfgExpr};
use ide::{FileId, RunnableKind, TestId};
use project_model::{self, TargetKind};
use vfs::AbsPathBuf;
@ -24,7 +24,7 @@ impl CargoTargetSpec {
snap: &GlobalStateSnapshot,
spec: Option<CargoTargetSpec>,
kind: &RunnableKind,
cfgs: &[CfgExpr],
cfg: &Option<CfgExpr>,
) -> Result<(Vec<String>, Vec<String>)> {
let mut args = Vec::new();
let mut extra_args = Vec::new();
@ -87,7 +87,7 @@ impl CargoTargetSpec {
args.push("--all-features".to_string());
} else {
let mut features = Vec::new();
for cfg in cfgs {
if let Some(cfg) = cfg.as_ref() {
required_features(cfg, &mut features);
}
for feature in &snap.config.cargo.features {
@ -160,7 +160,9 @@ impl CargoTargetSpec {
/// Fill minimal features needed
fn required_features(cfg_expr: &CfgExpr, features: &mut Vec<String>) {
match cfg_expr {
CfgExpr::KeyValue { key, value } if key == "feature" => features.push(value.to_string()),
CfgExpr::Atom(CfgAtom::KeyValue { key, value }) if key == "feature" => {
features.push(value.to_string())
}
CfgExpr::All(preds) => {
preds.iter().for_each(|cfg| required_features(cfg, features));
}

View file

@ -762,7 +762,7 @@ pub(crate) fn runnable(
let workspace_root = spec.as_ref().map(|it| it.workspace_root.clone());
let target = spec.as_ref().map(|s| s.target.clone());
let (cargo_args, executable_args) =
CargoTargetSpec::runnable_args(snap, spec, &runnable.kind, &runnable.cfg_exprs)?;
CargoTargetSpec::runnable_args(snap, spec, &runnable.kind, &runnable.cfg)?;
let label = runnable.label(target);
let location = location_link(snap, None, runnable.nav)?;