Merge #83

83: Minor cleanup around the project r=Niki4tap a=xFrednet

This is a collection of smaller things which have been at the back of my mind. Each commit addresses on TODO. For the ident commit, it's noteworthy that I only changed usages in the items module.

It's probably best to review this on a commit by commit basis.

Co-authored-by: xFrednet <[email protected]>

Author: bors[bot]

Cargo.lock

@@ -54,7 +54,7 @@ impl<'ast> AstPathSegment<'ast> {
 }
 
 impl<'ast> AstPathSegment<'ast> {
-    pub fn ident(&self) -> String {
+    pub fn ident(&self) -> &str {
         with_cx(self, |cx| cx.symbol_str(self.ident))
     }
 

marker_api/src/ast/mod.rs renamed to marker_api/src/ast.rs

@@ -75,7 +75,7 @@ impl<'ast> Parameter<'ast> {
 impl<'ast> Parameter<'ast> {
     // Function items actually use patterns and not names. Patterns are not yet
     // implemented though. A name should be good enough for now.
-    pub fn name(&self) -> Option<String> {
+    pub fn name(&self) -> Option<&str> {
         self.name.get().map(|sym| with_cx(self, |cx| cx.symbol_str(*sym)))
     }
 

marker_api/src/ast/common/ast_path.rs

@@ -1,12 +1,12 @@
-use std::path::PathBuf;
+use std::{marker::PhantomData, path::PathBuf};
 
 use crate::context::with_cx;
 
-use super::{Applicability, AstPath, ItemId, SpanId};
+use super::{Applicability, AstPath, ItemId, SpanId, SymbolId};
 
 #[repr(C)]
 #[doc(hidden)]
-#[derive(Copy, Clone, PartialEq, Eq, Hash)]
+#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
 #[cfg_attr(feature = "driver-api", visibility::make(pub))]
 #[allow(clippy::exhaustive_enums)]
 enum SpanSource<'ast> {
@@ -15,7 +15,7 @@ enum SpanSource<'ast> {
 }
 
 #[repr(C)]
-#[derive(Clone)]
+#[derive(Debug, Clone)]
 pub struct Span<'ast> {
     source: SpanSource<'ast>,
     /// The start marks the first byte in the [`SpanSource`] that is included in this
@@ -147,3 +147,69 @@ impl From<SpanId> for SpanOwner {
         Self::SpecificSpan(id)
     }
 }
+
+pub struct Ident<'ast> {
+    _lifetime: PhantomData<&'ast ()>,
+    sym: SymbolId,
+    span: SpanId,
+}
+
+impl<'ast> Ident<'ast> {
+    pub fn name(&self) -> &str {
+        with_cx(self, |cx| cx.symbol_str(self.sym))
+    }
+
+    pub fn span(&self) -> &Span<'ast> {
+        with_cx(self, |cx| cx.get_span(self.span))
+    }
+}
+
+#[cfg(feature = "driver-api")]
+impl<'ast> Ident<'ast> {
+    pub fn new(sym: SymbolId, span: SpanId) -> Self {
+        Self {
+            _lifetime: PhantomData,
+            sym,
+            span,
+        }
+    }
+}
+
+impl<'ast> std::fmt::Debug for Ident<'ast> {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("Ident")
+            .field("name", &self.name())
+            .field("span", &self.span())
+            .finish()
+    }
+}
+
+macro_rules! impl_ident_eq_for {
+    ($ty:ty) => {
+        impl<'ast> PartialEq<$ty> for Ident<'ast> {
+            fn eq(&self, other: &$ty) -> bool {
+                self.name().eq(other)
+            }
+        }
+        impl<'ast> PartialEq<Ident<'ast>> for $ty {
+            fn eq(&self, other: &Ident<'ast>) -> bool {
+                other.name().eq(self)
+            }
+        }
+    };
+    ($($ty:ty),+) => {
+        $(
+            impl_ident_eq_for!($ty);
+        )+
+    };
+}
+
+use impl_ident_eq_for;
+
+impl_ident_eq_for!(
+    str,
+    String,
+    std::ffi::OsStr,
+    std::ffi::OsString,
+    std::borrow::Cow<'_, str>
+);

