Skip to content

Commit

Permalink
Auto merge of #109005 - Nilstrieb:dont-forgor-too-much-from-cfg, r=pe…
Browse files Browse the repository at this point in the history
…trochenkov

Remember names of `cfg`-ed out items to mention them in diagnostics

# Examples

## `serde::Deserialize` without the `derive` feature (a classic beginner mistake)

I had to slightly modify serde so that it uses explicit re-exports instead of a glob re-export. (Update: a serde PR was merged that adds the manual re-exports)

```
error[E0433]: failed to resolve: could not find `Serialize` in `serde`
   --> src/main.rs:1:17
    |
1   | #[derive(serde::Serialize)]
    |                 ^^^^^^^^^ could not find `Serialize` in `serde`
    |
note: crate `serde` has an item named `Serialize` but it is inactive because its cfg predicate evaluated to false
   --> /home/gh-Nilstrieb/.cargo/registry/src/index.crates.io-6f17d22bba15001f/serde-1.0.160/src/lib.rs:343:1
    |
343 | #[cfg(feature = "serde_derive")]
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
344 | pub use serde_derive::{Deserialize, Serialize};
    |                                     ^^^^^^^^^
    = note: the item is gated behind the `serde_derive` feature
    = note: see https://doc.rust-lang.org/cargo/reference/features.html for how to activate a crate's feature
```
(the suggestion is not ideal but that's serde's fault)

I already tested the metadata size impact locally by compiling the `windows` crate without any features. `800k`  -> `809k`

r? `@ghost`
  • Loading branch information
bors committed Jun 7, 2023
2 parents b2807b2 + a647ba2 commit a97c36d
Show file tree
Hide file tree
Showing 30 changed files with 599 additions and 84 deletions.
17 changes: 17 additions & 0 deletions compiler/rustc_ast/src/expand/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,20 @@
//! Definitions shared by macros / syntax extensions and e.g. `rustc_middle`.

use rustc_span::{def_id::DefId, symbol::Ident};

use crate::MetaItem;

pub mod allocator;

#[derive(Debug, Clone, Encodable, Decodable, HashStable_Generic)]
pub struct StrippedCfgItem<ModId = DefId> {
pub parent_module: ModId,
pub name: Ident,
pub cfg: MetaItem,
}

