clean up TODOs

Author: jdonszelmann

compiler/rustc_attr_data_structures/src/attributes.rs

@@ -141,26 +141,16 @@ impl Deprecation {
 /// happen.
 ///
 /// For more docs, look in [`rustc_attr`](https://doc.rust-lang.org/stable/nightly-rustc/rustc_attr/index.html)
-// FIXME(jdonszelmann): rename to AttributeKind once hir::AttributeKind is dissolved
 #[derive(Clone, Debug, HashStable_Generic, Encodable, Decodable)]
 pub enum AttributeKind {
     // tidy-alphabetical-start
-    Allow,
     AllowConstFnUnstable(ThinVec<Symbol>),
-    AllowInternalUnsafe,
     AllowInternalUnstable(ThinVec<(Symbol, Span)>),
-    AutoDiff,
-    AutomaticallyDerived,
     BodyStability {
         stability: DefaultBodyStability,
         /// Span of the `#[rustc_default_body_unstable(...)]` attribute
         span: Span,
     },
-    Cfg,
-    CfgAttr,
-    CfiEncoding, // FIXME(cfi_encoding)
-    Cold,
-    CollapseDebuginfo,
     Confusables {
         symbols: ThinVec<Symbol>,
         // FIXME(jdonszelmann): remove when target validation code is moved
@@ -172,14 +162,6 @@ pub enum AttributeKind {
         span: Span,
     },
     ConstStabilityIndirect,
-    ConstTrait,
-    Coroutine,
-    Coverage,
-    CustomMir,
-    DebuggerVisualizer,
-    DefaultLibAllocator,
-    Deny,
-    DeprecatedSafe, // FIXME(deprecated_safe)
     Deprecation {
         deprecation: Deprecation,
         span: Span,
@@ -195,57 +177,12 @@ pub enum AttributeKind {
         span: Span,
         comment: Symbol,
     },
-    Expect,
-    ExportName,
-    FfiConst,
-    FfiPure,
-    Forbid,
-    Fundamental,
-    Ignore,
-    // TODO: must contain span for clippy
-    Inline,
-    InstructionSet, // broken on stable!!!
-    Lang,
-    Link,
-    Linkage,
-    LinkName,
-    LinkOrdinal,
-    LinkSection,
-    MacroExport,
     MacroTransparency(Transparency),
-    MacroUse,
-    Marker,
-    MayDangle,
-    MustNotSuspend,
-    MustUse,
-    NeedsAllocator,
-    NoImplicitPrelude,
-    NoLink,
-    NoMangle,
-    NonExhaustive,
-    NoSanitize,
-    OmitGdbPrettyPrinterSection, // FIXME(omit_gdb_pretty_printer_section)
-    PanicHandler,
-    PatchableFunctionEntry, // FIXME(patchable_function_entry)
-    Path,
-    Pointee, // FIXME(derive_smart_pointer)
-    PreludeImport,
-    ProcMacro,
-    ProcMacroAttribute,
-    ProcMacroDerive,
     Repr(ThinVec<(ReprAttr, Span)>),
     Stability {
         stability: Stability,
         /// Span of the `#[stable(...)]` or `#[unstable(...)]` attribute
         span: Span,
     },
-    Start,
-    TargetFeature,
-    ThreadLocal,
-    TrackCaller,
-    Unstable,
-    Used,
-    Warn,
-    WindowsSubsystem, // broken on stable!!!
-                      // tidy-alphabetical-end
+    // tidy-alphabetical-end
 }

compiler/rustc_attr_parsing/src/attributes/cfg.rs

@@ -1,4 +1,3 @@
-// TODO: convert cfg properly.... And learn how cfg works I guess
 use rustc_ast::{LitKind, MetaItem, MetaItemInner, MetaItemKind, MetaItemLit, NodeId};
 use rustc_ast_pretty::pprust;
 use rustc_attr_data_structures::RustcVersion;

compiler/rustc_attr_parsing/src/attributes/confusables.rs

@@ -14,7 +14,7 @@ pub(crate) struct ConfusablesParser {
 impl AttributeParser for ConfusablesParser {
     const ATTRIBUTES: AcceptMapping<Self> = &[(&[sym::rustc_confusables], |this, cx, args| {
         let Some(list) = args.list() else {
-            // TODO: error when not a list? Bring validation code here.
+            // FIXME(jdonszelmann): error when not a list? Bring validation code here.
             //       NOTE: currently subsequent attributes are silently ignored using
             //       tcx.get_attr().
             return;

compiler/rustc_attr_parsing/src/attributes/repr.rs

@@ -70,7 +70,6 @@ macro_rules! int_pat {
     };
 }
 
-// TODO: inline
 fn int_type_of_word(s: Symbol) -> Option<IntType> {
     use IntType::*;
 

compiler/rustc_attr_parsing/src/attributes/transparency.rs

@@ -7,7 +7,7 @@ use crate::parser::ArgParser;
 
 pub(crate) struct TransparencyParser;
 
-// TODO: fix this but I don't want to rn
+// FIXME(jdonszelmann): make these proper diagnostics
 #[allow(rustc::untranslatable_diagnostic)]
 #[allow(rustc::diagnostic_outside_of_impl)]
 impl SingleAttributeParser for TransparencyParser {

compiler/rustc_attr_parsing/src/attributes/util.rs

@@ -19,7 +19,6 @@ pub fn parse_version(s: Symbol) -> Option<RustcVersion> {
     Some(RustcVersion { major, minor, patch })
 }
 
-// TODO: create single word attribute map
 pub fn is_builtin_attr(attr: &impl AttributeExt) -> bool {
     attr.is_doc_comment() || attr.ident().is_some_and(|ident| is_builtin_attr_name(ident.name))
 }

compiler/rustc_attr_parsing/src/lib.rs

@@ -62,7 +62,7 @@ rustc_fluent_macro::fluent_messages! { "../messages.ftl" }
 ///
 /// A little like `matches` but for attributes.
 ///
-/// ```rust,no_run
+/// ```rust,ignore (illustrative)
 /// // finds the repr attribute
 /// if let Some(r) = find_attr!(attrs, AttributeKind::Repr(r) => r) {
 ///

compiler/rustc_attr_parsing/src/parser.rs

@@ -1,4 +1,3 @@
-use core::panic;
 use std::fmt::{Debug, Display};
 use std::iter::Peekable;
 
@@ -231,7 +230,7 @@ impl<'a> Debug for MetaItemParser<'a> {
 
 impl<'a> MetaItemParser<'a> {
     /// Create a new parser from a [`NormalAttr`], which is stored inside of any
-    /// [`ast::Attribute`](Attribute)
+    /// [`ast::Attribute`](rustc_ast::Attribute)
     pub fn from_attr(attr: &'a NormalAttr, dcx: DiagCtxtHandle<'a>) -> Self {
         Self {
             path: PathParser::Ast(&attr.item.path),
@@ -268,8 +267,6 @@ impl<'a> MetaItemParser<'a> {
     /// - `#[rustfmt::skip]`: `rustfmt::skip` is a path
     /// - `#[allow(clippy::complexity)]`: `clippy::complexity` is a path
     /// - `#[inline]`: `inline` is a single segment path
-    /// - `#[inline(always)]`: `always` is a single segment path, but `inline` is *not and
-    ///    should be parsed using [`list`](Self::list)
     pub fn path(&self) -> (PathParser<'a>, &ArgParser<'a>) {
         self.deconstruct()
     }
@@ -291,11 +288,8 @@ impl<'a> MetaItemParser<'a> {
     ///
     /// Some examples:
     /// - `#[inline]`: `inline` is a word
-    /// - `#[rustfmt::skip]`: `rustfmt::skip` is a path, and not a word
-    /// - `#[inline(always)]`: `always` is a word, but `inline` is *not and should be parsed
-    ///   using [`path_list`](Self::path_list)
-    /// - `#[allow(clippy::complexity)]`: `clippy::complexity` is *not* a word, and should instead be parsed
-    ///   using [`path`](Self::path)
+    /// - `#[rustfmt::skip]`: `rustfmt::skip` is a path,
+    ///   and not a word and should instead be parsed using [`path`](Self::path)
     pub fn word(&self) -> Option<(Ident, &ArgParser<'a>)> {
         let (path, args) = self.deconstruct();
         Some((path.word()?, args))
@@ -406,7 +400,7 @@ impl<'a> MetaItemListParserContext<'a> {
                     {
                         segments.push(Ident::new(name, span));
                     } else {
-                        unreachable!()
+                        return None;
                     }
                     if let Some(TokenTree::Token(Token { kind: token::PathSep, .. }, _)) =
                         self.inside_delimiters.peek()
@@ -422,16 +416,14 @@ impl<'a> MetaItemListParserContext<'a> {
             Some(TokenTree::Token(
                 Token { kind: token::OpenDelim(_) | token::CloseDelim(_), .. },
                 _,
-            )) => {
-                panic!("Should be `AttrTokenTree::Delimited`, not delim tokens: {:?}", tt);
-            }
-            Some(x) => {
+            )) => None,
+            Some(_) => {
                 // malformed attributes can get here. We can't crash, but somewhere else should've
                 // already warned for this.
-                self.dcx.span_delayed_bug(
-                    x.span(),
-                    format!("unexpected token {x:?} in built-in attribute path"),
-                );
+                // self.dcx.span_delayed_bug(
+                //     x.span(),
+                //     format!("unexpected token {x:?} in built-in attribute path"),
+                // );
                 None
             }
             None => None,
@@ -450,7 +442,7 @@ impl<'a> MetaItemListParserContext<'a> {
             Some(TokenTree::Token(ref token, _)) => {
                 if let Some(s) = MetaItemLit::from_token(token) { Some(s) } else { None }
             }
-            x => unreachable!("{x:?}"),
+            x => None,
         }
     }
 
@@ -532,9 +524,9 @@ impl<'a> MetaItemListParserContext<'a> {
                     )),
                 }
             }
-            Some(TokenTree::Delimited(span, ..)) => {
+            Some(TokenTree::Delimited(_, ..)) => {
                 self.inside_delimiters.next();
-                self.dcx.span_delayed_bug(span.entire(), "wrong delimiters");
+                // self.dcx.span_delayed_bug(span.entire(), "wrong delimiters");
                 return None;
             }
             Some(TokenTree::Token(Token { kind: token::Eq, span }, _)) => {

compiler/rustc_hir/src/hir.rs

@@ -1107,6 +1107,7 @@ impl AttributeExt for Attribute {
             Attribute::Unparsed(u) => u.span,
             // FIXME: should not be needed anymore when all attrs are parsed
             Attribute::Parsed(AttributeKind::Deprecation { span, .. }) => *span,
+            Attribute::Parsed(AttributeKind::DocComment { span, .. }) => *span,
             a => panic!("can't get the span of an arbitrary parsed attribute: {a:?}"),
         }
     }
@@ -1149,6 +1150,7 @@ impl AttributeExt for Attribute {
     fn style(&self) -> AttrStyle {
         match &self {
             Attribute::Unparsed(u) => u.style,
+            Attribute::Parsed(AttributeKind::DocComment { style, .. }) => *style,
             _ => panic!(),
         }
     }

compiler/rustc_lint/src/expect.rs

@@ -40,10 +40,6 @@ fn check_expectations(tcx: TyCtxt<'_>, tool_filter: Option<Symbol>) {
                 // We are an `eval_always` query, so looking at the attribute's `AttrId` is ok.
                 let attr_id = tcx.hir().attrs(hir_id)[attr_index as usize].id();
 
-                // TODO: we're only dealling with allow/deny/etc lint attributes. They can have an
-                // ID, while other attributes don't necessarily. however, how do we assert that
-                // nicely?
-                // let attr_id = todo!();
                 (attr_id, lint_index)
             }
             _ => panic!("fulfilled expectations must have a lint index"),