From 6eb4735acc50f58e501b42c5d75ae73b3d74b44d Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 22 Apr 2021 13:28:43 -0400 Subject: Unify rustc and rustdoc parsing of `cfg()` This extracts a new `parse_cfg` function that's used between both. - Treat `#[doc(cfg(x), cfg(y))]` the same as `#[doc(cfg(x)] #[doc(cfg(y))]`. Previously it would be completely ignored. - Treat `#[doc(inline, cfg(x))]` the same as `#[doc(inline)] #[doc(cfg(x))]`. Previously, the cfg would be ignored. - Pass the cfg predicate through to rustc_expand to be validated Co-authored-by: Vadim Petrochenkov --- src/librustdoc/clean/inline.rs | 4 +-- src/librustdoc/clean/mod.rs | 2 +- src/librustdoc/clean/types.rs | 54 +++++++++++++------------------- src/librustdoc/doctest.rs | 2 +- src/librustdoc/html/render/context.rs | 2 +- src/librustdoc/html/render/print_item.rs | 2 +- src/test/rustdoc-ui/invalid-cfg.rs | 4 +++ src/test/rustdoc-ui/invalid-cfg.stderr | 14 +++++++++ src/test/rustdoc/doc-cfg.rs | 8 +++++ 9 files changed, 53 insertions(+), 39 deletions(-) create mode 100644 src/test/rustdoc-ui/invalid-cfg.rs create mode 100644 src/test/rustdoc-ui/invalid-cfg.stderr (limited to 'src') diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index 3e89c1ac4c5..4c3b86b2e2b 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -307,10 +307,10 @@ fn merge_attrs( } else { Attributes::from_ast(&both, None) }, - both.cfg(cx.sess().diagnostic()), + both.cfg(cx.sess()), ) } else { - (old_attrs.clean(cx), old_attrs.cfg(cx.sess().diagnostic())) + (old_attrs.clean(cx), old_attrs.cfg(cx.sess())) } } diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 6563f398edb..a7bc3c20a3d 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -2006,7 +2006,7 @@ fn clean_extern_crate( def_id: crate_def_id, visibility: krate.vis.clean(cx), kind: box ExternCrateItem { src: orig_name }, - cfg: attrs.cfg(cx.sess().diagnostic()), + cfg: attrs.cfg(cx.sess()), }] } diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 5e47144588b..a2a03dfd15b 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -320,7 +320,7 @@ impl Item { kind, box ast_attrs.clean(cx), cx, - ast_attrs.cfg(cx.sess().diagnostic()), + ast_attrs.cfg(cx.sess()), ) } @@ -332,7 +332,7 @@ impl Item { cx: &mut DocContext<'_>, cfg: Option>, ) -> Item { - debug!("name={:?}, def_id={:?}", name, def_id); + trace!("name={:?}, def_id={:?}", name, def_id); Item { def_id, @@ -681,7 +681,7 @@ crate trait AttributesExt { fn other_attrs(&self) -> Vec; - fn cfg(&self, diagnostic: &::rustc_errors::Handler) -> Option>; + fn cfg(&self, sess: &Session) -> Option>; } impl AttributesExt for [ast::Attribute] { @@ -706,17 +706,28 @@ impl AttributesExt for [ast::Attribute] { self.iter().filter(|attr| attr.doc_str().is_none()).cloned().collect() } - fn cfg(&self, diagnostic: &::rustc_errors::Handler) -> Option> { + fn cfg(&self, sess: &Session) -> Option> { let mut cfg = Cfg::True; for attr in self.iter() { + // #[doc] if attr.doc_str().is_none() && attr.has_name(sym::doc) { - if let Some(mi) = attr.meta() { - if let Some(cfg_mi) = Attributes::extract_cfg(&mi) { - // Extracted #[doc(cfg(...))] - match Cfg::parse(cfg_mi) { - Ok(new_cfg) => cfg &= new_cfg, - Err(e) => diagnostic.span_err(e.span, e.msg), + // #[doc(...)] + if let Some(list) = attr.meta().as_ref().and_then(|mi| mi.meta_item_list()) { + for item in list { + // #[doc(include)] + if !item.has_name(sym::cfg) { + continue; + } + // #[doc(cfg(...))] + if let Some(cfg_mi) = item + .meta_item() + .and_then(|item| rustc_expand::config::parse_cfg(&item, sess)) + { + match Cfg::parse(&cfg_mi) { + Ok(new_cfg) => cfg &= new_cfg, + Err(e) => sess.span_err(e.span, e.msg), + } } } } @@ -883,29 +894,6 @@ impl Attributes { self.other_attrs.lists(name) } - /// Extracts the content from an attribute `#[doc(cfg(content))]`. - crate fn extract_cfg(mi: &ast::MetaItem) -> Option<&ast::MetaItem> { - use rustc_ast::NestedMetaItem::MetaItem; - - if let ast::MetaItemKind::List(ref nmis) = mi.kind { - if nmis.len() == 1 { - if let MetaItem(ref cfg_mi) = nmis[0] { - if cfg_mi.has_name(sym::cfg) { - if let ast::MetaItemKind::List(ref cfg_nmis) = cfg_mi.kind { - if cfg_nmis.len() == 1 { - if let MetaItem(ref content_mi) = cfg_nmis[0] { - return Some(content_mi); - } - } - } - } - } - } - } - - None - } - /// Reads a `MetaItem` from within an attribute, looks for whether it is a /// `#[doc(include="file")]`, and returns the filename and contents of the file as loaded from /// its expansion. diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index 466d1b65406..69a47c5b67a 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -1095,7 +1095,7 @@ impl<'a, 'hir, 'tcx> HirCollector<'a, 'hir, 'tcx> { let ast_attrs = self.tcx.hir().attrs(hir_id); let mut attrs = Attributes::from_ast(ast_attrs, None); - if let Some(ref cfg) = ast_attrs.cfg(self.sess.diagnostic()) { + if let Some(ref cfg) = ast_attrs.cfg(self.sess) { if !cfg.matches(&self.sess.parse_sess, Some(&self.sess.features_untracked())) { return; } diff --git a/src/librustdoc/html/render/context.rs b/src/librustdoc/html/render/context.rs index 4c8ba0e7b49..268974169ea 100644 --- a/src/librustdoc/html/render/context.rs +++ b/src/librustdoc/html/render/context.rs @@ -154,7 +154,7 @@ impl<'tcx> Context<'tcx> { &self.cache } - fn sess(&self) -> &'tcx Session { + pub(super) fn sess(&self) -> &'tcx Session { &self.shared.tcx.sess } diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index 1bb1db00e88..d33a31ef1ee 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -292,7 +292,7 @@ fn item_module(w: &mut Buffer, cx: &Context<'_>, item: &clean::Item, items: &[cl let import_item = clean::Item { def_id: import_def_id, attrs: import_attrs, - cfg: ast_attrs.cfg(cx.tcx().sess.diagnostic()), + cfg: ast_attrs.cfg(cx.sess()), ..myitem.clone() }; diff --git a/src/test/rustdoc-ui/invalid-cfg.rs b/src/test/rustdoc-ui/invalid-cfg.rs new file mode 100644 index 00000000000..d237b8605c0 --- /dev/null +++ b/src/test/rustdoc-ui/invalid-cfg.rs @@ -0,0 +1,4 @@ +#![feature(doc_cfg)] +#[doc(cfg = "x")] //~ ERROR not followed by parentheses +#[doc(cfg(x, y))] //~ ERROR multiple `cfg` predicates +struct S {} diff --git a/src/test/rustdoc-ui/invalid-cfg.stderr b/src/test/rustdoc-ui/invalid-cfg.stderr new file mode 100644 index 00000000000..dae238b052b --- /dev/null +++ b/src/test/rustdoc-ui/invalid-cfg.stderr @@ -0,0 +1,14 @@ +error: `cfg` is not followed by parentheses + --> $DIR/invalid-cfg.rs:2:7 + | +LL | #[doc(cfg = "x")] + | ^^^^^^^^^ help: expected syntax is: `cfg(/* predicate */)` + +error: multiple `cfg` predicates are specified + --> $DIR/invalid-cfg.rs:3:14 + | +LL | #[doc(cfg(x, y))] + | ^ + +error: aborting due to 2 previous errors + diff --git a/src/test/rustdoc/doc-cfg.rs b/src/test/rustdoc/doc-cfg.rs index 89a61a289fd..1fc80b3e76c 100644 --- a/src/test/rustdoc/doc-cfg.rs +++ b/src/test/rustdoc/doc-cfg.rs @@ -91,3 +91,11 @@ pub unsafe fn uses_target_feature() { pub fn uses_cfg_target_feature() { uses_target_feature(); } + +// multiple attributes should be allowed +// @has doc_cfg/fn.multiple_attrs.html \ +// '//*[@id="main"]/*[@class="item-info"]/*[@class="stab portability"]' \ +// 'This is supported on x and y and z only.' +#[doc(inline, cfg(x))] +#[doc(cfg(y), cfg(z))] +pub fn multiple_attrs() {} -- cgit 1.4.1-3-g733a5 From 97658e58f0f5af5fec694c5c5df143685ee421b0 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 6 May 2021 08:47:55 -0700 Subject: rustc: Support Rust-specific features in -Ctarget-feature Since the beginning of time the `-Ctarget-feature` flag on the command line has largely been passed unmodified to LLVM. Afterwards, though, the `#[target_feature]` attribute was stabilized and some of the names in this attribute do not match the corresponding LLVM name. This is because Rust doesn't always want to stabilize the exact feature name in LLVM for the equivalent functionality in Rust. This creates a situation, however, where in Rust you'd write: #[target_feature(enable = "pclmulqdq")] unsafe fn foo() { // ... } but on the command line you would write: RUSTFLAGS="-Ctarget-feature=+pclmul" cargo build --release This difference is somewhat odd to deal with if you're a newcomer and the situation may be made worse with upcoming features like [WebAssembly SIMD](https://github.com/rust-lang/rust/issues/74372) which may be more prevalent. This commit implements a mapping to translate requests via `-Ctarget-feature` through the same name-mapping functionality that's present for attributes in Rust going to LLVM. This means that `+pclmulqdq` will work on x86 targets where as previously it did not. I've attempted to keep this backwards-compatible where the compiler will just opportunistically attempt to remap features found in `-Ctarget-feature`, but if there's something it doesn't understand it gets passed unmodified to LLVM just as it was before. --- compiler/rustc_codegen_llvm/src/llvm_util.rs | 38 +++++++++++++--------- .../rust-specific-name-no-warnings.rs | 5 +++ 2 files changed, 28 insertions(+), 15 deletions(-) create mode 100644 src/test/ui/target-feature/rust-specific-name-no-warnings.rs (limited to 'src') diff --git a/compiler/rustc_codegen_llvm/src/llvm_util.rs b/compiler/rustc_codegen_llvm/src/llvm_util.rs index b44553e4f6d..6101b90aea6 100644 --- a/compiler/rustc_codegen_llvm/src/llvm_util.rs +++ b/compiler/rustc_codegen_llvm/src/llvm_util.rs @@ -339,24 +339,32 @@ pub fn llvm_global_features(sess: &Session) -> Vec { Some(_) | None => {} }; + let filter = |s: &str| { + if s.is_empty() { + return None; + } + let feature = if s.starts_with("+") || s.starts_with("-") { + &s[1..] + } else { + return Some(s.to_string()); + }; + // Rustc-specific feature requests like `+crt-static` or `-crt-static` + // are not passed down to LLVM. + if RUSTC_SPECIFIC_FEATURES.contains(&feature) { + return None; + } + // ... otherwise though we run through `to_llvm_feature` feature when + // passing requests down to LLVM. This means that all in-language + // features also work on the command line instead of having two + // different names when the LLVM name and the Rust name differ. + Some(format!("{}{}", &s[..1], to_llvm_feature(sess, feature))) + }; + // Features implied by an implicit or explicit `--target`. - features.extend( - sess.target - .features - .split(',') - .filter(|f| !f.is_empty() && !RUSTC_SPECIFIC_FEATURES.iter().any(|s| f.contains(s))) - .map(String::from), - ); + features.extend(sess.target.features.split(',').filter_map(&filter)); // -Ctarget-features - features.extend( - sess.opts - .cg - .target_feature - .split(',') - .filter(|f| !f.is_empty() && !RUSTC_SPECIFIC_FEATURES.iter().any(|s| f.contains(s))) - .map(String::from), - ); + features.extend(sess.opts.cg.target_feature.split(',').filter_map(&filter)); features } diff --git a/src/test/ui/target-feature/rust-specific-name-no-warnings.rs b/src/test/ui/target-feature/rust-specific-name-no-warnings.rs new file mode 100644 index 00000000000..1708a71a981 --- /dev/null +++ b/src/test/ui/target-feature/rust-specific-name-no-warnings.rs @@ -0,0 +1,5 @@ +// build-pass +// only-x86 +// compile-flags: -C target-feature=+pclmulqdq + +fn main() {} -- cgit 1.4.1-3-g733a5