Rollup merge of #100029 - hdelc:master, r=cjgillot

Prevent ICE for `doc_alias` on match arm, statement, expression Fixes #99777. This is a pretty minimal fix that should be safe, since rustdoc doesn't generate documentation for match arms, statements, or expressions. I mentioned in the linked issue that the `doc_alias` target checking should probably be improved to avoid future ICEs, but as a new contributor, I'm not confident enough with the HIR types to make a larger change.
rust-lang · Aug 3, 2022 · f8e6617 · f8e6617
2 parents 02fcec2 + 2be0094
commit f8e6617
Show file tree

Hide file tree

Showing 7 changed files with 109 additions and 60 deletions.
diff --git a/compiler/rustc_hir/src/target.rs b/compiler/rustc_hir/src/target.rs
@@ -60,51 +60,7 @@ pub enum Target {
 
 impl Display for Target {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(
-            f,
-            "{}",
-            match *self {
-                Target::ExternCrate => "extern crate",
-                Target::Use => "use",
-                Target::Static => "static item",
-                Target::Const => "constant item",
-                Target::Fn => "function",
-                Target::Closure => "closure",
-                Target::Mod => "module",
-                Target::ForeignMod => "foreign module",
-                Target::GlobalAsm => "global asm",
-                Target::TyAlias => "type alias",
-                Target::OpaqueTy => "opaque type",
-                Target::Enum => "enum",
-                Target::Variant => "enum variant",
-                Target::Struct => "struct",
-                Target::Field => "struct field",
-                Target::Union => "union",
-                Target::Trait => "trait",
-                Target::TraitAlias => "trait alias",
-                Target::Impl => "item",
-                Target::Expression => "expression",
-                Target::Statement => "statement",
-                Target::Arm => "match arm",
-                Target::AssocConst => "associated const",
-                Target::Method(kind) => match kind {
-                    MethodKind::Inherent => "inherent method",
-                    MethodKind::Trait { body: false } => "required trait method",
-                    MethodKind::Trait { body: true } => "provided trait method",
-                },
-                Target::AssocTy => "associated type",
-                Target::ForeignFn => "foreign function",
-                Target::ForeignStatic => "foreign static item",
-                Target::ForeignTy => "foreign type",
-                Target::GenericParam(kind) => match kind {
-                    GenericParamKind::Type => "type parameter",
-                    GenericParamKind::Lifetime => "lifetime parameter",
-                    GenericParamKind::Const => "const parameter",
-                },
-                Target::MacroDef => "macro def",
-                Target::Param => "function param",
-            }
-        )
+        write!(f, "{}", Self::name(*self))
     }
 }
 
@@ -185,4 +141,48 @@ impl Target {
             hir::GenericParamKind::Const { .. } => Target::GenericParam(GenericParamKind::Const),
         }
     }
+
+    pub fn name(self) -> &'static str {
+        match self {
+            Target::ExternCrate => "extern crate",
+            Target::Use => "use",
+            Target::Static => "static item",
+            Target::Const => "constant item",
+            Target::Fn => "function",
+            Target::Closure => "closure",
+            Target::Mod => "module",
+            Target::ForeignMod => "foreign module",
+            Target::GlobalAsm => "global asm",
+            Target::TyAlias => "type alias",
+            Target::OpaqueTy => "opaque type",
+            Target::Enum => "enum",
+            Target::Variant => "enum variant",
+            Target::Struct => "struct",
+            Target::Field => "struct field",
+            Target::Union => "union",
+            Target::Trait => "trait",
+            Target::TraitAlias => "trait alias",
+            Target::Impl => "implementation block",
+            Target::Expression => "expression",
+            Target::Statement => "statement",
+            Target::Arm => "match arm",
+            Target::AssocConst => "associated const",
+            Target::Method(kind) => match kind {
+                MethodKind::Inherent => "inherent method",
+                MethodKind::Trait { body: false } => "required trait method",
+                MethodKind::Trait { body: true } => "provided trait method",
+            },
+            Target::AssocTy => "associated type",
+            Target::ForeignFn => "foreign function",
+            Target::ForeignStatic => "foreign static item",
+            Target::ForeignTy => "foreign type",
+            Target::GenericParam(kind) => match kind {
+                GenericParamKind::Type => "type parameter",
+                GenericParamKind::Lifetime => "lifetime parameter",
+                GenericParamKind::Const => "const parameter",
+            },
+            Target::MacroDef => "macro def",
+            Target::Param => "function param",
+        }
+    }
 }
diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs
@@ -596,8 +596,6 @@ impl CheckAttrVisitor<'_> {
 
         let span = meta.span();
         if let Some(location) = match target {
-            Target::Impl => Some("implementation block"),
-            Target::ForeignMod => Some("extern block"),
             Target::AssocTy => {
                 let parent_hir_id = self.tcx.hir().get_parent_item(hir_id);
                 let containing_item = self.tcx.hir().expect_item(parent_hir_id);
@@ -619,7 +617,34 @@ impl CheckAttrVisitor<'_> {
             }
             // we check the validity of params elsewhere
             Target::Param => return false,
-            _ => None,
+            Target::Expression
+            | Target::Statement
+            | Target::Arm
+            | Target::ForeignMod
+            | Target::Closure
+            | Target::Impl => Some(target.name()),
+            Target::ExternCrate
+            | Target::Use
+            | Target::Static
+            | Target::Const
+            | Target::Fn
+            | Target::Mod
+            | Target::GlobalAsm
+            | Target::TyAlias
+            | Target::OpaqueTy
+            | Target::Enum
+            | Target::Variant
+            | Target::Struct
+            | Target::Field
+            | Target::Union
+            | Target::Trait
+            | Target::TraitAlias
+            | Target::Method(..)
+            | Target::ForeignFn
+            | Target::ForeignStatic
+            | Target::ForeignTy
+            | Target::GenericParam(..)
+            | Target::MacroDef => None,
         } {
             tcx.sess.emit_err(errors::DocAliasBadLocation { span, attr_str, location });
             return false;

diff --git a/src/test/rustdoc-ui/check-doc-alias-attr-location.stderr b/src/test/rustdoc-ui/check-doc-alias-attr-location.stderr
@@ -1,4 +1,4 @@
-error: `#[doc(alias = "...")]` isn't allowed on extern block
+error: `#[doc(alias = "...")]` isn't allowed on foreign module
   --> $DIR/check-doc-alias-attr-location.rs:7:7
    |
 LL | #[doc(alias = "foo")]

diff --git a/src/test/ui/feature-gates/issue-43106-gating-of-builtin-attrs.stderr b/src/test/ui/feature-gates/issue-43106-gating-of-builtin-attrs.stderr
@@ -212,7 +212,7 @@ note: the lint level is defined here
 LL | #![warn(unused_attributes, unknown_lints)]
    |         ^^^^^^^^^^^^^^^^^
 
-warning: `#[automatically_derived]` only has an effect on items
+warning: `#[automatically_derived]` only has an effect on implementation blocks
   --> $DIR/issue-43106-gating-of-builtin-attrs.rs:266:1
    |
 LL | #[automatically_derived]
@@ -515,25 +515,25 @@ warning: `#[path]` only has an effect on modules
 LL |     #[path = "3800"] impl S { }
    |     ^^^^^^^^^^^^^^^^
 
-warning: `#[automatically_derived]` only has an effect on items
+warning: `#[automatically_derived]` only has an effect on implementation blocks
   --> $DIR/issue-43106-gating-of-builtin-attrs.rs:269:17
    |
 LL |     mod inner { #![automatically_derived] }
    |                 ^^^^^^^^^^^^^^^^^^^^^^^^^
 
-warning: `#[automatically_derived]` only has an effect on items
+warning: `#[automatically_derived]` only has an effect on implementation blocks
   --> $DIR/issue-43106-gating-of-builtin-attrs.rs:272:5
    |
 LL |     #[automatically_derived] fn f() { }
    |     ^^^^^^^^^^^^^^^^^^^^^^^^
 
-warning: `#[automatically_derived]` only has an effect on items
+warning: `#[automatically_derived]` only has an effect on implementation blocks
   --> $DIR/issue-43106-gating-of-builtin-attrs.rs:275:5
    |
 LL |     #[automatically_derived] struct S;
    |     ^^^^^^^^^^^^^^^^^^^^^^^^
 
-warning: `#[automatically_derived]` only has an effect on items
+warning: `#[automatically_derived]` only has an effect on implementation blocks
   --> $DIR/issue-43106-gating-of-builtin-attrs.rs:278:5
    |
 LL |     #[automatically_derived] type T = S;
@@ -923,7 +923,7 @@ warning: `#[must_use]` has no effect when applied to a type alias
 LL |     #[must_use] type T = S;
    |     ^^^^^^^^^^^
 
-warning: `#[must_use]` has no effect when applied to an item
+warning: `#[must_use]` has no effect when applied to an implementation block
   --> $DIR/issue-43106-gating-of-builtin-attrs.rs:614:5
    |
 LL |     #[must_use] impl S { }

diff --git a/src/test/ui/lint/unused/unused_attributes-must_use.stderr b/src/test/ui/lint/unused/unused_attributes-must_use.stderr
@@ -45,7 +45,7 @@ error: `#[must_use]` has no effect when applied to a static item
 LL | #[must_use]
    | ^^^^^^^^^^^
 
-error: `#[must_use]` has no effect when applied to an item
+error: `#[must_use]` has no effect when applied to an implementation block
   --> $DIR/unused_attributes-must_use.rs:33:1
    |
 LL | #[must_use]
@@ -69,7 +69,7 @@ error: `#[must_use]` has no effect when applied to a type parameter
 LL | fn qux<#[must_use] T>(_: T) {}
    |        ^^^^^^^^^^^
 
-error: `#[must_use]` has no effect when applied to an item
+error: `#[must_use]` has no effect when applied to an implementation block
   --> $DIR/unused_attributes-must_use.rs:79:1
    |
 LL | #[must_use]

diff --git a/src/test/ui/rustdoc/check-doc-alias-attr-location.rs b/src/test/ui/rustdoc/check-doc-alias-attr-location.rs
@@ -21,6 +21,12 @@ impl Foo for Bar {
     type X = i32;
     fn foo(#[doc(alias = "qux")] _x: u32) -> Self::X {
         //~^ ERROR
-        0
+        #[doc(alias = "stmt")] //~ ERROR
+        let x = 0;
+        #[doc(alias = "expr")] //~ ERROR
+        match x {
+            #[doc(alias = "arm")] //~ ERROR
+            _ => 0
+        }
     }
 }
diff --git a/src/test/ui/rustdoc/check-doc-alias-attr-location.stderr b/src/test/ui/rustdoc/check-doc-alias-attr-location.stderr
@@ -4,7 +4,7 @@ error: allow, cfg, cfg_attr, deny, expect, forbid, and warn are the only allowed
 LL |     fn foo(#[doc(alias = "qux")] _x: u32) -> Self::X {
    |            ^^^^^^^^^^^^^^^^^^^^^
 
-error: `#[doc(alias = "...")]` isn't allowed on extern block
+error: `#[doc(alias = "...")]` isn't allowed on foreign module
   --> $DIR/check-doc-alias-attr-location.rs:9:7
    |
 LL | #[doc(alias = "foo")]
@@ -28,5 +28,23 @@ error: `#[doc(alias = "...")]` isn't allowed on type alias in implementation blo
 LL |     #[doc(alias = "assoc")]
    |           ^^^^^^^^^^^^^^^
 
-error: aborting due to 5 previous errors
+error: `#[doc(alias = "...")]` isn't allowed on statement
+  --> $DIR/check-doc-alias-attr-location.rs:24:15
+   |
+LL |         #[doc(alias = "stmt")]
+   |               ^^^^^^^^^^^^^^
+
+error: `#[doc(alias = "...")]` isn't allowed on expression
+  --> $DIR/check-doc-alias-attr-location.rs:26:15
+   |
+LL |         #[doc(alias = "expr")]
+   |               ^^^^^^^^^^^^^^
+
+error: `#[doc(alias = "...")]` isn't allowed on match arm
+  --> $DIR/check-doc-alias-attr-location.rs:28:19
+   |
+LL |             #[doc(alias = "arm")]
+   |                   ^^^^^^^^^^^^^
+
+error: aborting due to 8 previous errors