Remove doc_cfg show/hide conflict error and correctly handle values(any())

Author: GuillaumeGomez

compiler/rustc_attr_parsing/src/attributes/doc.rs

@@ -329,17 +329,17 @@ impl DocParser {
                     kind: LitKind::Str(symbol, _),
                     span,
                     ..
-                }) => match &mut cfg_values.values {
-                    DocCfgHideShowValue::Any(any_span) => {
+                }) => match &mut cfg_values {
+                    DocCfgHideShow::Any(any_span) => {
                         cx.emit_lint(
                             rustc_session::lint::builtin::INVALID_DOC_ATTRIBUTES,
                             DocAutoCfgHideShowValuesMix { value_span: *span },
                             *any_span,
                         );
                     }
-                    DocCfgHideShowValue::List(symbols) => {
+                    DocCfgHideShow::List(symbols) => {
                         if values_set.insert(symbol) {
-                            symbols.push((*symbol, *span));
+                            symbols.push(DocCfgHideShowValue::new(*symbol, *span));
                         }
                     }
                 },
@@ -358,19 +358,19 @@ impl DocParser {
                         && list.mixed().count() == 0
                     {
                         if ident.name == sym::any {
-                            if let DocCfgHideShowValue::List(values) = &cfg_values.values
-                                && let Some((_, span)) = values.first()
+                            if let DocCfgHideShow::List(values) = &cfg_values
+                                && let Some(value) = values.first()
                             {
                                 cx.emit_lint(
                                     rustc_session::lint::builtin::INVALID_DOC_ATTRIBUTES,
-                                    DocAutoCfgHideShowValuesMix { value_span: *span },
+                                    DocAutoCfgHideShowValuesMix { value_span: value.span },
                                     sub_item.span(),
                                 );
                             } else {
-                                cfg_values.values = DocCfgHideShowValue::Any(sub_item.span());
+                                cfg_values.merge_with(&DocCfgHideShow::Any(sub_item.span()));
                             }
                         } else {
-                            cfg_values.only_key = Some(sub_item.span());
+                            cfg_values.push_none(sub_item.span());
                         }
                     } else {
                         cx.emit_lint(

compiler/rustc_hir/src/attrs/data_structures.rs

@@ -515,98 +515,68 @@ pub enum HideOrShow {
     Show,
 }
 
-#[derive(Clone, Debug, StableHash, Encodable, Decodable, PrintAttribute, PartialEq)]
-pub enum DocCfgHideShowValue {
-    Any(Span),
-    List(ThinVec<(Symbol, Span)>),
+#[derive(Clone, Copy, Debug, StableHash, Encodable, Decodable, PrintAttribute, PartialEq)]
+pub struct DocCfgHideShowValue {
+    pub span: Span,
+    /// If `value` is `None`, then it's a `none()` value.
+    pub value: Option<Symbol>,
 }
 
-#[derive(Clone, Debug, StableHash, Encodable, Decodable, PrintAttribute)]
-pub struct DocCfgHideShow {
-    /// If `Some`, then `cfg` without values (like `cfg(windows)`) will be shown/hidden.
-    /// The `Span` comes from where this value was set.
-    pub only_key: Option<Span>,
-    /// The values of this `cfg` to shown/hidden.
-    pub values: DocCfgHideShowValue,
+impl DocCfgHideShowValue {
+    pub fn new(value: Symbol, span: Span) -> Self {
+        Self { span, value: Some(value) }
+    }
+
+    pub fn new_none(span: Span) -> Self {
+        Self { span, value: None }
+    }
+}
+
+#[derive(Clone, Debug, StableHash, Encodable, Decodable, PrintAttribute, PartialEq)]
+pub enum DocCfgHideShow {
+    Any(Span),
+    List(ThinVec<DocCfgHideShowValue>),
 }
 
 impl DocCfgHideShow {
     pub fn new() -> Self {
-        Self { only_key: None, values: DocCfgHideShowValue::List(ThinVec::new()) }
+        Self::List(ThinVec::new())
     }
 
     pub fn new_with_only_key(span: Span) -> Self {
-        Self { only_key: Some(span), values: DocCfgHideShowValue::List(ThinVec::new()) }
+        let mut values = ThinVec::with_capacity(1);
+        values.push(DocCfgHideShowValue { span, value: None });
+        Self::List(values)
     }
 
-    pub fn merge_with(&mut self, other: &Self) {
-        if self.only_key.is_none()
-            && let Some(span) = other.only_key
+    pub fn push_none(&mut self, span: Span) {
+        if let Self::List(values) = self
+            && !values.iter().any(|v| v.value.is_none())
         {
-            self.only_key = Some(span);
+            values.push(DocCfgHideShowValue { span, value: None });
         }
-        match (&mut self.values, &other.values) {
-            (DocCfgHideShowValue::Any(_), DocCfgHideShowValue::Any(_)) => {
+    }
+
+    pub fn merge_with(&mut self, other: &Self) {
+        match (self, other) {
+            (Self::Any(_), Self::Any(_) | Self::List(_)) => {
                 // Nothing to do.
             }
-            (_, DocCfgHideShowValue::Any(span)) => {
+            (s, Self::Any(span)) => {
                 // We "upgrade" the list values to "all".
-                self.values = DocCfgHideShowValue::Any(*span);
-            }
-            (DocCfgHideShowValue::List(values), DocCfgHideShowValue::List(other_values)) => {
-                // Having duplicates is not an issue, we simply ignore them. Would be more
-                // convenient to have a `set` type though. T_T
-                for (other_symbol, other_span) in other_values {
-                    if !values.iter().any(|(symbol, _)| symbol == other_symbol) {
-                        values.push((*other_symbol, *other_span));
-                    }
-                }
+                *s = Self::Any(*span);
             }
-            (DocCfgHideShowValue::Any(_), DocCfgHideShowValue::List(_)) => {
-                // Nothing to do here either, we already accept all values.
-            }
-        }
-    }
-
-    pub fn update_with(&mut self, other: &Self) {
-        if self.only_key.is_none()
-            && let Some(span) = other.only_key
-        {
-            self.only_key = Some(span);
-        }
-        match (&mut self.values, &other.values) {
-            (DocCfgHideShowValue::Any(_), _) => {}
-            (DocCfgHideShowValue::List(_), DocCfgHideShowValue::Any(span)) => {
-                self.values = DocCfgHideShowValue::Any(*span);
-            }
-            (DocCfgHideShowValue::List(values), DocCfgHideShowValue::List(other_values)) => {
+            (Self::List(values), Self::List(other_values)) => {
                 // Having duplicates is not an issue, we simply ignore them. Would be more
                 // convenient to have a `set` type though. T_T
-                for (other_symbol, other_span) in other_values {
-                    if !values.iter().any(|(symbol, _)| symbol == other_symbol) {
-                        values.push((*other_symbol, *other_span));
+                for other in other_values {
+                    if !values.iter().any(|value| value.value == other.value) {
+                        values.push(*other);
                     }
                 }
             }
         }
     }
-
-    pub fn remove_overlap(&mut self, other: &Self) {
-        if other.only_key.is_some() {
-            self.only_key = None;
-        }
-        match (&mut self.values, &other.values) {
-            (_, DocCfgHideShowValue::Any(_)) => {
-                self.values = DocCfgHideShowValue::List(ThinVec::new());
-            }
-            (DocCfgHideShowValue::Any(_), DocCfgHideShowValue::List(values)) => {
-                self.values = DocCfgHideShowValue::List(values.clone());
-            }
-            (DocCfgHideShowValue::List(current), DocCfgHideShowValue::List(values)) => {
-                current.retain(|(name, _)| !values.iter().any(|(other, _)| other == name));
-            }
-        }
-    }
 }
 
 #[derive(Clone, Debug, StableHash, Encodable, Decodable, PrintAttribute)]

src/doc/rustdoc/src/unstable-features.md

@@ -867,26 +867,40 @@ In this case, `#[cfg(feature = "linux")]`, `#[cfg(feature = "something")]`, `#[c
 ```rust,ignore (nightly)
 #[doc(auto_cfg(
     hide(feature, values("something")),
-    hide(target_os, values("linux"))))]
+    hide(target_os, values("linux")),
+))]
+```
+
+Now, only `#[cfg(feature = "something")]` and `#[cfg(target_os = "linux")]` will be hidden. If you want to hide a key and all its values, you can use `any()`:
+
+```rust,ignore (nightly)
+#[doc(auto_cfg(
+    hide(feature, values(any())),
+))]
 ```
 
-Now, only `#[cfg(feature = "something")]` and `#[cfg(target_os = "linux")]` will be hidden. If you want to hide all values of a key, you can use `any()`:
+If you want to hide only when there is no value you can use `none()`:
 
 ```rust,ignore (nightly)
-#[doc(auto_cfg(hide(feature, values(any()))))]
+#[doc(auto_cfg(
+    hide(feature, values("something", none())),
+))]
 ```
 
-If you want to hide when there is no value you can use `none()`:
+So now, if you want to forbid all values for a key, but allow the key itself, you can do:
 
 ```rust,ignore (nightly)
-#[doc(auto_cfg(hide(feature, values("something", none()))))]
+#[doc(auto_cfg(
+    hide(feature, values(any())), // We completely hide "feature".
+    show(feature), // We show again "feature" (but not any value).
+))]
 ```
 
 If the previous example, both `#[cfg(feature)]` and `#[cfg(feature = "something")]` will be hidden.
 
 Rustdoc currently hides `test`, `doc` and `doctest` attributes by default and reserves the right to change the list of "hidden by default" attributes.
 
-The attribute accepts only a list of identifiers or key/value items. So you can write:
+The attribute accepts only a list of identifiers and `values()`. So you can write:
 
 ```rust,ignore (nightly)
 #[doc(auto_cfg(
@@ -916,14 +930,6 @@ However, it only impacts the `unix` cfg, not the feature:
 #[cfg(feature = "unix")] // `feature = "unix"` is displayed
 ```
 
-If `cfg_auto(show(...))` and `cfg_auto(hide(...))` are used to show/hide a same `cfg` on a same item, it'll emit an error. Example:
-
-```rust,ignore (nightly)
-#[doc(auto_cfg(hide(unix)))]
-#[doc(auto_cfg(show(unix)))] // Error!
-pub fn foo() {}
-```
-
 Using this attribute will re-enable `auto_cfg` if it was disabled at this location:
 
 ```rust,ignore (nightly)
@@ -941,14 +947,6 @@ pub mod module {
 }
 ```
 
-However, using `doc(auto_cfg = ...)` and `doc(auto_cfg(...))` on the same item will emit an error:
-
-```rust,ignore (nightly)
-#[doc(auto_cfg = false)]
-#[doc(auto_cfg(hide(unix)))] // error
-pub fn foo() {}
-```
-
 The reason behind this is that `doc(auto_cfg = ...)` enables or disables the feature, whereas `doc(auto_cfg(...))` enables it unconditionally, making the first attribute to appear useless as it will be overidden by the next `doc(auto_cfg)` attribute.
 
 ### `#[doc(auto_cfg(show(...)))]`