Merge #103

103: Add `QualifiedAstPath` and `PathExpr` representation r=Niki4tap a=xFrednet

This is a draft PR to get some early feedback on the `QualifiedAstPath` representation. [Qualified paths](https://doc.rust-lang.org/reference/paths.html?search=QualifiedPathInType#qualified-paths) can be used in [expressions](https://doc.rust-lang.org/reference/expressions/path-expr.html) and [types](https://doc.rust-lang.org/reference/types.html?highlight=QualifiedPathInType#type-expressions). These are type relative and allow the definition of the `Self` type for traits (Example: `<Vec<u32> as Default>::default()`). This can be relevant, if a struct has several functions with the same name. An example is shown at the end of the PR description.

## Representation

The current [`AstPath`](https://github.com/rust-marker/marker/blob/master/marker_api/src/ast/common/ast_path.rs#L13) representation is a simple slice of segments, which have an ident and potentially generic args. This representation is already a hybrid of [simple paths](https://doc.rust-lang.org/reference/paths.html#simple-paths) and [expression paths](https://doc.rust-lang.org/reference/paths.html#paths-in-expressions). The simple path, has the same structure as an expression path, with the difference, that it can't have generic arguments. In the current implementation, this simply means that all generic arguments are empty, for simple paths.

[Qualified paths](https://doc.rust-lang.org/reference/paths.html?search=QualifiedPathInType#qualified-paths) are not as easily combinable with the existing `AstPath` as it can contain segments with `Self` type specification. Generally, all places, where a qualified path can be used, also a normal path is usable. It therefore makes sense to combine them in some shape or form. I thought of a two main ways to represent this:

1. Everything can be represented as one `AstPath`. The segments would then be an enum, with either an identifier or the `Self` type specification.

    The disadvantage is that the segment enum, would need to be handled everywhere, even in locations where the path can never be qualified.
2. Create a wrapper type, which optionally specifies a `Self` type. This is the way rustc does it with [`QPath`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_hir/enum.QPath.html)

    I always found this confusing until now, when I was forces to fully understand how qualified paths work. I also think that just providing a `Path` directly, can cause users to use it directly, without considering, if the `Self` type is relevant. In most cases, this will be fine, but it might be possible to express the semantics better.

This PR proposes a variant of the second option, with a lot of documentation. The `AstPath` representation can be accessed via the `TryInto` trait or the `to_path_lossy` function. This ensures that users, have simple access to the `AstPath` representation, but don't accadently ignore potentual `Self` type specifications.

## Naming

Another question I have, is about naming. I find `QualifiedAstPath` very wordy. Can we maybe find a short version? Maybe, one of the following, if the meaning is clear
1. `QAstPath`
2. `AstQPath`
3. `QPath`
4. `QualifiedPath`

---

Example when qualified paths are important:

```rs
trait Foo {
    fn foo() {
        println!("foo() from Trait");
    }
}

struct Item;

impl Item {
    fn foo() {
        println!("foo() from Item")
    }
}
impl Foo for Item {}

pub fn main() {
    Item::foo(); // "foo() from Item"
    <Item as Foo>::foo(); // "foo() from Trait"
}
```

---

I plan to keep this as a draft PR, until I have made some modifications to the type representation and added the rustc backend, to ensure that the chosen representation can really express the semantics. The representation can still be changed later. I might create an issue in the design repo, to have a second look at the representation before making it stable.

---

cc: #52

`@Niki4tap` Do you maybe have any thoughts on this? 🙃 

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

Author: 3 people

marker_api/src/ast/common/ast_path.rs

@@ -5,21 +5,234 @@
 // FIXME: It might be useful to not use a single path for everything, but instead
 // split it up into an `ItemPath`, `GenericPath` etc. implementation.
 
-use super::SymbolId;
-use crate::{ast::generic::GenericArgs, context::with_cx, ffi::FfiSlice};
+use super::{GenericId, Ident, ItemId, VarId};
+use crate::{
+    ast::{generic::GenericArgs, ty::TyKind},
+    ffi::{FfiOption, FfiSlice},
+};
+
+/// [`AstPath`]s are used to identify items. A qualified path (`QPath`) can be
+/// used in expressions and types to identify associated items on types. For
+/// traits it's additionally possible to specify the type that should be used
+/// as `Self`. This is sometimes needed to disambiguate an item, if it exists
+/// both as an associated item on a type, and as an associated item on traits,
+/// that this type implements.
+///
+/// In the following example, the `Item` has two implementations of the associated
+/// `foo()` function. One is provided by the `impl` block and the other one by the
+/// `Foo` trait. When calling `Item::foo()` the `impl` block implementation will
+/// targeted. To access the trait function, the path has to be specified, with `Item`
+/// declared as the `Self` type.
+///
+/// ```
+/// // Item
+/// struct Item;
+/// impl Item {
+///     fn foo() {
+///         println!("foo() from Item")
+///     }
+/// }
+///
+/// // trait
+/// trait Foo {
+///     fn foo();
+/// }
+/// impl Foo for Item {
+///     fn foo() {
+///         println!("foo() from Foo trait");
+///     }
+/// }
+///
+/// // Calls the `foo()` method of `impl Item`
+/// Item::foo(); // -> "foo() from Item"
+///
+/// // Calls the `foo()` method of `trait Foo` with `Item` as the `Self` type
+/// <Item as Foo>::foo(); // -> "foo() from Foo trait"
+/// ```
+///
+/// This representation can also be used to reference non-associated items, to
+/// make it more flexible. For these items, the path type will be [`None`]. The
+/// target can be resolved via the [`resolve()`](AstQPath::resolve) method.
+/// Alternatively, the [`AstPath`] representation can be accessed via
+/// [`as_path_lossy()`](AstQPath::as_path_lossy) or the [`TryInto`] implementation.
+#[repr(C)]
+#[derive(Debug, PartialEq, Eq, Hash)]
+pub struct AstQPath<'ast> {
+    self_ty: FfiOption<TyKind<'ast>>,
+    path_ty: FfiOption<TyKind<'ast>>,
+    path: AstPath<'ast>,
+    target: AstPathTarget,
+}
+
+impl<'ast> AstQPath<'ast> {
+    /// This method will return [`Some`], if the path has a specified `Self`
+    /// type. The main type for type relative paths is provided by
+    /// [`path_ty()`][`AstQPath::path_ty()`].
+    ///
+    /// ```
+    /// # let _: Vec<i32> =
+    ///     <Vec<_> as Default>::default();
+    /// //   ^^^^^^ The specified `Self` type `Vec<_>`
+    /// ```
+    ///
+    /// The [`AstQPath`] description contains more details, when this might
+    /// be necessary.
+    pub fn self_ty(&self) -> Option<TyKind<'ast>> {
+        self.self_ty.copy()
+    }
+
+    /// This method will return [`Some`], if the path is type relative.
+    ///
+    /// ```
+    /// # let _: Vec<i32> =
+    ///     <Vec<_> as Default>::default();
+    /// //             ^^^^^^^ The path is relative to the `Default` trait
+    /// ```
+    ///
+    /// The optional `Self` type can be accessed via [`self_ty`](AstQPath::self_ty()).
+    pub fn path_ty(&self) -> Option<TyKind<'ast>> {
+        self.path_ty.copy()
+    }
+
+    /// This returns a [`AstPath`] of the referenced item. For type relative
+    /// paths, this will include the type itself, if the type can be expressed
+    /// as a [`AstPathSegment`]. For some types l,ike slices, this is not possible.
+    /// The type has to be retrieved from [`path_ty()`][`AstQPath::path_ty()`].
+    ///
+    /// ### Examples:
+    ///
+    /// ```
+    /// let _: Vec<i32> = Vec::default();
+    /// // AstPath: `Vec::default`
+    ///
+    /// let _: Vec<i32> = Default::default();
+    /// // AstPath: `Default::default`
+    ///
+    /// let _: Vec<i32> = <Vec<_> as Default>::default();
+    /// // AstPath: `Default::default`
+    ///
+    ///  let _ = [0_u8].is_ascii();
+    /// // AstPath: `is_ascii`
+    /// ```
+    ///
+    /// ### Warning
+    ///
+    /// The method is lossy, as the optional `Self` type isn't included in this
+    /// path. The path type might also be missing, if it can't be represented as
+    /// a [`AstPathSegment`]. The conversion is lossless, if both types are none
+    /// or if the path type can be represented. To resolve a qualified path
+    /// [`resolve()`](Self::resolve()) should be used.
+    ///
+    /// Omitting the `Self` type can be useful, in cases, where access to associated
+    /// trait items should be analyzed, regardless of potential `Self` types.
+    /// Alternatively, [`segments()`](AstQPath::segments()) can be used to access the
+    /// segments directly.
+    pub fn as_path_lossy(&self) -> &AstPath<'ast> {
+        &self.path
+    }
+
+    /// This returns the [`AstPathSegment`]s of the path. For type relative
+    /// paths, this will include the type itself. The optional `Self` type
+    /// isn't represented in these segments. These segments are identical with
+    /// the segments provided by the path of
+    /// [`as_path_lossy()`](AstQPath::as_path_lossy()). The documentation
+    /// of that function contains more details.
+    pub fn segments(&self) -> &[AstPathSegment<'ast>] {
+        self.path.segments()
+    }
+
+    /// This function resolves the target of this path.
+    pub fn resolve(&self) -> AstPathTarget {
+        // For rust-analyzer or future drivers, it might make sense to return
+        // `Option<AstPathTarget>` instead, as the path might be dead,
+        // when a lint crate calls this function. However, I have the feeling
+        // that this would make the API less ergonomic. The `AstContext` will
+        // already need to handle these cases explicitly. Currently, a user can
+        // get a resolved id from the target, but the resolution of the ID, by
+        // the `AstContext`, might fail. The outcome is the same, but all
+        // "failable" resolution will be grouped in the `AstContext`
+        self.target
+    }
+
+    /// This returns the [`GenericArgs`] specified on the last segment of the path.
+    /// This is especially useful, for paths pointing to types or functions. For
+    /// example, the `u32` of the path `Vec<u32>`, is stored in the [`GenericArgs`]
+    /// as a type parameter.
+    pub fn generics(&self) -> &GenericArgs<'ast> {
+        self.path.generics()
+    }
+}
+
+impl<'a, 'ast> TryFrom<&'a AstQPath<'ast>> for &'a AstPath<'ast> {
+    type Error = ();
+
+    fn try_from(value: &'a AstQPath<'ast>) -> Result<Self, Self::Error> {
+        fn is_segment_representable(ty: Option<TyKind<'_>>) -> bool {
+            if let Some(ty) = ty {
+                ty.is_primitive_ty()
+                    || matches!(ty, TyKind::Path(path_ty) if is_segment_representable(path_ty.path().path_ty()))
+            } else {
+                true
+            }
+        }
+        if value.self_ty.is_some() && is_segment_representable(value.path_ty.copy()) {
+            Err(())
+        } else {
+            Ok(&value.path)
+        }
+    }
+}
+
+#[cfg(feature = "driver-api")]
+impl<'ast> AstQPath<'ast> {
+    pub fn new(
+        self_ty: Option<TyKind<'ast>>,
+        path_ty: Option<TyKind<'ast>>,
+        path: AstPath<'ast>,
+        target: AstPathTarget,
+    ) -> Self {
+        Self {
+            self_ty: self_ty.into(),
+            path_ty: path_ty.into(),
+            path,
+            target,
+        }
+    }
+}
+
+#[repr(C)]
+#[non_exhaustive]
+#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
+pub enum AstPathTarget {
+    /// The `Self` type, the [`ItemId`] points to the item,
+    /// that the `Self` originates from. This will usually be an
+    /// [`ImplItem`](crate::ast::item::ImplItem) or
+    /// [`TraitItem`](crate::ast::item::TraitItem).
+    SelfTy(ItemId),
+    /// The path points to an item, identified by the [`ItemId`]. For example
+    /// [`ConstItem`](crate::ast::item::ConstItem),
+    /// [`StaticItem`](crate::ast::item::ImplItem),
+    /// [`FnItem`](crate::ast::item::FnItem).
+    Item(ItemId),
+    /// The path target is a local variable, identified by the [`VarId`].
+    Var(VarId),
+    /// The path target is a generic type, identified by the [`GenericId`].
+    Generic(GenericId),
+    /// The target wasn't resolved, but should be available in the future.
+    Wip,
+}
 
 #[repr(C)]
 #[derive(Debug, PartialEq, Eq, Hash)]
 pub struct AstPath<'ast> {
     // FIXME: Add optional target ID for values, lifetimes, etc that is faster to compare
