Remove unnecessary CfgFlag definition in project-model

crates/base-db/src/input.rs

@@ -690,6 +690,14 @@ impl Env {
     pub fn extend_from_other(&mut self, other: &Env) {
         self.entries.extend(other.entries.iter().map(|(x, y)| (x.to_owned(), y.to_owned())));
     }
+
+    pub fn is_empty(&self) -> bool {
+        self.entries.is_empty()
+    }
+
+    pub fn insert(&mut self, k: impl Into<String>, v: impl Into<String>) -> Option<String> {
+        self.entries.insert(k.into(), v.into())
+    }
 }
 
 impl From<Env> for Vec<(String, String)> {
@@ -700,6 +708,15 @@ impl From<Env> for Vec<(String, String)> {
     }
 }
 
+impl<'a> IntoIterator for &'a Env {
+    type Item = (&'a String, &'a String);
+    type IntoIter = std::collections::hash_map::Iter<'a, String, String>;
+
+    fn into_iter(self) -> Self::IntoIter {
+        self.entries.iter()
+    }
+}
+
 #[derive(Debug)]
 pub struct CyclicDependenciesError {
     path: Vec<(CrateId, Option<CrateDisplayName>)>,

crates/cfg/src/lib.rs

@@ -108,6 +108,14 @@ impl<'a> IntoIterator for &'a CfgOptions {
     }
 }
 