marker_api/src/ast/common/callable.rs

@@ -21,30 +21,18 @@ impl<'ast> StrLitExpr<'ast> {
     /// This returns the UTF-8 string value of the string, if possible. Normal
     /// and raw strings in Rust are required to be UTF-8. Byte strings will be
     /// converted to UTF-8 if possible, otherwise `None` will be returned
-    pub fn str_value(&self) -> Option<String> {
+    pub fn str_value(&self) -> Option<&str> {
         match &self.str_data {
             StrKindWithData::Str(sym) | StrKindWithData::Raw(sym) => Some(with_cx(self, |cx| cx.symbol_str(*sym))),
-            StrKindWithData::Byte(bytes) => std::str::from_utf8(bytes.get()).map(ToString::to_string).ok(),
+            StrKindWithData::Byte(bytes) => std::str::from_utf8(bytes.get()).ok(),
         }
     }
 
     /// Returns the value of the string as bytes.
-    pub fn byte_value(&self) -> Box<[u8]> {
-        // The context currently returns symbols as a `String` which makes it
-        // difficult to return the bytes as a byte slice. This could be improved
-        // if the `symbol_str` returns `&'ast str`. But that's a larger todo for
-        // another time :)
-        fn box_slice(bytes: &[u8]) -> Box<[u8]> {
-            let mut vec = Vec::with_capacity(bytes.len());
-            vec.extend_from_slice(bytes);
-            vec.into_boxed_slice()
-        }
+    pub fn byte_value(&self) -> &[u8] {
         match &self.str_data {
-            StrKindWithData::Str(sym) | StrKindWithData::Raw(sym) => {
-                let str_value = with_cx(self, |cx| cx.symbol_str(*sym));
-                box_slice(str_value.as_bytes())
-            },
-            StrKindWithData::Byte(bytes) => box_slice(bytes.get()),
+            StrKindWithData::Str(sym) | StrKindWithData::Raw(sym) => with_cx(self, |cx| cx.symbol_str(*sym)).as_bytes(),
+            StrKindWithData::Byte(bytes) => bytes.get(),
         }
     }
 }

marker_api/src/ast/common/span.rs

@@ -67,7 +67,7 @@ impl<'ast> Lifetime<'ast> {
     }
 
     /// Note that the `'static` lifetime is not a label and will therefore return [`None`]
-    pub fn label(&self) -> Option<String> {
+    pub fn label(&self) -> Option<&str> {
         match self.kind {
             LifetimeKind::Label(sym, _) => Some(with_cx(self, |cx| cx.symbol_str(sym))),
             _ => None,
@@ -121,7 +121,7 @@ impl<'ast> BindingGenericArg<'ast> {
     /// ```
     ///
     /// Would return `Item` as the identifier.
-    pub fn ident(&self) -> String {
+    pub fn ident(&self) -> &str {
         with_cx(self, |cx| cx.symbol_str(self.ident))
     }
 

marker_api/src/ast/expr/str_lit_expr.rs

@@ -90,7 +90,7 @@ impl<'ast> TyParam<'ast> {
         self.id
     }
 
-    pub fn name(&self) -> String {
+    pub fn name(&self) -> &str {
         with_cx(self, |cx| cx.symbol_str(self.name))
     }
 }
@@ -140,7 +140,7 @@ impl<'ast> LifetimeParam<'ast> {
         self.id
     }
 
-    pub fn name(&self) -> String {
+    pub fn name(&self) -> &str {
         with_cx(self, |cx| cx.symbol_str(self.name))
     }
 }

marker_api/src/ast/generic/arg.rs

@@ -1,7 +1,7 @@
 use std::{fmt::Debug, marker::PhantomData};
 
 use super::expr::ExprKind;
-use super::{ItemId, Span, SymbolId};
+use super::{Ident, ItemId, Span};
 
 // Item implementations
 mod extern_crate_item;
@@ -10,8 +10,8 @@ mod mod_item;
 pub use mod_item::ModItem;
 mod static_item;
 pub use self::static_item::StaticItem;
-mod use_decl_item;
-pub use self::use_decl_item::*;
+mod use_item;
+pub use self::use_item::*;
 mod const_item;
 pub use self::const_item::ConstItem;
 mod fn_item;
@@ -42,7 +42,7 @@ pub trait ItemData<'ast>: Debug {
     fn visibility(&self) -> &Visibility<'ast>;
 
     /// This function can return [`None`] if the item was generated and has no real name
-    fn name(&self) -> Option<String>;
+    fn ident(&self) -> Option<&Ident<'ast>>;
 
     /// This returns this [`ItemData`] instance as a [`ItemKind`]. This can be useful for
     /// functions that take [`ItemKind`] as a parameter. For general function calls it's better
@@ -76,7 +76,7 @@ impl<'ast> ItemKind<'ast> {
     impl_item_type_fn!(ItemKind: id() -> ItemId);
     impl_item_type_fn!(ItemKind: span() -> &Span<'ast>);
     impl_item_type_fn!(ItemKind: visibility() -> &Visibility<'ast>);
-    impl_item_type_fn!(ItemKind: name() -> Option<String>);
+    impl_item_type_fn!(ItemKind: ident() -> Option<&Ident<'ast>>);
     impl_item_type_fn!(ItemKind: attrs() -> ());
 }
 
@@ -92,7 +92,7 @@ impl<'ast> AssocItemKind<'ast> {
     impl_item_type_fn!(AssocItemKind: id() -> ItemId);
     impl_item_type_fn!(AssocItemKind: span() -> &Span<'ast>);
     impl_item_type_fn!(AssocItemKind: visibility() -> &Visibility<'ast>);
-    impl_item_type_fn!(AssocItemKind: name() -> Option<String>);
+    impl_item_type_fn!(AssocItemKind: ident() -> Option<&Ident<'ast>>);
     impl_item_type_fn!(AssocItemKind: attrs() -> ());
     impl_item_type_fn!(AssocItemKind: as_item() -> ItemKind<'ast>);
     // FIXME: Potentially add a field to the items to optionally store the owner id
@@ -132,7 +132,7 @@ impl<'ast> ExternItemKind<'ast> {
     impl_item_type_fn!(ExternItemKind: id() -> ItemId);
     impl_item_type_fn!(ExternItemKind: span() -> &Span<'ast>);
     impl_item_type_fn!(ExternItemKind: visibility() -> &Visibility<'ast>);
-    impl_item_type_fn!(ExternItemKind: name() -> Option<String>);
+    impl_item_type_fn!(ExternItemKind: ident() -> Option<&Ident<'ast>>);
     impl_item_type_fn!(ExternItemKind: attrs() -> ());
     impl_item_type_fn!(ExternItemKind: as_item() -> ItemKind<'ast>);
 }
@@ -194,7 +194,7 @@ use impl_item_type_fn;
 struct CommonItemData<'ast> {
     id: ItemId,
     vis: Visibility<'ast>,
-    name: SymbolId,
+    ident: Ident<'ast>,
 }
 
 macro_rules! impl_item_data {
@@ -212,8 +212,8 @@ macro_rules! impl_item_data {
                 &self.data.vis
             }
 
-            fn name(&self) -> Option<String> {
-                Some($crate::context::with_cx(self, |cx| cx.symbol_str(self.data.name)))
+            fn ident(&self) -> Option<&crate::ast::Ident<'ast>> {
+                Some(&self.data.ident)
             }
 
             fn as_item(&'ast self) -> crate::ast::item::ItemKind<'ast> {
@@ -231,11 +231,11 @@ use impl_item_data;
 
 #[cfg(feature = "driver-api")]
 impl<'ast> CommonItemData<'ast> {
-    pub fn new(id: ItemId, name: SymbolId) -> Self {
+    pub fn new(id: ItemId, ident: Ident<'ast>) -> Self {
         Self {
             id,
             vis: Visibility::new(id),
-            name,
+            ident,
         }
     }
 }

marker_api/src/ast/generic/param.rs

@@ -100,7 +100,7 @@ pub struct EnumVariant<'ast> {
 }
 
 impl<'ast> EnumVariant<'ast> {
-    pub fn ident(&self) -> String {
+    pub fn ident(&self) -> &str {
         with_cx(self, |cx| cx.symbol_str(self.ident))
     }
 
@@ -270,7 +270,7 @@ impl<'ast> Field<'ast> {
         &self.vis
     }
 
-    pub fn ident(&self) -> String {
+    pub fn ident(&self) -> &str {
         with_cx(self, |cx| cx.symbol_str(self.ident))
     }
 

marker_api/src/ast/item.rs

@@ -26,11 +26,11 @@ super::impl_item_data!(ExternCrateItem, ExternCrate);
 
 impl<'ast> ExternCrateItem<'ast> {
     /// This will return the original name of external crate. This will only differ
-    /// with [`ItemData::get_name`](`super::ItemData::name`) if the user has
+    /// with [`ItemData::ident`](`super::ItemData::ident`) if the user has
     /// declared an alias with `as`.
     ///
     /// In most cases, you want to use this over the `get_name()` function.
-    pub fn crate_name(&self) -> String {
+    pub fn crate_name(&self) -> &str {
         with_cx(self, |cx| cx.symbol_str(self.crate_name))
     }
 }

marker_api/src/ast/item/adt_item.rs

@@ -15,7 +15,7 @@ pub struct UnstableItem<'ast> {
 super::impl_item_data!(UnstableItem, Unstable);
 
 impl<'ast> UnstableItem<'ast> {
-    pub fn feature(&self) -> Option<String> {
+    pub fn feature(&self) -> Option<&str> {
         self.feature
             .get()
             .map(|feature| with_cx(self, |cx| cx.symbol_str(*feature)))

marker_api/src/ast/item/extern_crate_item.rs

@@ -18,7 +18,7 @@ pub struct IdentPat<'ast> {
 }
 
 impl<'ast> IdentPat<'ast> {
-    pub fn name(&self) -> String {
+    pub fn name(&self) -> &str {
         with_cx(self, |cx| cx.symbol_str(self.name))
     }
 

marker_api/src/ast/item/unstable_item.rs

@@ -69,7 +69,7 @@ impl<'ast> StructFieldPat<'ast> {
         with_cx(self, |cx| cx.get_span(self.span))
     }
 
-    pub fn ident(&self) -> String {
+    pub fn ident(&self) -> &str {
         with_cx(self, |cx| cx.symbol_str(self.ident))
     }
 

marker_api/src/ast/item/use_decl_item.rs renamed to marker_api/src/ast/item/use_item.rs

@@ -18,7 +18,7 @@ impl<'ast> RelativeTy<'ast> {
         self.ty
     }
 
-    pub fn name(&self) -> String {
+    pub fn name(&self) -> &str {
         with_cx(self, |cx| cx.symbol_str(self.name))
     }
 }

marker_api/src/ast/pat/ident_pat.rs

@@ -143,7 +143,7 @@ impl<'ast> AstContext<'ast> {
         self.driver.call_get_span(&span_owner.into())
     }
 
-    pub(crate) fn symbol_str(&self, sym: SymbolId) -> String {
+    pub(crate) fn symbol_str(&self, sym: SymbolId) -> &'ast str {
         self.driver.call_symbol_str(sym)
     }
 }
@@ -198,8 +198,8 @@ impl<'ast> DriverCallbacks<'ast> {
         let result: Option<ffi::Str> = (self.span_snippet)(self.driver_context, span).into();
         result.map(|x| x.to_string())
     }
-    fn call_symbol_str(&self, sym: SymbolId) -> String {
-        (self.symbol_str)(self.driver_context, sym).to_string()
+    fn call_symbol_str(&self, sym: SymbolId) -> &'ast str {
+        (self.symbol_str)(self.driver_context, sym).get()
     }
     pub fn call_body(&self, id: BodyId) -> &'ast Body<'ast> {
         (self.body)(self.driver_context, id)

marker_api/src/ast/pat/struct_pat.rs

@@ -30,6 +30,15 @@ impl<'a> From<&'a str> for Str<'a> {
     }
 }
 
+impl<'a> Str<'a> {
+    pub fn get(&self) -> &'a str {
+        unsafe {
+            let data = slice::from_raw_parts(self.data, self.len);
+            std::str::from_utf8_unchecked(data)
+        }
+    }
+}
+
 impl<'a> From<&Str<'a>> for &'a str {
     fn from(src: &Str<'a>) -> Self {
         unsafe {