-    //
-    // You were last trying to fix the compiler error related to lifetime identification and paths
     segments: FfiSlice<'ast, AstPathSegment<'ast>>,
 }
 
 #[cfg(feature = "driver-api")]
 impl<'ast> AstPath<'ast> {
     pub fn new(segments: &'ast [AstPathSegment<'ast>]) -> Self {
+        debug_assert!(!segments.is_empty());
         Self {
             segments: segments.into(),
         }
@@ -30,25 +243,38 @@ impl<'ast> AstPath<'ast> {
     pub fn segments(&self) -> &[AstPathSegment<'ast>] {
         self.segments.get()
     }
+
+    /// This returns the [`GenericArgs`] specified on the last segment of the path.
+    /// This is especially useful, for paths pointing to types or functions. For
+    /// example, the `u32` of the path `Vec<u32>`, is stored in the [`GenericArgs`]
+    /// as a type parameter.
+    pub fn generics(&self) -> &GenericArgs<'ast> {
+        self.segments
+            .get()
+            .last()
+            .expect("a path always has at least one segment")
+            .generics()
+    }
 }
 
 #[repr(C)]
 #[derive(Debug, PartialEq, Eq, Hash)]
+#[cfg_attr(feature = "driver-api", derive(Clone))]
 pub struct AstPathSegment<'ast> {
