Skip to content

Commit

Permalink
chore: clean up some TODOs
Browse files Browse the repository at this point in the history
  • Loading branch information
DaniPopes committed Nov 4, 2024
1 parent 92e0500 commit e6193d1
Show file tree
Hide file tree
Showing 9 changed files with 54 additions and 52 deletions.
5 changes: 2 additions & 3 deletions crates/interface/src/diagnostics/emitter/human.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ use std::{
sync::Arc,
};

// TODO: Tabs are not formatted correctly: https://github.com/rust-lang/annotate-snippets-rs/issues/25

type Writer = dyn Write + Send + 'static;

const DEFAULT_RENDERER: Renderer = Renderer::plain()
Expand Down Expand Up @@ -331,7 +333,6 @@ impl OwnedSnippet {
for line in &mut sub_file.lines {
for ann in &mut line.annotations {
ann.level = Some(sub.level);
// TODO: Is this right?
if ann.is_primary && ann.label.is_none() {
ann.label = Some(label.to_string());
}
Expand Down Expand Up @@ -378,8 +379,6 @@ impl OwnedSnippet {
}
}

// TODO: Tabs are not handled correctly.

/// Merges back multi-line annotations that were split across multiple lines into a single
/// annotation that's suitable for `annotate-snippets`.
///
Expand Down
2 changes: 1 addition & 1 deletion crates/interface/src/source_map/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ impl std::error::Error for OffsetOverflowError {}

impl From<OffsetOverflowError> for io::Error {
fn from(e: OffsetOverflowError) -> Self {
// TODO: Use `FileTooLarge` when `io_error_more` is stable.
// TODO(MSRV-1.83): Use `FileTooLarge`.
Self::new(io::ErrorKind::Other, e)
}
}
Expand Down
4 changes: 2 additions & 2 deletions crates/sema/src/ast_lowering/linearize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ impl super::LoweringContext<'_, '_, '_> {
for &decl in decls {
// Import if it was declared in the base, is not the constructor and is
// visible in derived classes.
let Res::Item(decl_item_id) = decl.kind else { continue };
let Res::Item(decl_item_id) = decl.res else { continue };
let decl_item = self.hir.item(decl_item_id);
if decl_item.contract() != Some(base_id) {
continue;
Expand All @@ -52,7 +52,7 @@ impl super::LoweringContext<'_, '_, '_> {
use hir::ItemId::*;
use Res::*;

let Item(conflict_id) = conflict.kind else { continue };
let Item(conflict_id) = conflict.res else { continue };
match (decl_item_id, conflict_id) {
// Usual shadowing is not an error.
(Function(a), Function(b)) => {
Expand Down
60 changes: 30 additions & 30 deletions crates/sema/src/ast_lowering/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ impl super::LoweringContext<'_, '_, '_> {
for &item_id in source.items {
let item = self.hir.item(item_id);
if let Some(name) = item.name() {
let decl = Declaration { kind: Res::Item(item_id), span: name.span };
let decl = Declaration { res: Res::Item(item_id), span: name.span };
let _ = self.declare_in(&mut scope, name.name, decl);
}
}
Expand Down Expand Up @@ -54,7 +54,7 @@ impl super::LoweringContext<'_, '_, '_> {
match import.items {
ast::ImportItems::Plain(alias) | ast::ImportItems::Glob(alias) => {
if let Some(alias) = alias {
let _ = source_scope.declare_kind(
let _ = source_scope.declare_res(
self.sess,
&self.hir,
alias,
Expand Down Expand Up @@ -130,7 +130,7 @@ impl super::LoweringContext<'_, '_, '_> {
sess.source_map().filename_for_diagnostics(&source.file.name)
);
let guar = sess.dcx.err(msg).span(import.span).emit();
let _ = source_scope.declare_kind(sess, hir, name, Res::Err(guar));
let _ = source_scope.declare_res(sess, hir, name, Res::Err(guar));
}
}

Expand All @@ -148,9 +148,9 @@ impl super::LoweringContext<'_, '_, '_> {

// Declare `this` and `super`.
let span = Span::DUMMY;
let this = Declaration { kind: Res::Builtin(Builtin::This), span };
let this = Declaration { res: Res::Builtin(Builtin::This), span };
let _ = self.declare_in(&mut scope, sym::this, this);
let super_ = Declaration { kind: Res::Builtin(Builtin::Super), span };
let super_ = Declaration { res: Res::Builtin(Builtin::Super), span };
let _ = self.declare_in(&mut scope, sym::super_, super_);

for &item_id in contract.items {
Expand Down Expand Up @@ -572,7 +572,7 @@ impl super::LoweringContext<'_, '_, '_> {
name: Ident,
decl: Res,
) -> Result<(), ErrorGuaranteed> {
scope.declare_kind(self.sess, &self.hir, name, decl)
scope.declare_res(self.sess, &self.hir, name, decl)
}

fn declare_in(
Expand Down Expand Up @@ -635,7 +635,7 @@ impl<'sess, 'hir, 'a> ResolveContext<'sess, 'hir, 'a> {

fn resolve_path(&self, path: &ast::PathSlice) -> Result<&'hir [Res], ErrorGuaranteed> {
self.resolve_paths(path)
.map(|decls| &*self.arena.alloc_slice_fill_iter(decls.iter().map(|decl| decl.kind)))
.map(|decls| &*self.arena.alloc_slice_fill_iter(decls.iter().map(|decl| decl.res)))
}

fn resolve_path_as<T: TryFrom<Res>>(
Expand Down Expand Up @@ -741,8 +741,8 @@ impl<'sess, 'hir, 'a> ResolveContext<'sess, 'hir, 'a> {
self.hir.variables[id].initializer = self.lower_expr_opt(var.initializer.as_deref());
let mut guar = Ok(());
if let Some(name) = var.name {
let decl = Res::Item(hir::ItemId::Variable(id));
guar = self.scopes.current_scope().declare_kind(self.sess, self.hir, name, decl);
let res = Res::Item(hir::ItemId::Variable(id));
guar = self.scopes.current_scope().declare_res(self.sess, self.hir, name, res);
}
(id, guar)
}
Expand Down Expand Up @@ -871,7 +871,7 @@ impl<'sess, 'hir, 'a> ResolveContext<'sess, 'hir, 'a> {
ast::ExprKind::Ident(name) => {
match self.resolve_paths(ast::PathSlice::from_ref(name)) {
Ok(decls) => hir::ExprKind::Ident(
self.arena.alloc_slice_fill_iter(decls.iter().map(|decl| decl.kind)),
self.arena.alloc_slice_fill_iter(decls.iter().map(|decl| decl.res)),
),
Err(guar) => hir::ExprKind::Err(guar),
}
Expand Down Expand Up @@ -1022,18 +1022,18 @@ pub(crate) struct SymbolResolver<'sess> {
pub(crate) source_scopes: IndexVec<hir::SourceId, Declarations>,
pub(crate) contract_scopes: IndexVec<hir::ContractId, Declarations>,
global_builtin_scope: Declarations,
inner_builtin_scopes: Box<[Option<Declarations>; Builtin::COUNT]>,
builtin_members_scopes: Box<[Option<Declarations>; Builtin::COUNT]>,
}

impl<'sess> SymbolResolver<'sess> {
pub(crate) fn new(dcx: &'sess DiagCtxt) -> Self {
let (global_builtin_scope, inner_builtin_scopes) = crate::builtins::scopes();
let (global_builtin_scope, builtin_members_scopes) = crate::builtins::scopes();
Self {
dcx,
source_scopes: IndexVec::new(),
contract_scopes: IndexVec::new(),
global_builtin_scope,
inner_builtin_scopes,
builtin_members_scopes,
}
}

Expand All @@ -1044,10 +1044,10 @@ impl<'sess> SymbolResolver<'sess> {
description: &str,
) -> Result<T, ErrorGuaranteed> {
let decl = self.resolve_path(path, scopes).map_err(self.emit_resolver_error())?;
if let Res::Err(guar) = decl.kind {
if let Res::Err(guar) = decl.res {
return Err(guar);
}
T::try_from(decl.kind)
T::try_from(decl.res)
.map_err(|_| self.report_expected(description, decl.description(), path.span()))
}

Expand Down Expand Up @@ -1086,11 +1086,11 @@ impl<'sess> SymbolResolver<'sess> {
ResolverErrorKind::MultipleDeclarations,
));
};
if decl.kind.is_err() {
if decl.res.is_err() {
return Ok(decls);
}
let scope = self.scope_of(decl.kind).ok_or_else(|| {
ResolverError::from_path(path, prev_i, ResolverErrorKind::NotAScope(decl.kind))
let scope = self.scope_of(decl.res).ok_or_else(|| {
ResolverError::from_path(path, prev_i, ResolverErrorKind::NotAScope(decl.res))
})?;
decls = scope.resolve(segment).ok_or_else(|| {
ResolverError::from_path(path, prev_i + 1, ResolverErrorKind::Unresolved)
Expand All @@ -1111,7 +1111,7 @@ impl<'sess> SymbolResolver<'sess> {
match declaration {
Res::Item(hir::ItemId::Contract(id)) => Some(&self.contract_scopes[id]),
Res::Namespace(id) => Some(&self.source_scopes[id]),
Res::Builtin(builtin) => self.inner_builtin_scopes[builtin as usize].as_ref(),
Res::Builtin(builtin) => self.builtin_members_scopes[builtin as usize].as_ref(),
_ => None,
}
}
Expand Down Expand Up @@ -1213,14 +1213,14 @@ impl Declarations {
/// Declares `Ident { name, span } => kind` by converting it to
/// `name => Declaration { kind, span }`.
#[inline]
pub(crate) fn declare_kind(
pub(crate) fn declare_res(
&mut self,
sess: &Session,
hir: &hir::Hir<'_>,
name: Ident,
kind: Res,
res: Res,
) -> Result<(), ErrorGuaranteed> {
self.declare(sess, hir, name.name, Declaration { kind, span: name.span })
self.declare(sess, hir, name.name, Declaration { res, span: name.span })
}

pub(crate) fn declare(
Expand Down Expand Up @@ -1270,19 +1270,19 @@ impl Declarations {
}

// https://github.com/ethereum/solidity/blob/de1a017ccb935d149ed6bcbdb730d89883f8ce02/libsolidity/analysis/DeclarationContainer.cpp#L35
if matches!(decl.kind, Item(Function(_) | Event(_))) {
if matches!(decl.res, Item(Function(_) | Event(_))) {
let mut getter = None;
if let Item(Function(id)) = decl.kind {
if let Item(Function(id)) = decl.res {
getter = Some(id);
let f = hir.function(id);
if !f.kind.is_ordinary() {
return Some(declarations[0]);
}
}
let same_kind = |decl2: &Declaration| match decl2.kind {
let same_kind = |decl2: &Declaration| match decl2.res {
Item(Variable(v)) => hir.variable(v).getter == getter,
Item(Function(f)) => hir.function(f).kind.is_ordinary(),
ref k => k.matches(&decl.kind),
ref k => k.matches(&decl.res),
};
declarations.iter().find(|&decl2| !same_kind(decl2)).copied()
} else if declarations == [decl] {
Expand All @@ -1295,7 +1295,7 @@ impl Declarations {

#[derive(Clone, Copy, Debug)]
pub(crate) struct Declaration {
pub(crate) kind: Res,
pub(crate) res: Res,
pub(crate) span: Span,
}

Expand All @@ -1304,14 +1304,14 @@ impl std::ops::Deref for Declaration {

#[inline]
fn deref(&self) -> &Self::Target {
&self.kind
&self.res
}
}

impl PartialEq for Declaration {
#[inline]
fn eq(&self, other: &Self) -> bool {
self.kind == other.kind
self.res == other.res
}
}

Expand All @@ -1329,7 +1329,7 @@ pub(super) fn report_conflict(
let mut err = sess.dcx.err(format!("identifier `{name}` already declared")).span(decl.span);

// If `previous` is coming from an import, show both the import and the real span.
if let Res::Item(item_id) = previous.kind {
if let Res::Item(item_id) = previous.res {
if let Ok(snippet) = sess.source_map().span_to_snippet(previous.span) {
if snippet.starts_with("import") {
err = err.span_note(previous.span, "previous declaration imported here");
Expand Down
13 changes: 8 additions & 5 deletions crates/sema/src/builtins/members.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,15 @@ pub(crate) fn members_of<'gcx>(gcx: Gcx<'gcx>, ty: Ty<'gcx>) -> MemberList<'gcx>
TyKind::Udvt(_ty, _id) => Default::default(),
TyKind::Error(_tys, _id) => Member::of_builtins(gcx, [Builtin::ErrorSelector]),
TyKind::Event(_tys, _id) => Member::of_builtins(gcx, [Builtin::EventSelector]),
TyKind::Module(_id) => {
// TODO: needs symbol resolver
Default::default()
}
TyKind::Module(id) => gcx.symbol_resolver.source_scopes[id]
.declarations
.iter()
.flat_map(|(&name, decls)| {
decls.iter().map(move |decl| Member::new(name, gcx.type_of_res(decl.res)))
})
.collect(),
TyKind::BuiltinModule(builtin) => builtin
.inner()
.members()
.unwrap_or_else(|| panic!("builtin module {builtin:?} has no inner builtins"))
.iter()
.map(|&b| Member::of_builtin(gcx, b))
Expand Down
12 changes: 6 additions & 6 deletions crates/sema/src/builtins/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,16 @@ pub use members::{Member, MemberList};

pub(crate) fn scopes() -> (Declarations, Box<[Option<Declarations>; Builtin::COUNT]>) {
let global = declarations(Builtin::global().iter().copied());
let inner = Box::new(std::array::from_fn(|i| {
Some(declarations(Builtin::from_index(i).unwrap().inner()?.iter().copied()))
let members_map = Box::new(std::array::from_fn(|i| {
Some(declarations(Builtin::from_index(i).unwrap().members()?.iter().copied()))
}));
(global, inner)
(global, members_map)
}

fn declarations(builtins: impl IntoIterator<Item = Builtin>) -> Declarations {
let mut declarations = Declarations::new();
for builtin in builtins {
let decl = Declaration { kind: hir::Res::Builtin(builtin), span: Span::DUMMY };
let decl = Declaration { res: hir::Res::Builtin(builtin), span: Span::DUMMY };
declarations.declarations.entry(builtin.name()).or_default().push(decl);
}
declarations
Expand Down Expand Up @@ -290,8 +290,8 @@ impl Builtin {
builtin_range_slice!(Self::FIRST_GLOBAL, Self::LAST_GLOBAL)
}

/// Returns the inner builtins.
pub fn inner(self) -> Option<&'static [Self]> {
/// Returns the builtin's members.
pub fn members(self) -> Option<&'static [Self]> {
use Builtin::*;
Some(match self {
Block => builtin_range_slice!(Self::FIRST_BLOCK, Self::LAST_BLOCK),
Expand Down
2 changes: 1 addition & 1 deletion crates/sema/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -751,7 +751,7 @@ pub struct Stmt<'hir> {
/// A kind of statement.
#[derive(Debug)]
pub enum StmtKind<'hir> {
// TODO
// TODO: Yul to HIR.
// /// An assembly block, with optional flags: `assembly "evmasm" (...) { ... }`.
// Assembly(StmtAssembly<'hir>),
/// A single-variable declaration statement: `uint256 foo = 42;`.
Expand Down
6 changes: 3 additions & 3 deletions crates/sema/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -409,13 +409,13 @@ impl<'gcx> Gcx<'gcx> {
}
}

#[allow(dead_code)]
fn type_of_res(self, res: hir::Res) -> Ty<'gcx> {
/// Returns the type of the given [`hir::Res`].
pub fn type_of_res(self, res: hir::Res) -> Ty<'gcx> {
match res {
hir::Res::Item(id) => self.type_of_item(id),
hir::Res::Namespace(id) => self.mk_ty(TyKind::Module(id)),
hir::Res::Builtin(builtin) => builtin.ty(self),
hir::Res::Err(error_guaranteed) => self.mk_ty(TyKind::Err(error_guaranteed)),
hir::Res::Err(guar) => self.mk_ty_err(guar),
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/sema/src/typeck/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pub(crate) fn check(gcx: Gcx<'_>) {
/// Checks for definitions that have the same name and parameter types in the given scope.
fn check_duplicate_definitions(gcx: Gcx<'_>, scope: &Declarations) {
let is_duplicate = |a: Declaration, b: Declaration| -> bool {
let (Res::Item(a), Res::Item(b)) = (a.kind, b.kind) else { return false };
let (Res::Item(a), Res::Item(b)) = (a.res, b.res) else { return false };
if !a.matches(&b) {
return false;
}
Expand Down

0 comments on commit e6193d1

Please sign in to comment.