+impl FromIterator<CfgAtom> for CfgOptions {
+    fn from_iter<T: IntoIterator<Item = CfgAtom>>(iter: T) -> Self {
+        let mut options = CfgOptions::default();
+        options.extend(iter);
+        options
+    }
+}
+
 #[derive(Default, Clone, Debug, PartialEq, Eq)]
 pub struct CfgDiff {
     // Invariants: No duplicates, no atom that's both in `enable` and `disable`.

crates/project-model/src/build_dependencies.rs

@@ -1,14 +1,16 @@
-//! Workspace information we get from cargo consists of two pieces. The first is
+//! Logic to invoke `cargo` for building build-dependencies (build scripts and proc-macros) as well as
-//! the output of `cargo metadata`. The second is the output of running
+//! executing the build scripts to fetch required dependency information (`OUT_DIR` env var, extra
-//! `build.rs` files (`OUT_DIR` env var, extra cfg flags) and compiling proc
+//! cfg flags, etc).
-//! macro.
 //!
-//! This module implements this second part. We use "build script" terminology
+//! In essence this just invokes `cargo` with the appropriate output format which we consume,
-//! here, but it covers procedural macros as well.
+//! but if enabled we will also use `RUSTC_WRAPPER` to only compile the build scripts and
+//! proc-macros and skip everything else.
 
-use std::{cell::RefCell, io, mem, path, process::Command};
+use std::{cell::RefCell, io, mem, process::Command};
 
+use base_db::Env;
 use cargo_metadata::{camino::Utf8Path, Message};
+use cfg::CfgAtom;
 use itertools::Itertools;
 use la_arena::ArenaMap;
 use paths::{AbsPath, AbsPathBuf, Utf8PathBuf};
@@ -17,7 +19,7 @@ use serde::Deserialize;
 use toolchain::Tool;
 
 use crate::{
-    cfg::CfgFlag, utf8_stdout, CargoConfig, CargoFeatures, CargoWorkspace, InvocationLocation,
+    utf8_stdout, CargoConfig, CargoFeatures, CargoWorkspace, InvocationLocation,
     InvocationStrategy, ManifestPath, Package, Sysroot, TargetKind,
 };
 
@@ -32,12 +34,12 @@ pub struct WorkspaceBuildScripts {
 #[derive(Debug, Clone, Default, PartialEq, Eq)]
 pub(crate) struct BuildScriptOutput {
     /// List of config flags defined by this package's build script.
-    pub(crate) cfgs: Vec<CfgFlag>,
+    pub(crate) cfgs: Vec<CfgAtom>,
     /// List of cargo-related environment variables with their value.
     ///
     /// If the package has a build script which defines environment variables,
     /// they can also be found here.
-    pub(crate) envs: Vec<(String, String)>,
+    pub(crate) envs: Env,
     /// Directory where a build script might place its output.
     pub(crate) out_dir: Option<AbsPathBuf>,
     /// Path to the proc-macro library file if this package exposes proc-macros.
@@ -45,7 +47,7 @@ pub(crate) struct BuildScriptOutput {
 }
 
 impl BuildScriptOutput {
-    fn is_unchanged(&self) -> bool {
+    fn is_empty(&self) -> bool {
         self.cfgs.is_empty()
             && self.envs.is_empty()
             && self.out_dir.is_none()
@@ -54,85 +56,6 @@ impl BuildScriptOutput {
 }
 
 impl WorkspaceBuildScripts {
-    fn build_command(
-        config: &CargoConfig,
-        allowed_features: &FxHashSet<String>,
-        manifest_path: &ManifestPath,
-        sysroot: &Sysroot,
-    ) -> io::Result<Command> {
-        let mut cmd = match config.run_build_script_command.as_deref() {
-            Some([program, args @ ..]) => {
-                let mut cmd = Command::new(program);
-                cmd.args(args);
-                cmd
-            }
-            _ => {
-                let mut cmd = sysroot.tool(Tool::Cargo);
-
-                cmd.args(["check", "--quiet", "--workspace", "--message-format=json"]);
-                cmd.args(&config.extra_args);
-
-                cmd.arg("--manifest-path");
-                cmd.arg(manifest_path);
-
-                if let Some(target_dir) = &config.target_dir {
-                    cmd.arg("--target-dir").arg(target_dir);
-                }
-
-                // --all-targets includes tests, benches and examples in addition to the
-                // default lib and bins. This is an independent concept from the --target
-                // flag below.
-                if config.all_targets {
-                    cmd.arg("--all-targets");
-                }
-
-                if let Some(target) = &config.target {
-                    cmd.args(["--target", target]);
-                }
-
-                match &config.features {
-                    CargoFeatures::All => {
-                        cmd.arg("--all-features");
-                    }
-                    CargoFeatures::Selected { features, no_default_features } => {
-                        if *no_default_features {
-                            cmd.arg("--no-default-features");
-                        }
-                        if !features.is_empty() {
-                            cmd.arg("--features");
-                            cmd.arg(
-                                features
-                                    .iter()
-                                    .filter(|&feat| allowed_features.contains(feat))
-                                    .join(","),
-                            );
-                        }
-                    }
-                }
-
-                if manifest_path.is_rust_manifest() {
-                    cmd.arg("-Zscript");
-                }
-
-                cmd.arg("--keep-going");
-
-                cmd
-            }
-        };
-
-        cmd.envs(&config.extra_env);
-        if config.wrap_rustc_in_build_scripts {
-            // Setup RUSTC_WRAPPER to point to `rust-analyzer` binary itself. We use
-            // that to compile only proc macros and build scripts during the initial
-            // `cargo check`.
-            let myself = std::env::current_exe()?;
-            cmd.env("RUSTC_WRAPPER", myself);
-            cmd.env("RA_RUSTC_WRAPPER", "1");
-        }
-
-        Ok(cmd)
-    }
-
     /// Runs the build scripts for the given workspace
     pub(crate) fn run_for_workspace(
         config: &CargoConfig,
@@ -141,17 +64,19 @@ impl WorkspaceBuildScripts {
         sysroot: &Sysroot,
     ) -> io::Result<WorkspaceBuildScripts> {
         let current_dir = match &config.invocation_location {
-            InvocationLocation::Root(root) if config.run_build_script_command.is_some() => {
+            InvocationLocation::Root(root) if config.run_build_script_command.is_some() => root,
-                root.as_path()
-            }
             _ => workspace.workspace_root(),
-        }
+        };
-        .as_ref();
 
         let allowed_features = workspace.workspace_features();
-        let cmd =
+        let cmd = Self::build_command(
-            Self::build_command(config, &allowed_features, workspace.manifest_path(), sysroot)?;
+            config,
-        Self::run_per_ws(cmd, workspace, current_dir, progress)
+            &allowed_features,
+            workspace.manifest_path(),
+            current_dir,
+            sysroot,
+        )?;
+        Self::run_per_ws(cmd, workspace, progress)
     }
 
     /// Runs the build scripts by invoking the configured command *once*.
@@ -178,6 +103,7 @@ impl WorkspaceBuildScripts {
             &Default::default(),
             // This is not gonna be used anyways, so just construct a dummy here
             &ManifestPath::try_from(workspace_root.clone()).unwrap(),
+            current_dir,
             &Sysroot::empty(),
         )?;
         // NB: Cargo.toml could have been modified between `cargo metadata` and
@@ -206,7 +132,6 @@ impl WorkspaceBuildScripts {
 
         let errors = Self::run_command(
             cmd,
-            current_dir.as_path().as_ref(),
             |package, cb| {
                 if let Some(&(package, workspace)) = by_id.get(package) {
                     cb(&workspaces[workspace][package].name, &mut res[workspace].outputs[package]);
@@ -225,7 +150,7 @@ impl WorkspaceBuildScripts {
             for (idx, workspace) in workspaces.iter().enumerate() {
                 for package in workspace.packages() {
                     let package_build_data = &mut res[idx].outputs[package];
-                    if !package_build_data.is_unchanged() {
+                    if !package_build_data.is_empty() {
                         tracing::info!(
                             "{}: {package_build_data:?}",
                             workspace[package].manifest.parent(),
@@ -238,151 +163,6 @@ impl WorkspaceBuildScripts {
         Ok(res)
     }
 
-    fn run_per_ws(
-        cmd: Command,
-        workspace: &CargoWorkspace,
-        current_dir: &path::Path,
-        progress: &dyn Fn(String),
-    ) -> io::Result<WorkspaceBuildScripts> {
-        let mut res = WorkspaceBuildScripts::default();
-        let outputs = &mut res.outputs;
-        // NB: Cargo.toml could have been modified between `cargo metadata` and
-        // `cargo check`. We shouldn't assume that package ids we see here are
-        // exactly those from `config`.
-        let mut by_id: FxHashMap<String, Package> = FxHashMap::default();
-        for package in workspace.packages() {
-            outputs.insert(package, BuildScriptOutput::default());
-            by_id.insert(workspace[package].id.clone(), package);
-        }
-
-        res.error = Self::run_command(
-            cmd,
-            current_dir,
-            |package, cb| {
-                if let Some(&package) = by_id.get(package) {
-                    cb(&workspace[package].name, &mut outputs[package]);
-                }
-            },
-            progress,
-        )?;
-
-        if tracing::enabled!(tracing::Level::INFO) {
-            for package in workspace.packages() {
-                let package_build_data = &outputs[package];
-                if !package_build_data.is_unchanged() {
-                    tracing::info!(
-                        "{}: {package_build_data:?}",
-                        workspace[package].manifest.parent(),
-                    );
-                }
-            }
-        }
-
-        Ok(res)
-    }
-
-    fn run_command(
-        mut cmd: Command,
-        current_dir: &path::Path,
-        // ideally this would be something like:
-        // with_output_for: impl FnMut(&str, dyn FnOnce(&mut BuildScriptOutput)),
-        // but owned trait objects aren't a thing
-        mut with_output_for: impl FnMut(&str, &mut dyn FnMut(&str, &mut BuildScriptOutput)),
-        progress: &dyn Fn(String),
-    ) -> io::Result<Option<String>> {
-        let errors = RefCell::new(String::new());
-        let push_err = |err: &str| {
-            let mut e = errors.borrow_mut();
-            e.push_str(err);
-            e.push('\n');
-        };
-
-        tracing::info!("Running build scripts in {}: {:?}", current_dir.display(), cmd);
-        cmd.current_dir(current_dir);
-        let output = stdx::process::spawn_with_streaming_output(
-            cmd,
-            &mut |line| {
-                // Copy-pasted from existing cargo_metadata. It seems like we
-                // should be using serde_stacker here?
-                let mut deserializer = serde_json::Deserializer::from_str(line);
-                deserializer.disable_recursion_limit();
-                let message = Message::deserialize(&mut deserializer)
-                    .unwrap_or_else(|_| Message::TextLine(line.to_owned()));
-
-                match message {
-                    Message::BuildScriptExecuted(mut message) => {
-                        with_output_for(&message.package_id.repr, &mut |name, data| {
-                            progress(format!("running build-script: {name}"));
-                            let cfgs = {
-                                let mut acc = Vec::new();
-                                for cfg in &message.cfgs {
-                                    match cfg.parse::<CfgFlag>() {
-                                        Ok(it) => acc.push(it),
-                                        Err(err) => {
-                                            push_err(&format!(
-                                                "invalid cfg from cargo-metadata: {err}"
-                                            ));
-                                            return;
-                                        }
-                                    };
-                                }
-                                acc
-                            };
-                            if !message.env.is_empty() {
-                                data.envs = mem::take(&mut message.env);
-                            }
-                            // cargo_metadata crate returns default (empty) path for
-                            // older cargos, which is not absolute, so work around that.
-                            let out_dir = mem::take(&mut message.out_dir);
-                            if !out_dir.as_str().is_empty() {
-                                let out_dir = AbsPathBuf::assert(out_dir);
-                                // inject_cargo_env(package, package_build_data);
-                                data.envs.push(("OUT_DIR".to_owned(), out_dir.as_str().to_owned()));
-                                data.out_dir = Some(out_dir);
-                                data.cfgs = cfgs;
-                            }
-                        });
-                    }
-                    Message::CompilerArtifact(message) => {
-                        with_output_for(&message.package_id.repr, &mut |name, data| {
-                            progress(format!("building proc-macros: {name}"));
-                            if message.target.kind.iter().any(|k| k == "proc-macro") {
-                                // Skip rmeta file
-                                if let Some(filename) =
-                                    message.filenames.iter().find(|name| is_dylib(name))
-                                {
-                                    let filename = AbsPath::assert(filename);
-                                    data.proc_macro_dylib_path = Some(filename.to_owned());
-                                }
-                            }
-                        });
-                    }
-                    Message::CompilerMessage(message) => {
-                        progress(message.target.name);
-
-                        if let Some(diag) = message.message.rendered.as_deref() {
-                            push_err(diag);
-                        }
-                    }
-                    Message::BuildFinished(_) => {}
-                    Message::TextLine(_) => {}
-                    _ => {}
-                }
-            },
-            &mut |line| {
-                push_err(line);
-            },
-        )?;
-
-        let errors = if !output.status.success() {
-            let errors = errors.into_inner();
-            Some(if errors.is_empty() { "cargo check failed".to_owned() } else { errors })
-        } else {
-            None
-        };
-        Ok(errors)
-    }
-
     pub fn error(&self) -> Option<&str> {
         self.error.as_deref()
     }
@@ -391,6 +171,7 @@ impl WorkspaceBuildScripts {
         self.outputs.get(idx)
     }
 
+    /// Assembles build script outputs for the rustc crates via `--print target-libdir`.
     pub(crate) fn rustc_crates(
         rustc: &CargoWorkspace,
         current_dir: &AbsPath,
@@ -457,7 +238,7 @@ impl WorkspaceBuildScripts {
             if tracing::enabled!(tracing::Level::INFO) {
                 for package in rustc.packages() {
                     let package_build_data = &bs.outputs[package];
-                    if !package_build_data.is_unchanged() {
+                    if !package_build_data.is_empty() {
                         tracing::info!(
                             "{}: {package_build_data:?}",
                             rustc[package].manifest.parent(),
@@ -472,6 +253,226 @@ impl WorkspaceBuildScripts {
         }
         bs
     }
+
+    fn run_per_ws(
+        cmd: Command,
+        workspace: &CargoWorkspace,
+        progress: &dyn Fn(String),
+    ) -> io::Result<WorkspaceBuildScripts> {
+        let mut res = WorkspaceBuildScripts::default();
+        let outputs = &mut res.outputs;
+        // NB: Cargo.toml could have been modified between `cargo metadata` and
+        // `cargo check`. We shouldn't assume that package ids we see here are
+        // exactly those from `config`.
+        let mut by_id: FxHashMap<String, Package> = FxHashMap::default();
+        for package in workspace.packages() {
+            outputs.insert(package, BuildScriptOutput::default());
+            by_id.insert(workspace[package].id.clone(), package);
+        }
+
+        res.error = Self::run_command(
+            cmd,
+            |package, cb| {
+                if let Some(&package) = by_id.get(package) {
+                    cb(&workspace[package].name, &mut outputs[package]);
+                }
+            },
+            progress,
+        )?;
+
+        if tracing::enabled!(tracing::Level::INFO) {
+            for package in workspace.packages() {
+                let package_build_data = &outputs[package];
+                if !package_build_data.is_empty() {
+                    tracing::info!(
+                        "{}: {package_build_data:?}",
+                        workspace[package].manifest.parent(),
+                    );
+                }
+            }
+        }
+
+        Ok(res)
+    }
+
+    fn run_command(
+        cmd: Command,
+        // ideally this would be something like:
+        // with_output_for: impl FnMut(&str, dyn FnOnce(&mut BuildScriptOutput)),
+        // but owned trait objects aren't a thing
+        mut with_output_for: impl FnMut(&str, &mut dyn FnMut(&str, &mut BuildScriptOutput)),
+        progress: &dyn Fn(String),
+    ) -> io::Result<Option<String>> {
+        let errors = RefCell::new(String::new());
+        let push_err = |err: &str| {
+            let mut e = errors.borrow_mut();
+            e.push_str(err);
+            e.push('\n');
+        };
+
+        tracing::info!("Running build scripts: {:?}", cmd);
+        let output = stdx::process::spawn_with_streaming_output(
+            cmd,
+            &mut |line| {
+                // Copy-pasted from existing cargo_metadata. It seems like we
+                // should be using serde_stacker here?
+                let mut deserializer = serde_json::Deserializer::from_str(line);
+                deserializer.disable_recursion_limit();
+                let message = Message::deserialize(&mut deserializer)
+                    .unwrap_or_else(|_| Message::TextLine(line.to_owned()));
+
+                match message {
+                    Message::BuildScriptExecuted(mut message) => {
+                        with_output_for(&message.package_id.repr, &mut |name, data| {
+                            progress(format!("running build-script: {name}"));
+                            let cfgs = {
+                                let mut acc = Vec::new();
+                                for cfg in &message.cfgs {
+                                    match crate::parse_cfg(cfg) {
+                                        Ok(it) => acc.push(it),
+                                        Err(err) => {
+                                            push_err(&format!(
+                                                "invalid cfg from cargo-metadata: {err}"
+                                            ));
+                                            return;
+                                        }
+                                    };
+                                }
+                                acc
+                            };
+                            data.envs.extend(message.env.drain(..));
+                            // cargo_metadata crate returns default (empty) path for
+                            // older cargos, which is not absolute, so work around that.
+                            let out_dir = mem::take(&mut message.out_dir);
+                            if !out_dir.as_str().is_empty() {
+                                let out_dir = AbsPathBuf::assert(out_dir);
+                                // inject_cargo_env(package, package_build_data);
+                                data.envs.insert("OUT_DIR", out_dir.as_str());
+                                data.out_dir = Some(out_dir);
+                                data.cfgs = cfgs;
+                            }
+                        });
+                    }
+                    Message::CompilerArtifact(message) => {
+                        with_output_for(&message.package_id.repr, &mut |name, data| {
+                            progress(format!("building proc-macros: {name}"));
+                            if message.target.kind.iter().any(|k| k == "proc-macro") {
+                                // Skip rmeta file
+                                if let Some(filename) =
+                                    message.filenames.iter().find(|file| is_dylib(file))
+                                {
+                                    let filename = AbsPath::assert(filename);
+                                    data.proc_macro_dylib_path = Some(filename.to_owned());
+                                }
+                            }
+                        });
+                    }
+                    Message::CompilerMessage(message) => {
+                        progress(message.target.name);
+
+                        if let Some(diag) = message.message.rendered.as_deref() {
+                            push_err(diag);
+                        }
+                    }
+                    Message::BuildFinished(_) => {}
+                    Message::TextLine(_) => {}
+                    _ => {}
+                }
+            },
+            &mut |line| {
+                push_err(line);
+            },
+        )?;
+
+        let errors = if !output.status.success() {
+            let errors = errors.into_inner();
+            Some(if errors.is_empty() { "cargo check failed".to_owned() } else { errors })
+        } else {
+            None
+        };
+        Ok(errors)
+    }
+
+    fn build_command(
+        config: &CargoConfig,
+        allowed_features: &FxHashSet<String>,
+        manifest_path: &ManifestPath,
+        current_dir: &AbsPath,
+        sysroot: &Sysroot,
+    ) -> io::Result<Command> {
+        let mut cmd = match config.run_build_script_command.as_deref() {
+            Some([program, args @ ..]) => {
+                let mut cmd = Command::new(program);
+                cmd.args(args);
+                cmd
+            }
+            _ => {
+                let mut cmd = sysroot.tool(Tool::Cargo);
+
+                cmd.args(["check", "--quiet", "--workspace", "--message-format=json"]);
+                cmd.args(&config.extra_args);
+
+                cmd.arg("--manifest-path");
+                cmd.arg(manifest_path);
+
+                if let Some(target_dir) = &config.target_dir {
+                    cmd.arg("--target-dir").arg(target_dir);
+                }
+
+                // --all-targets includes tests, benches and examples in addition to the
+                // default lib and bins. This is an independent concept from the --target
+                // flag below.
+                if config.all_targets {
+                    cmd.arg("--all-targets");
+                }
+
+                if let Some(target) = &config.target {
+                    cmd.args(["--target", target]);
+                }
+
+                match &config.features {
+                    CargoFeatures::All => {
+                        cmd.arg("--all-features");
+                    }
+                    CargoFeatures::Selected { features, no_default_features } => {
+                        if *no_default_features {
+                            cmd.arg("--no-default-features");
+                        }
+                        if !features.is_empty() {
+                            cmd.arg("--features");
+                            cmd.arg(
+                                features
+                                    .iter()
+                                    .filter(|&feat| allowed_features.contains(feat))
+                                    .join(","),
+                            );
+                        }
+                    }
+                }
+
+                if manifest_path.is_rust_manifest() {
+                    cmd.arg("-Zscript");
+                }
+
+                cmd.arg("--keep-going");
+
+                cmd
+            }
+        };
+
+        cmd.current_dir(current_dir);
+        cmd.envs(&config.extra_env);
+        if config.wrap_rustc_in_build_scripts {
+            // Setup RUSTC_WRAPPER to point to `rust-analyzer` binary itself. We use
+            // that to compile only proc macros and build scripts during the initial
+            // `cargo check`.
+            let myself = std::env::current_exe()?;
+            cmd.env("RUSTC_WRAPPER", myself);
+            cmd.env("RA_RUSTC_WRAPPER", "1");
+        }
+
+        Ok(cmd)
+    }
 }
 
 // FIXME: Find a better way to know if it is a dylib.

crates/project-model/src/cfg.rs

@@ -1,100 +0,0 @@
-//! Parsing of CfgFlags as command line arguments, as in
-//!
-//! rustc main.rs --cfg foo --cfg 'feature="bar"'
-use std::{fmt, str::FromStr};
-
-use cfg::{CfgDiff, CfgOptions};
-use intern::Symbol;
-use rustc_hash::FxHashMap;
-use serde::Serialize;
-
-#[derive(Clone, Eq, PartialEq, Debug, Serialize)]
-pub enum CfgFlag {
-    Atom(String),
-    KeyValue { key: String, value: String },
-}
-
-impl FromStr for CfgFlag {
-    type Err = String;
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
-        let res = match s.split_once('=') {
-            Some((key, value)) => {
-                if !(value.starts_with('"') && value.ends_with('"')) {
-                    return Err(format!("Invalid cfg ({s:?}), value should be in quotes"));
-                }
-                let key = key.to_owned();
-                let value = value[1..value.len() - 1].to_string();
-                CfgFlag::KeyValue { key, value }
-            }
-            None => CfgFlag::Atom(s.into()),
-        };
-        Ok(res)
-    }
-}
-
-impl<'de> serde::Deserialize<'de> for CfgFlag {
-    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
-    where
-        D: serde::Deserializer<'de>,
-    {
-        String::deserialize(deserializer)?.parse().map_err(serde::de::Error::custom)
-    }
-}
-
-impl Extend<CfgFlag> for CfgOptions {
-    fn extend<T: IntoIterator<Item = CfgFlag>>(&mut self, iter: T) {
-        for cfg_flag in iter {
-            match cfg_flag {
-                CfgFlag::Atom(it) => self.insert_atom(Symbol::intern(&it)),
-                CfgFlag::KeyValue { key, value } => {
-                    self.insert_key_value(Symbol::intern(&key), Symbol::intern(&value))
-                }
-            }
-        }
-    }
-}
-
-impl FromIterator<CfgFlag> for CfgOptions {
-    fn from_iter<T: IntoIterator<Item = CfgFlag>>(iter: T) -> Self {
-        let mut this = CfgOptions::default();
-        this.extend(iter);
-        this
-    }
-}
-
-impl fmt::Display for CfgFlag {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match self {
-            CfgFlag::Atom(atom) => f.write_str(atom),
-            CfgFlag::KeyValue { key, value } => {
-                f.write_str(key)?;
-                f.write_str("=")?;
-                f.write_str(value)
-            }
-        }
-    }
-}
-
-/// A set of cfg-overrides per crate.
-#[derive(Default, Debug, Clone, Eq, PartialEq)]
-pub struct CfgOverrides {
-    /// A global set of overrides matching all crates.
-    pub global: CfgDiff,
-    /// A set of overrides matching specific crates.
-    pub selective: FxHashMap<String, CfgDiff>,
-}
-
-impl CfgOverrides {
-    pub fn len(&self) -> usize {
-        self.global.len() + self.selective.values().map(|it| it.len()).sum::<usize>()
-    }
-
-    pub fn apply(&self, cfg_options: &mut CfgOptions, name: &str) {
-        if !self.global.is_empty() {
-            cfg_options.apply_diff(self.global.clone());
-        };
-        if let Some(diff) = self.selective.get(name) {
-            cfg_options.apply_diff(diff.clone());
-        };
-    }
-}

crates/project-model/src/lib.rs

@@ -15,9 +15,8 @@
 //!   procedural macros).
 //! * Lowering of concrete model to a [`base_db::CrateGraph`]
 
-mod build_scripts;
+mod build_dependencies;
 mod cargo_workspace;
-mod cfg;
 mod env;
 mod manifest_path;
 pub mod project_json;
@@ -41,12 +40,11 @@ use paths::{AbsPath, AbsPathBuf, Utf8PathBuf};
 use rustc_hash::FxHashSet;
 
 pub use crate::{
-    build_scripts::WorkspaceBuildScripts,
+    build_dependencies::WorkspaceBuildScripts,
     cargo_workspace::{
         CargoConfig, CargoFeatures, CargoWorkspace, Package, PackageData, PackageDependency,
         RustLibSource, Target, TargetData, TargetKind,
     },
-    cfg::CfgOverrides,
     manifest_path::ManifestPath,
     project_json::{ProjectJson, ProjectJsonData},
     sysroot::Sysroot,
@@ -201,3 +199,42 @@ pub enum InvocationLocation {
     #[default]
     Workspace,
 }
+
+/// A set of cfg-overrides per crate.
+#[derive(Default, Debug, Clone, Eq, PartialEq)]
+pub struct CfgOverrides {
+    /// A global set of overrides matching all crates.
+    pub global: cfg::CfgDiff,
+    /// A set of overrides matching specific crates.
+    pub selective: rustc_hash::FxHashMap<String, cfg::CfgDiff>,
+}
+
+impl CfgOverrides {
+    pub fn len(&self) -> usize {
+        self.global.len() + self.selective.values().map(|it| it.len()).sum::<usize>()
+    }
+
+    pub fn apply(&self, cfg_options: &mut cfg::CfgOptions, name: &str) {
+        if !self.global.is_empty() {
+            cfg_options.apply_diff(self.global.clone());
+        };
+        if let Some(diff) = self.selective.get(name) {
+            cfg_options.apply_diff(diff.clone());
+        };
+    }
+}
+
+fn parse_cfg(s: &str) -> Result<cfg::CfgAtom, String> {
+    let res = match s.split_once('=') {
+        Some((key, value)) => {
+            if !(value.starts_with('"') && value.ends_with('"')) {
+                return Err(format!("Invalid cfg ({s:?}), value should be in quotes"));
+            }
+            let key = intern::Symbol::intern(key);
+            let value = intern::Symbol::intern(&value[1..value.len() - 1]);
+            cfg::CfgAtom::KeyValue { key, value }
+        }
+        None => cfg::CfgAtom::Flag(intern::Symbol::intern(s)),
+    };
+    Ok(res)
+}

crates/project-model/src/project_json.rs

@@ -50,12 +50,13 @@
 //! rust-project.json over time via configuration request!)
 
 use base_db::{CrateDisplayName, CrateName};
+use cfg::CfgAtom;
 use paths::{AbsPath, AbsPathBuf, Utf8PathBuf};
 use rustc_hash::FxHashMap;
 use serde::{de, Deserialize, Serialize};
 use span::Edition;
 
-use crate::{cfg::CfgFlag, ManifestPath, TargetKind};
+use crate::{ManifestPath, TargetKind};
 
 /// Roots and crates that compose this Rust project.
 #[derive(Clone, Debug, Eq, PartialEq)]
@@ -82,7 +83,7 @@ pub struct Crate {
     pub(crate) edition: Edition,
     pub(crate) version: Option<String>,
     pub(crate) deps: Vec<Dep>,
-    pub(crate) cfg: Vec<CfgFlag>,
+    pub(crate) cfg: Vec<CfgAtom>,
     pub(crate) target: Option<String>,
     pub(crate) env: FxHashMap<String, String>,
     pub(crate) proc_macro_dylib_path: Option<AbsPathBuf>,
@@ -319,7 +320,8 @@ struct CrateData {
     version: Option<semver::Version>,
     deps: Vec<Dep>,
     #[serde(default)]
-    cfg: Vec<CfgFlag>,
+    #[serde(with = "cfg_")]
+    cfg: Vec<CfgAtom>,
     target: Option<String>,
     #[serde(default)]
     env: FxHashMap<String, String>,
@@ -334,6 +336,33 @@ struct CrateData {
     build: Option<BuildData>,
 }
 
+mod cfg_ {
+    use cfg::CfgAtom;
+    use serde::{Deserialize, Serialize};
+
+    pub(super) fn deserialize<'de, D>(deserializer: D) -> Result<Vec<CfgAtom>, D::Error>
+    where
+        D: serde::Deserializer<'de>,
+    {
+        let cfg: Vec<String> = Vec::deserialize(deserializer)?;
+        cfg.into_iter().map(|it| crate::parse_cfg(&it).map_err(serde::de::Error::custom)).collect()
+    }
+    pub(super) fn serialize<S>(cfg: &[CfgAtom], serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        cfg.iter()
+            .map(|cfg| match cfg {
+                CfgAtom::Flag(flag) => flag.as_str().to_owned(),
+                CfgAtom::KeyValue { key, value } => {
+                    format!("{}=\"{}\"", key.as_str(), value.as_str())
+                }
+            })
+            .collect::<Vec<String>>()
+            .serialize(serializer)
+    }
+}
+
 #[derive(Serialize, Deserialize, Debug, Clone, Eq, PartialEq)]
 #[serde(rename = "edition")]
 enum EditionData {

crates/project-model/src/rustc_cfg.rs

@@ -1,10 +1,12 @@
 //! Runs `rustc --print cfg` to get built-in cfg flags.
 
 use anyhow::Context;
+use cfg::CfgAtom;
+use intern::Symbol;
 use rustc_hash::FxHashMap;
 use toolchain::Tool;
 
-use crate::{cfg::CfgFlag, utf8_stdout, ManifestPath, Sysroot};
+use crate::{utf8_stdout, ManifestPath, Sysroot};
 
 /// Determines how `rustc --print cfg` is discovered and invoked.
 pub(crate) enum RustcCfgConfig<'a> {
@@ -20,15 +22,15 @@ pub(crate) fn get(
     target: Option<&str>,
     extra_env: &FxHashMap<String, String>,
     config: RustcCfgConfig<'_>,
-) -> Vec<CfgFlag> {
+) -> Vec<CfgAtom> {
     let _p = tracing::info_span!("rustc_cfg::get").entered();
-    let mut res = Vec::with_capacity(6 * 2 + 1);
+    let mut res: Vec<_> = Vec::with_capacity(6 * 2 + 1);
 
     // Some nightly-only cfgs, which are required for stdlib
-    res.push(CfgFlag::Atom("target_thread_local".into()));
+    res.push(CfgAtom::Flag(Symbol::intern("target_thread_local")));
     for ty in ["8", "16", "32", "64", "cas", "ptr"] {
         for key in ["target_has_atomic", "target_has_atomic_load_store"] {
-            res.push(CfgFlag::KeyValue { key: key.to_owned(), value: ty.into() });
+            res.push(CfgAtom::KeyValue { key: Symbol::intern(key), value: Symbol::intern(ty) });
         }
     }
 
@@ -42,8 +44,7 @@ pub(crate) fn get(
         }
     };
 
-    let rustc_cfgs =
+    let rustc_cfgs = rustc_cfgs.lines().map(crate::parse_cfg).collect::<Result<Vec<_>, _>>();
-        rustc_cfgs.lines().map(|it| it.parse::<CfgFlag>()).collect::<Result<Vec<_>, _>>();
 
     match rustc_cfgs {
         Ok(rustc_cfgs) => {

crates/project-model/src/workspace.rs

@@ -20,16 +20,15 @@ use tracing::instrument;
 use triomphe::Arc;
 
 use crate::{
-    build_scripts::BuildScriptOutput,
+    build_dependencies::BuildScriptOutput,
     cargo_workspace::{DepKind, PackageData, RustLibSource},
-    cfg::{CfgFlag, CfgOverrides},
     env::{cargo_config_env, inject_cargo_env, inject_cargo_package_env, inject_rustc_tool_env},
     project_json::{Crate, CrateArrayIdx},
     rustc_cfg::{self, RustcCfgConfig},
     sysroot::{SysrootCrate, SysrootMode},
     target_data_layout::{self, RustcDataLayoutConfig},
-    utf8_stdout, CargoConfig, CargoWorkspace, InvocationStrategy, ManifestPath, Package,
+    utf8_stdout, CargoConfig, CargoWorkspace, CfgOverrides, InvocationStrategy, ManifestPath,
-    ProjectJson, ProjectManifest, Sysroot, TargetData, TargetKind, WorkspaceBuildScripts,
+    Package, ProjectJson, ProjectManifest, Sysroot, TargetData, TargetKind, WorkspaceBuildScripts,
 };
 use tracing::{debug, error, info};
 
@@ -55,7 +54,7 @@ pub struct ProjectWorkspace {
     /// `rustc --print cfg`.
     // FIXME: make this a per-crate map, as, eg, build.rs might have a
     // different target.
-    pub rustc_cfg: Vec<CfgFlag>,
+    pub rustc_cfg: Vec<CfgAtom>,
     /// The toolchain version used by this workspace.
     pub toolchain: Option<Version>,
     /// The target data layout queried for workspace.
@@ -842,7 +841,7 @@ impl ProjectWorkspace {
 
 #[instrument(skip_all)]
 fn project_json_to_crate_graph(
-    rustc_cfg: Vec<CfgFlag>,
+    rustc_cfg: Vec<CfgAtom>,
     load: FileLoader<'_>,
     project: &ProjectJson,
     sysroot: &Sysroot,
@@ -854,8 +853,8 @@ fn project_json_to_crate_graph(
     let (public_deps, libproc_macro) =
         sysroot_to_crate_graph(crate_graph, sysroot, rustc_cfg.clone(), load);
 
-    let r_a_cfg_flag = CfgFlag::Atom("rust_analyzer".to_owned());
+    let r_a_cfg_flag = CfgAtom::Flag(sym::rust_analyzer.clone());
-    let mut cfg_cache: FxHashMap<&str, Vec<CfgFlag>> = FxHashMap::default();
+    let mut cfg_cache: FxHashMap<&str, Vec<CfgAtom>> = FxHashMap::default();
 
     let idx_to_crate_id: FxHashMap<CrateArrayIdx, CrateId> = project
         .crates()
@@ -962,7 +961,7 @@ fn cargo_to_crate_graph(
     rustc: Option<&(CargoWorkspace, WorkspaceBuildScripts)>,
     cargo: &CargoWorkspace,
     sysroot: &Sysroot,
-    rustc_cfg: Vec<CfgFlag>,
+    rustc_cfg: Vec<CfgAtom>,
     override_cfg: &CfgOverrides,
     build_scripts: &WorkspaceBuildScripts,
 ) -> (CrateGraph, ProcMacroPaths) {
@@ -1145,7 +1144,7 @@ fn cargo_to_crate_graph(
 }
 
 fn detached_file_to_crate_graph(
-    rustc_cfg: Vec<CfgFlag>,
+    rustc_cfg: Vec<CfgAtom>,
     load: FileLoader<'_>,
     detached_file: &ManifestPath,
     sysroot: &Sysroot,
@@ -1308,11 +1307,10 @@ fn add_target_crate_root(
         None
     } else {
         let mut potential_cfg_options = cfg_options.clone();
-        potential_cfg_options.extend(
+        potential_cfg_options.extend(pkg.features.iter().map(|feat| CfgAtom::KeyValue {
-            pkg.features
+            key: sym::feature.clone(),
-                .iter()
+            value: Symbol::intern(feat.0),
-                .map(|feat| CfgFlag::KeyValue { key: "feature".into(), value: feat.0.into() }),
+        }));
-        );
         Some(potential_cfg_options)
     };
     let cfg_options = {
@@ -1378,7 +1376,7 @@ impl SysrootPublicDeps {
 fn sysroot_to_crate_graph(
     crate_graph: &mut CrateGraph,
     sysroot: &Sysroot,
-    rustc_cfg: Vec<CfgFlag>,
+    rustc_cfg: Vec<CfgAtom>,
     load: FileLoader<'_>,
 ) -> (SysrootPublicDeps, Option<CrateId>) {
     let _p = tracing::info_span!("sysroot_to_crate_graph").entered();