-    ident: SymbolId,
+    ident: Ident<'ast>,
     generics: GenericArgs<'ast>,
 }
 
 #[cfg(feature = "driver-api")]
 impl<'ast> AstPathSegment<'ast> {
-    pub fn new(ident: SymbolId, generics: GenericArgs<'ast>) -> Self {
+    pub fn new(ident: Ident<'ast>, generics: GenericArgs<'ast>) -> Self {
         Self { ident, generics }
     }
 }
 
 impl<'ast> AstPathSegment<'ast> {
-    pub fn ident(&self) -> &str {
-        with_cx(self, |cx| cx.symbol_str(self.ident))
+    pub fn ident(&self) -> &Ident<'ast> {
+        &self.ident
     }
 
     pub fn generics(&self) -> &GenericArgs<'ast> {

marker_api/src/ast/common/span.rs

@@ -148,6 +148,8 @@ impl From<SpanId> for SpanOwner {
     }
 }
 
+#[derive(PartialEq, Eq, Hash)]
+#[cfg_attr(feature = "driver-api", derive(Clone))]
 pub struct Ident<'ast> {
     _lifetime: PhantomData<&'ast ()>,
     sym: SymbolId,

marker_api/src/ast/expr.rs

@@ -15,10 +15,14 @@ pub use int_lit_expr::*;
 pub use str_lit_expr::*;
 // other expressions
 mod block_expr;
+mod call_exprs;
 mod op_exprs;
+mod path_expr;
 mod unstable_expr;
 pub use block_expr::*;
+pub use call_exprs::*;
 pub use op_exprs::*;
+pub use path_expr::*;
 pub use unstable_expr::*;
 
 pub trait ExprData<'ast>: Debug {
@@ -50,6 +54,8 @@ pub enum ExprKind<'ast> {
     BinaryOp(&'ast BinaryOpExpr<'ast>),
     QuestionMark(&'ast QuestionMarkExpr<'ast>),
     As(&'ast AsExpr<'ast>),
+    Path(&'ast PathExpr<'ast>),
+    Call(&'ast CallExpr<'ast>),
     Unstable(&'ast UnstableExpr<'ast>),
 }
 
