Skip to content

Commit

Permalink
API: Make item Visibility visible in the API (#294)
Browse files Browse the repository at this point in the history
* API: Make item `Visibility` visible in the API

* Lints: Test visibility in `not_using_has_span_trait` and update docs

* Doc: Update changelog

* API: Update item struct size test

* Chore: First PR comments

* uilints: compile on stable again

* Chore: Second PR comments

* API: Split `ast::Visibility` into a semantic and ast IR

* Rustc: Fix `local_crate_mod` after rebase

* API: Address reivew comments <3

* API: Use extensive matching
  • Loading branch information
xFrednet authored Nov 14, 2023
1 parent 544652e commit 4cbd4fa
Show file tree
Hide file tree
Showing 24 changed files with 604 additions and 98 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,12 @@ The [v0.4.0 milestone] contains a list of planned changes.
[v0.4.0 milestone]: https://github.com/rust-marker/marker/milestone/4
[#278]: https://github.com/rust-marker/marker/pull/278
[#288]: https://github.com/rust-marker/marker/pull/288
[#294]: https://github.com/rust-marker/marker/pull/294
[#306]: https://github.com/rust-marker/marker/pull/306

### Added
- [#306]: The `LintPass` trait now as a new `check_crate` method.
- [#294]: Items and fields now provide information about their visibility

### Fixed
- [#306]: Rustc's driver no longer ICEs on spans from compiler generated code.
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ Marker is still growing up, and that's a good thing. We can still shape the API
* Higher order types
* Attributes [#51](https://github.com/rust-marker/marker/issues/51)
* Macros [rust-marker/design#47](https://github.com/rust-marker/design/issues/47)
* Item visibility [#26](https://github.com/rust-marker/marker/issues/26)
* Item visibility [#26](https://github.com/rust-marker/marker/issues/26) (Will be added in v0.4)
* **Utility**: The API is currently lacking a lot of utility functions, to handle edge cases and make linting more pleasant.
* **Documentation**: Marker still requires a lot of documentation, in the form of doc comments and a book, which explains the basic concept and works as a guide for end-users, lint- and marker-devs alike.

Expand Down
10 changes: 3 additions & 7 deletions marker_api/src/ast/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -372,14 +372,10 @@ impl<'ast> ConstExpr<'ast> {

#[cfg(all(test, target_arch = "x86_64", target_pointer_width = "64"))]
mod test {
use super::*;
use expect_test::{expect, Expect};
use crate::test::assert_size_of;

#[track_caller]
fn assert_size_of<T>(expected: &Expect) {
let actual = std::mem::size_of::<T>();
expected.assert_eq(&actual.to_string());
}
use super::*;
use expect_test::expect;

#[test]
fn expr_struct_size() {
Expand Down
101 changes: 55 additions & 46 deletions marker_api/src/ast/item.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
use std::{fmt::Debug, marker::PhantomData};
use std::fmt::Debug;

use crate::{
common::{HasNodeId, ItemId, SpanId},
context::with_cx,
diagnostic::EmissionNode,
ffi::FfiOption,
private::Sealed,
span::{HasSpan, Ident, Span},
CtorBlocker,
Expand Down Expand Up @@ -188,6 +190,7 @@ use impl_item_type_fn;
#[repr(C)]
#[derive(Debug)]
#[cfg_attr(feature = "driver-api", visibility::make(pub))]
#[cfg_attr(feature = "driver-api", derive(typed_builder::TypedBuilder))]
struct CommonItemData<'ast> {
id: ItemId,
span: SpanId,
Expand Down Expand Up @@ -236,43 +239,47 @@ macro_rules! impl_item_data {

use impl_item_data;

#[cfg(feature = "driver-api")]
impl<'ast> CommonItemData<'ast> {
pub fn new(id: ItemId, span: SpanId, ident: Ident<'ast>) -> Self {
Self {
id,
span,
vis: Visibility::new(id),
ident,
}
}
}

/// This struct represents the visibility of an item.
/// The declared visibility of an item or field.
///
/// Note that this is only the syntactic visibility. The item or field might be
/// reexported with a higher visibility, or have a high default visibility.
///
/// ```
/// // An item without a visibility
/// fn moon() {}
///
/// // A public item
/// pub fn sun() {}
///
/// Currently, it's only a placeholder until a proper representation is implemented.
/// rust-marker/marker#26 tracks the task of implementing this. You're welcome to
/// leave any comments in that issue.
/// // An item with a restricted scope
/// pub(crate) fn star() {}
///
/// pub trait Planet {
/// // An item without a visibility. But it still has the semantic visibility
/// // of `pub` as this is inside a trait declaration.
/// fn mass();
/// }
/// ```
#[repr(C)]
#[derive(Debug)]
#[cfg_attr(feature = "driver-api", derive(typed_builder::TypedBuilder))]
pub struct Visibility<'ast> {
_lifetime: PhantomData<&'ast ()>,
_item_id: ItemId,
#[cfg_attr(feature = "driver-api", builder(setter(into), default))]
span: FfiOption<SpanId>,
sem: crate::sem::Visibility<'ast>,
}

impl<'ast> Debug for Visibility<'ast> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("Visibility {{ /* WIP: See rust-marker/marker#26 */}}")
.finish()
impl<'ast> Visibility<'ast> {
/// The [`Span`] of the visibility, if it has been declared.
pub fn span(&self) -> Option<&Span<'ast>> {
self.span.copy().map(|span| with_cx(self, |cx| cx.span(span)))
}
}

#[cfg(feature = "driver-api")]
impl<'ast> Visibility<'ast> {
pub fn new(item_id: ItemId) -> Self {
Self {
_lifetime: PhantomData,
_item_id: item_id,
}
/// This function returns the semantic representation for the [`Visibility`]
/// of this item. That visibility can be used to check if the item is public
/// or restricted to specific modules.
pub fn semantics(&self) -> &crate::sem::Visibility<'ast> {
&self.sem
}
}

Expand Down Expand Up @@ -311,26 +318,28 @@ impl<'ast> Body<'ast> {

#[cfg(all(test, target_arch = "x86_64", target_pointer_width = "64"))]
mod test {
use crate::test::assert_size_of;

use super::*;
use std::mem::size_of;
use expect_test::expect;

#[test]
fn test_item_struct_size() {
// These sizes are allowed to change, this is just a check to have a
// general overview and to prevent accidental changes
assert_eq!(56, size_of::<ModItem<'_>>(), "ModItem");
assert_eq!(48, size_of::<ExternCrateItem<'_>>(), "ExternCrateItem");
assert_eq!(64, size_of::<UseItem<'_>>(), "UseItem");
assert_eq!(80, size_of::<StaticItem<'_>>(), "StaticItem");
assert_eq!(72, size_of::<ConstItem<'_>>(), "ConstItem");
assert_eq!(144, size_of::<FnItem<'_>>(), "FnItem");
assert_eq!(112, size_of::<TyAliasItem<'_>>(), "TyAliasItem");
assert_eq!(96, size_of::<StructItem<'_>>(), "StructItem");
assert_eq!(88, size_of::<EnumItem<'_>>(), "EnumItem");
assert_eq!(88, size_of::<UnionItem<'_>>(), "UnionItem");
assert_eq!(112, size_of::<TraitItem<'_>>(), "TraitItem");
assert_eq!(144, size_of::<ImplItem<'_>>(), "ImplItem");
assert_eq!(64, size_of::<ExternBlockItem<'_>>(), "ExternBlockItem");
assert_eq!(48, size_of::<UnstableItem<'_>>(), "UnstableItem");
assert_size_of::<ModItem<'_>>(&expect!["80"]);
assert_size_of::<ExternCrateItem<'_>>(&expect!["72"]);
assert_size_of::<UseItem<'_>>(&expect!["88"]);
assert_size_of::<StaticItem<'_>>(&expect!["104"]);
assert_size_of::<ConstItem<'_>>(&expect!["96"]);
assert_size_of::<FnItem<'_>>(&expect!["168"]);
assert_size_of::<TyAliasItem<'_>>(&expect!["136"]);
assert_size_of::<StructItem<'_>>(&expect!["120"]);
assert_size_of::<EnumItem<'_>>(&expect!["112"]);
assert_size_of::<UnionItem<'_>>(&expect!["112"]);
assert_size_of::<TraitItem<'_>>(&expect!["136"]);
assert_size_of::<ImplItem<'_>>(&expect!["168"]);
assert_size_of::<ExternBlockItem<'_>>(&expect!["88"]);
assert_size_of::<UnstableItem<'_>>(&expect!["72"]);
}
}
3 changes: 2 additions & 1 deletion marker_api/src/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,10 @@ impl std::hash::Hash for FfiStr<'_> {
/// This is an FFI safe option. In most cases it's better to pass a pointer and
/// then use `as_ref()` but this doesn't work for owned return values.
#[repr(C)]
#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq)]
#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq, Default)]
pub enum FfiOption<T> {
Some(T),
#[default]
None,
}

Expand Down
10 changes: 4 additions & 6 deletions marker_api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,7 @@
#![allow(clippy::missing_panics_doc)] // Temporary allow for `todo!`s
#![allow(clippy::new_without_default)] // Not very helpful as `new` is almost always cfged
#![cfg_attr(not(feature = "driver-api"), allow(dead_code))]
// FIXME(#26): this is commented out because it is the lint that we want to enable
// here makes sense only on a public items, but it triggers of private/pub(crate)
// methods today. There isn't a way to inspect the item visibility in this lint's
// impl yet. Once #26 is done and lint impl ignores private functions we may enable
// this lint.
// #![cfg_attr(marker, warn(marker::not_using_has_span_trait))]
#![cfg_attr(marker, warn(marker::marker_lints::not_using_has_span_trait))]

pub static MARKER_API_VERSION: &str = env!("CARGO_PKG_VERSION");

Expand All @@ -22,6 +17,9 @@ pub use interface::*;
mod lint;
pub use lint::*;

#[cfg(test)]
pub(crate) mod test;

pub mod ast;
pub mod common;
pub mod context;
Expand Down
2 changes: 2 additions & 0 deletions marker_api/src/sem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@

mod common;
mod generic;
mod item;
mod ty;

pub use common::*;
pub use generic::*;
pub use item::*;
pub use ty::*;
153 changes: 153 additions & 0 deletions marker_api/src/sem/item.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
use std::marker::PhantomData;

use crate::common::ItemId;

/// The declared visibility of an item or field.
///
/// ```
/// // An item without a visibility
/// fn moon() {}
///
/// // A public item
/// pub fn sun() {}
///
/// // An item with a restricted scope
/// pub(crate) fn star() {}
///
/// pub trait Planet {
/// // An item without a visibility. But it still has the semantic visibility
/// // of `pub` as this is inside a trait declaration.
/// fn mass();
/// }
/// ```
#[repr(C)]
#[derive(Debug)]
#[cfg_attr(feature = "driver-api", derive(typed_builder::TypedBuilder))]
pub struct Visibility<'ast> {
#[cfg_attr(feature = "driver-api", builder(setter(skip), default))]
_lifetime: PhantomData<&'ast ()>,
kind: VisibilityKind,
}

impl<'ast> Visibility<'ast> {
/// Returns `true` if the item is public, meaning that it can be visible outside
/// the crate.
///
/// ```
/// // This returns `true`
/// pub fn unicorn() {}
///
/// // This returns `false`, since the visibility is restricted to a specified path.
/// pub(crate) fn giraffe() {}
///
/// // This returns `false`, since the visibility is not defined
/// fn dragon() {}
///
/// pub trait MythicalCreature {
/// // This returns `true`, since the default visibility for trait items is public
/// fn name() -> &'static str;
/// }
/// ```
pub fn is_pub(&self) -> bool {
self.scope().is_none()
}

/// Returns `true` if the item is visible in the entire crate. This is
/// the case for items declared as `pub(crate)`. Items declared in the root
/// module of the crate or specify the path of the root module are also
/// scoped to the entire crate.
///
/// ```
/// // lib.rs
///
/// mod example_1 {
/// // Returns `false` since it's only visible in `crate::example_1`
/// fn foo() {}
///
/// // Returns `false` since it's only visible in `crate::example_1`
/// pub(in crate::example_1) fn bar() {}
///
/// // Returns `true` as the visibility is restricted to the root of the crate.
/// pub(crate) fn baz() {}
///
/// // Returns `true` as the visibility is restricted to `super` which is the
/// // root of the crate.
/// pub(super) fn boo() {}
/// }
///
/// // Returns `true` since this item is at the root of the crate and the default
/// // visibility is therefore `pub(crate)`
/// fn example_in_root() {}
///
/// # fn main() {}
/// ```
pub fn is_crate_scoped(&self) -> bool {
matches!(self.kind, VisibilityKind::Crate(_) | VisibilityKind::DefaultCrate(_))
}

/// Returns `true` if a visibility is the default visibility, meaning that it wasn't
/// declared.
pub fn is_default(&self) -> bool {
match self.kind {
VisibilityKind::DefaultPub | VisibilityKind::DefaultCrate(_) | VisibilityKind::Default(_) => true,
VisibilityKind::Public | VisibilityKind::Crate(_) | VisibilityKind::Path(_) => false,
}
}

/// Returns the [`ItemId`] of the module that this item or field is visible in.
/// It will return `None`, if the item is public, as the visibility extends past
/// the root module of the crate.
///
/// This function also works for items which have the default visibility, of the
/// module they are declared in.
///
/// ```
/// // lib.rs
///
/// mod scope {
/// // Returns `None` since this is even visible outside the current crate
/// pub fn turtle() {}
///
/// // Returns the `ItemId` of the root module of the crate
/// pub(crate) fn shark() {}
///
/// // Returns the `ItemId` of the module it is declared in, in this case `crate::scope`
/// fn dolphin() {}
/// }
/// ```
pub fn scope(&self) -> Option<ItemId> {
match self.kind {
VisibilityKind::Path(id)
| VisibilityKind::Crate(id)
| VisibilityKind::DefaultCrate(id)
| VisibilityKind::Default(id) => Some(id),
VisibilityKind::Public | VisibilityKind::DefaultPub => None,
}
}

// FIXME(xFrednet): Implement functions to check if an item is visible from a
// given `ItemId`. This can be done once rust-marker/marker#242 is implemented.
}

#[derive(Debug)]
#[allow(clippy::exhaustive_enums)]
#[cfg_attr(feature = "driver-api", visibility::make(pub))]
enum VisibilityKind {
/// The item is declared as `pub` without any restrictions
Public,
/// For items which are `pub` by default, like trait functions or enum variants
DefaultPub,
/// The visibility is restricted to the root module of the crate. The [`ItemId`]
/// identifies the root module.
Crate(ItemId),
/// The items doesn't have a declared visibility. The default is visible in the
/// entire crate. The [`ItemId`] identifies the root module.
DefaultCrate(ItemId),
/// The visibility is restricted to a specific module using `pub(<path>)`.
/// The module, targeted by the path is identified by the [`ItemId`].
/// The `pub(crate)` has it's own variant in this struct.
Path(ItemId),
/// The items doesn't have a declared visibility. The default is restricted to
/// a module, identified by the stored [`ItemId`]
Default(ItemId),
}
7 changes: 7 additions & 0 deletions marker_api/src/test.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
use expect_test::Expect;

#[track_caller]
pub(crate) fn assert_size_of<T>(expected: &Expect) {
let actual = std::mem::size_of::<T>();
expected.assert_eq(&actual.to_string());
}
Loading

0 comments on commit 4cbd4fa

Please sign in to comment.