impl<ModId> StrippedCfgItem<ModId> {
pub fn map_mod_id<New>(self, f: impl FnOnce(ModId) -> New) -> StrippedCfgItem<New> {
StrippedCfgItem { parent_module: f(self.parent_module), name: self.name, cfg: self.cfg }
}
}
4 changes: 3 additions & 1 deletion compiler/rustc_expand/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -947,6 +947,8 @@ pub trait ResolverExpand {
/// HIR proc macros items back to their harness items.
fn declare_proc_macro(&mut self, id: NodeId);

fn append_stripped_cfg_item(&mut self, parent_node: NodeId, name: Ident, cfg: ast::MetaItem);

/// Tools registered with `#![register_tool]` and used by tool attributes and lints.
fn registered_tools(&self) -> &RegisteredTools;
}
Expand All @@ -965,7 +967,7 @@ pub trait LintStoreExpand {

type LintStoreExpandDyn<'a> = Option<&'a (dyn LintStoreExpand + 'a)>;

#[derive(Clone, Default)]
#[derive(Debug, Clone, Default)]
pub struct ModuleData {
/// Path to the module starting from the crate name, like `my_crate::foo::bar`.
pub mod_path: Vec<Ident>,
Expand Down
20 changes: 14 additions & 6 deletions compiler/rustc_expand/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -416,20 +416,28 @@ impl<'a> StripUnconfigured<'a> {

/// Determines if a node with the given attributes should be included in this configuration.
fn in_cfg(&self, attrs: &[Attribute]) -> bool {
attrs.iter().all(|attr| !is_cfg(attr) || self.cfg_true(attr))
attrs.iter().all(|attr| !is_cfg(attr) || self.cfg_true(attr).0)
}

pub(crate) fn cfg_true(&self, attr: &Attribute) -> bool {
pub(crate) fn cfg_true(&self, attr: &Attribute) -> (bool, Option<MetaItem>) {
let meta_item = match validate_attr::parse_meta(&self.sess.parse_sess, attr) {
Ok(meta_item) => meta_item,
Err(mut err) => {
err.emit();
return true;
return (true, None);
}
};
parse_cfg(&meta_item, &self.sess).map_or(true, |meta_item| {
attr::cfg_matches(&meta_item, &self.sess.parse_sess, self.lint_node_id, self.features)
})
(
parse_cfg(&meta_item, &self.sess).map_or(true, |meta_item| {
attr::cfg_matches(
&meta_item,
&self.sess.parse_sess,
self.lint_node_id,
self.features,
)
}),
Some(meta_item),
)
}

/// If attributes are not allowed on expressions, emit an error for `attr`
Expand Down
49 changes: 44 additions & 5 deletions compiler/rustc_expand/src/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1042,6 +1042,12 @@ trait InvocationCollectorNode: HasAttrs + HasNodeId + Sized {
fn expand_cfg_false(&mut self, collector: &mut InvocationCollector<'_, '_>, span: Span) {
collector.cx.emit_err(RemoveNodeNotSupported { span, descr: Self::descr() });
}

/// All of the names (items) declared by this node.
/// This is an approximation and should only be used for diagnostics.
fn declared_names(&self) -> Vec<Ident> {
vec![]
}
}

impl InvocationCollectorNode for P<ast::Item> {
Expand Down Expand Up @@ -1148,6 +1154,27 @@ impl InvocationCollectorNode for P<ast::Item> {
collector.cx.current_expansion.module = orig_module;
res
}
fn declared_names(&self) -> Vec<Ident> {
if let ItemKind::Use(ut) = &self.kind {
fn collect_use_tree_leaves(ut: &ast::UseTree, idents: &mut Vec<Ident>) {
match &ut.kind {
ast::UseTreeKind::Glob => {}
ast::UseTreeKind::Simple(_) => idents.push(ut.ident()),
ast::UseTreeKind::Nested(nested) => {
for (ut, _) in nested {
collect_use_tree_leaves(&ut, idents);
}
}
}
}

let mut idents = Vec::new();
collect_use_tree_leaves(&ut, &mut idents);
return idents;
}

vec![self.ident]
}
}

struct TraitItemTag;
Expand Down Expand Up @@ -1685,16 +1712,17 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
node: &mut impl HasAttrs,
attr: ast::Attribute,
pos: usize,
) -> bool {
let res = self.cfg().cfg_true(&attr);
) -> (bool, Option<ast::MetaItem>) {
let (res, meta_item) = self.cfg().cfg_true(&attr);
if res {
// FIXME: `cfg(TRUE)` attributes do not currently remove themselves during expansion,
// and some tools like rustdoc and clippy rely on that. Find a way to remove them
// while keeping the tools working.
self.cx.expanded_inert_attrs.mark(&attr);
node.visit_attrs(|attrs| attrs.insert(pos, attr));
}
res

(res, meta_item)
}

fn expand_cfg_attr(&self, node: &mut impl HasAttrs, attr: &ast::Attribute, pos: usize) {
Expand All @@ -1715,9 +1743,20 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
return match self.take_first_attr(&mut node) {
Some((attr, pos, derives)) => match attr.name_or_empty() {
sym::cfg => {
if self.expand_cfg_true(&mut node, attr, pos) {
let (res, meta_item) = self.expand_cfg_true(&mut node, attr, pos);
if res {
continue;
}

if let Some(meta_item) = meta_item {
for name in node.declared_names() {
self.cx.resolver.append_stripped_cfg_item(
self.cx.current_expansion.lint_node_id,
name,
meta_item.clone(),
)
}
}
Default::default()
}
sym::cfg_attr => {
Expand Down Expand Up @@ -1761,7 +1800,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
Some((attr, pos, derives)) => match attr.name_or_empty() {
sym::cfg => {
let span = attr.span;
if self.expand_cfg_true(node, attr, pos) {
if self.expand_cfg_true(node, attr, pos).0 {
continue;
}

Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_metadata/src/rmeta/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -995,6 +995,15 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
)
}

fn get_stripped_cfg_items(self, cnum: CrateNum, tcx: TyCtxt<'tcx>) -> &'tcx [StrippedCfgItem] {
let item_names = self
.root
.stripped_cfg_items
.decode((self, tcx))
.map(|item| item.map_mod_id(|index| DefId { krate: cnum, index }));
tcx.arena.alloc_from_iter(item_names)
}

/// Iterates over the diagnostic items in the given crate.
fn get_diagnostic_items(self) -> DiagnosticItems {
let mut id_to_name = FxHashMap::default();
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,7 @@ provide! { tcx, def_id, other, cdata,
stability_implications => {
cdata.get_stability_implications(tcx).iter().copied().collect()
}
stripped_cfg_items => { cdata.get_stripped_cfg_items(cdata.cnum, tcx) }
is_intrinsic => { cdata.get_is_intrinsic(def_id.index) }
defined_lang_items => { cdata.get_lang_items(tcx) }
diagnostic_items => { cdata.get_diagnostic_items() }
Expand Down
13 changes: 13 additions & 0 deletions compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::rmeta::def_path_hash_map::DefPathHashMapRef;
use crate::rmeta::table::TableBuilder;
use crate::rmeta::*;

use rustc_ast::expand::StrippedCfgItem;
use rustc_ast::Attribute;
use rustc_data_structures::fingerprint::Fingerprint;
use rustc_data_structures::fx::{FxHashMap, FxIndexSet};
Expand Down Expand Up @@ -584,6 +585,8 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
(self.encode_lang_items(), self.encode_lang_items_missing())
});

let stripped_cfg_items = stat!("stripped-cfg-items", || self.encode_stripped_cfg_items());

let diagnostic_items = stat!("diagnostic-items", || self.encode_diagnostic_items());

let native_libraries = stat!("native-libs", || self.encode_native_libraries());
Expand Down Expand Up @@ -694,6 +697,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
lang_items,
diagnostic_items,
lang_items_missing,
stripped_cfg_items,
native_libraries,
foreign_modules,
source_map,
Expand Down Expand Up @@ -1939,6 +1943,15 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
self.lazy_array(&tcx.lang_items().missing)
}

fn encode_stripped_cfg_items(&mut self) -> LazyArray<StrippedCfgItem<DefIndex>> {
self.lazy_array(
self.tcx
.stripped_cfg_items(LOCAL_CRATE)
.into_iter()
.map(|item| item.clone().map_mod_id(|def_id| def_id.index)),
)
}

fn encode_traits(&mut self) -> LazyArray<DefIndex> {
empty_proc_macro!(self);
self.lazy_array(self.tcx.traits(LOCAL_CRATE).iter().map(|def_id| def_id.index))
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_metadata/src/rmeta/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use rustc_middle::middle::debugger_visualizer::DebuggerVisualizerFile;
use table::TableBuilder;

use rustc_ast as ast;
use rustc_ast::expand::StrippedCfgItem;
use rustc_attr as attr;
use rustc_data_structures::svh::Svh;
use rustc_hir as hir;
Expand Down Expand Up @@ -256,6 +257,7 @@ pub(crate) struct CrateRoot {
stability_implications: LazyArray<(Symbol, Symbol)>,
lang_items: LazyArray<(DefIndex, LangItem)>,
lang_items_missing: LazyArray<LangItem>,
stripped_cfg_items: LazyArray<StrippedCfgItem<DefIndex>>,
diagnostic_items: LazyArray<(Symbol, DefIndex)>,
native_libraries: LazyArray<NativeLib>,
foreign_modules: LazyArray<ForeignModule>,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/arena.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ macro_rules! arena_types {
[] predefined_opaques_in_body: rustc_middle::traits::solve::PredefinedOpaquesData<'tcx>,
[decode] doc_link_resolutions: rustc_hir::def::DocLinkResMap,
[] closure_kind_origin: (rustc_span::Span, rustc_middle::hir::place::Place<'tcx>),
[] stripped_cfg_items: rustc_ast::expand::StrippedCfgItem,
[] mod_child: rustc_middle::metadata::ModChild,
]);
)
Expand Down
11 changes: 10 additions & 1 deletion compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ use crate::ty::{
};
use rustc_arena::TypedArena;
use rustc_ast as ast;
use rustc_ast::expand::allocator::AllocatorKind;
use rustc_ast::expand::{allocator::AllocatorKind, StrippedCfgItem};
use rustc_attr as attr;
use rustc_data_structures::fingerprint::Fingerprint;
use rustc_data_structures::fx::{FxHashMap, FxIndexMap, FxIndexSet};
Expand Down Expand Up @@ -2174,6 +2174,15 @@ rustc_queries! {
query check_tys_might_be_eq(arg: Canonical<'tcx, (ty::ParamEnv<'tcx>, Ty<'tcx>, Ty<'tcx>)>) -> Result<(), NoSolution> {
desc { "check whether two const param are definitely not equal to eachother"}
}

/// Get all item paths that were stripped by a `#[cfg]` in a particular crate.
/// Should not be called for the local crate before the resolver outputs are created, as it
/// is only fed there.
query stripped_cfg_items(cnum: CrateNum) -> &'tcx [StrippedCfgItem] {
feedable
desc { "getting cfg-ed out item names" }
separate_provide_extern
}
}

rustc_query_append! { define_callbacks! }
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/ty/parameterized.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ trivially_parameterized_over_tcx! {
ty::fast_reject::SimplifiedType,
rustc_ast::Attribute,
rustc_ast::DelimArgs,
rustc_ast::expand::StrippedCfgItem<rustc_hir::def_id::DefIndex>,
rustc_attr::ConstStability,
rustc_attr::DefaultBodyStability,
rustc_attr::Deprecation,
Expand Down
Loading

0 comments on commit a97c36d

Please sign in to comment.