@@ -70,6 +76,7 @@ pub enum ExprPrecedence {
     Path = 0x1300_0000,
 
     Method = 0x1200_0000,
+    Call = 0x1200_0001,
 
     Field = 0x1100_0000,
 
@@ -143,7 +150,7 @@ macro_rules! impl_expr_kind_fn {
     ($method:ident () -> $return_ty:ty) => {
         impl_expr_kind_fn!($method() -> $return_ty,
             IntLit, FloatLit, StrLit, CharLit, BoolLit, Block, UnaryOp, Borrow,
-            BinaryOp, QuestionMark, As, Unstable
+            BinaryOp, QuestionMark, As, Path, Call, Unstable
         );
     };
     ($method:ident () -> $return_ty:ty $(, $kind:ident)+) => {

marker_api/src/ast/expr/call_exprs.rs

@@ -0,0 +1,55 @@
+use crate::ffi::FfiSlice;
+
+use super::{CommonExprData, ExprKind};
+
+/// A [call expression](https://doc.rust-lang.org/reference/expressions/call-expr.html#call-expressions)
+/// calling a function. The called function is identified using an expression,
+/// called *operand*. The following shows a few examples of call expressions:
+/// ```
+/// # pub fn foo(_: u32) {}
+/// //  vvv The operand pointing to a function called `foo`
+///     foo(1);
+/// //      ^ A number literal as an argument
+///
+/// # let _: Vec<u32> =
+///     Vec::new();
+/// //  ^^^^^^^^ The operand pointing to the associated function `new()` for
+/// //           the type `Vec<_>`. This is not a method call, as the function
+/// //           doesn't take `self` as an argument.
+///
+///     (|| "Closures are cool")();
+/// //  ^^^^^^^^^^^^^^^^^^^^^^^^ A closure expression as an operand
+/// ```
+#[repr(C)]
+#[derive(Debug)]
+pub struct CallExpr<'ast> {
+    data: CommonExprData<'ast>,
+    operand: ExprKind<'ast>,
+    args: FfiSlice<'ast, ExprKind<'ast>>,
+}
+
+impl<'ast> CallExpr<'ast> {
+    /// The expression identifying what will be called. This will often be a
+    /// [`PathExpr`](super::PathExpr).
+    pub fn operand(&self) -> ExprKind<'ast> {
+        self.operand
+    }
+
+    /// The arguments given to the operand.
+    pub fn args(&self) -> &[ExprKind<'ast>] {
+        self.args.get()
+    }
+}
+
+super::impl_expr_data!(CallExpr<'ast>, Call);
+
+#[cfg(feature = "driver-api")]
+impl<'ast> CallExpr<'ast> {
+    pub fn new(data: CommonExprData<'ast>, operand: ExprKind<'ast>, args: &'ast [ExprKind<'ast>]) -> Self {
+        Self {
+            data,
+            operand,
+            args: args.into(),
+        }
+    }
+}