Skip to content

Commit

Permalink
chore: eliminate unsafe operations
Browse files Browse the repository at this point in the history
  • Loading branch information
mtshiba committed May 27, 2023
1 parent 7049faf commit 8e48139
Show file tree
Hide file tree
Showing 17 changed files with 196 additions and 118 deletions.
2 changes: 1 addition & 1 deletion crates/els/completion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ fn load_modules(cfg: ErgConfig, cache: Cache) {
cache.insert("<module>".into(), module_completions());
}
let std_path = erg_pystd_path().display().to_string().replace('\\', "/");
for (path, entry) in shared.py_mod_cache.iter() {
for (path, entry) in shared.py_mod_cache.ref_inner().iter() {
let dir = entry.module.context.local_dir();
let mod_name = path.display().to_string().replace('\\', "/");
let mod_name = mod_name
Expand Down
2 changes: 2 additions & 0 deletions crates/els/rename.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ impl<Checker: BuildRunnable> Server<Checker> {
graph.sort().unwrap();
let self_node = graph.get_node(&path).unwrap();
graph
.ref_inner()
.iter()
.filter(|node| node.id == path || self_node.depends_on(&node.id))
.map(|node| NormalizedUrl::new(Url::from_file_path(&node.id).unwrap()))
Expand All @@ -160,6 +161,7 @@ impl<Checker: BuildRunnable> Server<Checker> {
let graph = &self.get_shared().unwrap().graph;
let path = util::uri_to_path(uri);
graph
.ref_inner()
.iter()
.filter(|node| node.depends_on(&path))
.map(|node| NormalizedUrl::new(Url::from_file_path(&node.id).unwrap()))
Expand Down
2 changes: 1 addition & 1 deletion crates/els/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,7 @@ impl<Checker: BuildRunnable> Server<Checker> {

pub(crate) fn get_builtin_module(&self) -> Option<&Context> {
self.get_shared()
.and_then(|mode| mode.mod_cache.ref_ctx(Path::new("<builtins>")))
.and_then(|mode| mode.mod_cache.raw_ref_ctx(Path::new("<builtins>")))
.map(|mc| &mc.context)
}

Expand Down
5 changes: 4 additions & 1 deletion crates/erg_common/spawn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ where
.spawn(run)
.unwrap();
// Wait for thread to join
child.join().unwrap()
child.join().unwrap_or_else(|err| {
eprintln!("Thread panicked: {err:?}");
std::process::exit(1);
})
} else {
run()
}
Expand Down
2 changes: 1 addition & 1 deletion crates/erg_compiler/context/generalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ impl Generalizer {
/// ```
fn generalize_t(&mut self, free_type: Type, uninit: bool) -> Type {
match free_type {
FreeVar(fv) if fv.is_linked() => self.generalize_t(fv.crack().clone(), uninit),
FreeVar(fv) if fv.is_linked() => self.generalize_t(fv.unsafe_crack().clone(), uninit),
FreeVar(fv) if fv.is_generalized() => Type::FreeVar(fv),
// TODO: Polymorphic generalization
FreeVar(fv) if fv.level().unwrap() > self.level => {
Expand Down
7 changes: 4 additions & 3 deletions crates/erg_compiler/context/initialize/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -772,7 +772,7 @@ impl Context {
);
self.consts.insert(name.clone(), val);
for impl_trait in ctx.super_traits.iter() {
if let Some(impls) = self.trait_impls().get_mut(&impl_trait.qual_name()) {
if let Some(mut impls) = self.trait_impls().get_mut(&impl_trait.qual_name()) {
impls.insert(TraitImpl::new(t.clone(), impl_trait.clone()));
} else {
self.trait_impls().register(
Expand Down Expand Up @@ -848,7 +848,7 @@ impl Context {
}
self.consts.insert(name.clone(), val);
for impl_trait in ctx.super_traits.iter() {
if let Some(impls) = self.trait_impls().get_mut(&impl_trait.qual_name()) {
if let Some(mut impls) = self.trait_impls().get_mut(&impl_trait.qual_name()) {
impls.insert(TraitImpl::new(t.clone(), impl_trait.clone()));
} else {
self.trait_impls().register(
Expand Down Expand Up @@ -912,7 +912,8 @@ impl Context {
}
}
if let ContextKind::GluePatch(tr_impl) = &ctx.kind {
if let Some(impls) = self.trait_impls().get_mut(&tr_impl.sup_trait.qual_name()) {
if let Some(mut impls) = self.trait_impls().get_mut(&tr_impl.sup_trait.qual_name())
{
impls.insert(tr_impl.clone());
} else {
self.trait_impls()
Expand Down
9 changes: 6 additions & 3 deletions crates/erg_compiler/context/inquire.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ impl Context {
return Some(self.get_module().unwrap())
}
self.opt_mod_cache()?
.ref_ctx(path)
.or_else(|| self.opt_py_mod_cache()?.ref_ctx(path))
.raw_ref_ctx(path)
.or_else(|| self.opt_py_mod_cache()?.raw_ref_ctx(path))
.map(|mod_ctx| &mod_ctx.context)
}

Expand Down Expand Up @@ -2463,7 +2463,10 @@ impl Context {
pub(crate) fn get_mod_with_path(&self, path: &Path) -> Option<&Context> {
(self.cfg.input.path() == Some(path)) // module itself
.then_some(self)
.or(self.mod_cache().get(path).map(|ent| &ent.module.context))
.or(self
.mod_cache()
.raw_ref_ctx(path)
.map(|mod_ctx| &mod_ctx.context))
}

// FIXME: 現在の実装だとimportしたモジュールはどこからでも見れる
Expand Down
7 changes: 6 additions & 1 deletion crates/erg_compiler/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -977,7 +977,12 @@ impl Context {
if self.kind != ContextKind::Module || &self.path()[..] != "<builtins>" {
self.shared
.as_ref()
.map(|shared| shared.mod_cache.ref_ctx(Path::new("<builtins>")).unwrap())
.map(|shared| {
shared
.mod_cache
.raw_ref_ctx(Path::new("<builtins>"))
.unwrap()
})
.map(|mod_ctx| &mod_ctx.context)
} else {
None
Expand Down
4 changes: 2 additions & 2 deletions crates/erg_compiler/context/register.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1653,7 +1653,7 @@ impl Context {
self.decls.insert(name.clone(), vi);
self.consts.insert(name.clone(), val);
for impl_trait in ctx.super_traits.iter() {
if let Some(impls) = self.trait_impls().get_mut(&impl_trait.qual_name()) {
if let Some(mut impls) = self.trait_impls().get_mut(&impl_trait.qual_name()) {
impls.insert(TraitImpl::new(t.clone(), impl_trait.clone()));
} else {
self.trait_impls().register(
Expand Down Expand Up @@ -1733,7 +1733,7 @@ impl Context {
self.consts
.insert(name.clone(), ValueObj::Type(TypeObj::Generated(gen)));
for impl_trait in ctx.super_traits.iter() {
if let Some(impls) = self.trait_impls().get_mut(&impl_trait.qual_name()) {
if let Some(mut impls) = self.trait_impls().get_mut(&impl_trait.qual_name()) {
impls.insert(TraitImpl::new(t.clone(), impl_trait.clone()));
} else {
self.trait_impls().register(
Expand Down
2 changes: 1 addition & 1 deletion crates/erg_compiler/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ impl ASTLowerer {
if mode == "eval" {
return;
}
for (referee, value) in self.module.context.index().iter() {
for (referee, value) in self.module.context.index().members().iter() {
let code = referee.code();
let name = code.as_ref().map(|s| &s[..]).unwrap_or("");
let name_is_auto =
Expand Down
2 changes: 1 addition & 1 deletion crates/erg_compiler/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1892,7 +1892,7 @@ impl ASTLowerer {
trait_loc: &impl Locational,
) -> LowerResult<()> {
// TODO: polymorphic trait
if let Some(impls) = self
if let Some(mut impls) = self
.module
.context
.trait_impls()
Expand Down
50 changes: 33 additions & 17 deletions crates/erg_compiler/module/cache.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::borrow::Borrow;
use std::cell::{Ref, RefMut};
use std::fmt;
use std::hash::Hash;
use std::path::PathBuf;
Expand Down Expand Up @@ -173,20 +174,28 @@ impl SharedModuleCache {
self.0.borrow().cache.len()
}

pub fn get<Q: Eq + Hash + ?Sized>(&self, path: &Q) -> Option<&ModuleEntry>
pub fn get<Q: Eq + Hash + ?Sized>(&self, path: &Q) -> Option<Ref<ModuleEntry>>
where
PathBuf: Borrow<Q>,
{
let ref_ = unsafe { self.0.as_ptr().as_ref().unwrap() };
ref_.get(path)
if self.0.borrow().get(path).is_some() {
Some(Ref::map(self.0.borrow(), |cache| cache.get(path).unwrap()))
} else {
None
}
}

pub fn get_mut<Q: Eq + Hash + ?Sized>(&self, path: &Q) -> Option<&mut ModuleEntry>
pub fn get_mut<Q: Eq + Hash + ?Sized>(&self, path: &Q) -> Option<RefMut<ModuleEntry>>
where
PathBuf: Borrow<Q>,
{
let ref_ = unsafe { self.0.as_ptr().as_mut().unwrap() };
ref_.get_mut(path)
if self.0.borrow().get(path).is_some() {
Some(RefMut::map(self.0.borrow_mut(), |cache| {
cache.get_mut(path).unwrap()
}))
} else {
None
}
}

pub fn get_ctx<Q: Eq + Hash + ?Sized>(&self, path: &Q) -> Option<Rc<ModuleContext>>
Expand All @@ -196,7 +205,20 @@ impl SharedModuleCache {
self.0.borrow().get(path).map(|entry| entry.module.clone())
}

pub fn ref_ctx<Q: Eq + Hash + ?Sized>(&self, path: &Q) -> Option<&ModuleContext>
pub fn ref_ctx<Q: Eq + Hash + ?Sized>(&self, path: &Q) -> Option<Ref<ModuleContext>>
where
PathBuf: Borrow<Q>,
{
if self.0.borrow().get(path).is_some() {
Some(Ref::map(self.0.borrow(), |cache| {
cache.get(path).unwrap().module.as_ref()
}))
} else {
None
}
}

pub fn raw_ref_ctx<Q: Eq + Hash + ?Sized>(&self, path: &Q) -> Option<&ModuleContext>
where
PathBuf: Borrow<Q>,
{
Expand All @@ -223,18 +245,13 @@ impl SharedModuleCache {
self.0.borrow().get_similar_name(name)
}

pub fn keys(&self) -> impl Iterator<Item = PathBuf> {
let ref_ = unsafe { self.0.as_ptr().as_ref().unwrap() };
ref_.cache.keys().cloned()
}

pub fn initialize(&self) {
let builtin_path = PathBuf::from("<builtins>");
let Some(builtin) = self.remove(&builtin_path) else {
return;
};
for path in self.keys() {
self.remove(&path);
for path in self.ref_inner().keys() {
self.remove(path);
}
self.register(builtin_path, None, Rc::try_unwrap(builtin.module).unwrap());
}
Expand All @@ -243,8 +260,7 @@ impl SharedModuleCache {
self.0.borrow_mut().rename_path(path, new);
}

pub fn iter(&self) -> impl Iterator<Item = (&PathBuf, &ModuleEntry)> {
let ref_ = unsafe { self.0.as_ptr().as_ref().unwrap() };
ref_.iter()
pub fn ref_inner(&self) -> Ref<Dict<PathBuf, ModuleEntry>> {
Ref::map(self.0.borrow(), |mc| &mc.cache)
}
}
19 changes: 11 additions & 8 deletions crates/erg_compiler/module/graph.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::cell::Ref;
use std::fmt;
use std::path::{Path, PathBuf};

Expand Down Expand Up @@ -109,10 +110,14 @@ impl SharedModuleGraph {
Self(Shared::new(ModuleGraph::new()))
}

/// SAFETY: don't hold this reference before sorting
pub fn get_node(&self, path: &Path) -> Option<&Node<PathBuf, ()>> {
let ref_graph = unsafe { self.0.as_ptr().as_ref().unwrap() };
ref_graph.get_node(path)
pub fn get_node(&self, path: &Path) -> Option<Ref<Node<PathBuf, ()>>> {
if self.0.borrow().get_node(path).is_some() {
Some(Ref::map(self.0.borrow(), |graph| {
graph.get_node(path).unwrap()
}))
} else {
None
}
}

pub fn add_node_if_none(&self, path: &Path) {
Expand All @@ -123,10 +128,8 @@ impl SharedModuleGraph {
self.0.borrow_mut().inc_ref(referrer, depends_on);
}

/// SAFETY: don't hold this iterator before sorting
pub fn iter(&self) -> impl Iterator<Item = &Node<PathBuf, ()>> {
let ref_graph = unsafe { self.0.as_ptr().as_ref().unwrap() };
ref_graph.iter()
pub fn ref_inner(&self) -> Ref<ModuleGraph> {
self.0.borrow()
}

pub fn remove(&self, path: &Path) {
Expand Down
26 changes: 17 additions & 9 deletions crates/erg_compiler/module/impls.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::borrow::Borrow;
use std::cell::{Ref, RefMut};
use std::fmt;
use std::hash::Hash;

Expand Down Expand Up @@ -76,20 +77,28 @@ impl SharedTraitImpls {
Self(Shared::new(TraitImpls::new()))
}

pub fn get<Q: Eq + Hash + ?Sized>(&self, path: &Q) -> Option<&Set<TraitImpl>>
pub fn get<Q: Eq + Hash + ?Sized>(&self, path: &Q) -> Option<Ref<Set<TraitImpl>>>
where
Str: Borrow<Q>,
{
let ref_ = unsafe { self.0.as_ptr().as_ref().unwrap() };
ref_.get(path)
if self.0.borrow().get(path).is_some() {
Some(Ref::map(self.0.borrow(), |tis| tis.get(path).unwrap()))
} else {
None
}
}

pub fn get_mut<Q: Eq + Hash + ?Sized>(&self, path: &Q) -> Option<&mut Set<TraitImpl>>
pub fn get_mut<Q: Eq + Hash + ?Sized>(&self, path: &Q) -> Option<RefMut<Set<TraitImpl>>>
where
Str: Borrow<Q>,
{
let ref_ = unsafe { self.0.as_ptr().as_mut().unwrap() };
ref_.get_mut(path)
if self.0.borrow().get(path).is_some() {
Some(RefMut::map(self.0.borrow_mut(), |tis| {
tis.get_mut(path).unwrap()
}))
} else {
None
}
}

pub fn register(&self, name: Str, impls: Set<TraitImpl>) {
Expand All @@ -103,9 +112,8 @@ impl SharedTraitImpls {
self.0.borrow_mut().remove(path)
}

pub fn keys(&self) -> impl Iterator<Item = Str> {
let ref_ = unsafe { self.0.as_ptr().as_ref().unwrap() };
ref_.cache.keys().cloned()
pub fn ref_inner(&self) -> Ref<Dict<Str, Set<TraitImpl>>> {
Ref::map(self.0.borrow(), |tis| &tis.cache)
}

pub fn initialize(&self) {
Expand Down
39 changes: 27 additions & 12 deletions crates/erg_compiler/module/index.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::cell::Ref;
use std::collections::hash_map::{Iter, Keys, Values};
use std::fmt;
use std::path::Path;
Expand All @@ -9,6 +10,22 @@ use erg_common::shared::Shared;

use crate::varinfo::{AbsLocation, VarInfo};

pub struct Members<'a>(Ref<'a, Dict<AbsLocation, ModuleIndexValue>>);

impl<'a> Members<'a> {
pub fn iter(&self) -> Iter<AbsLocation, ModuleIndexValue> {
self.0.iter()
}

pub fn keys(&self) -> Keys<AbsLocation, ModuleIndexValue> {
self.0.keys()
}

pub fn values(&self) -> Values<AbsLocation, ModuleIndexValue> {
self.0.values()
}
}

#[derive(Debug, Clone, Default)]
pub struct ModuleIndexValue {
pub vi: VarInfo,
Expand Down Expand Up @@ -101,20 +118,18 @@ impl SharedModuleIndex {
self.0.borrow_mut().register(vi);
}

pub fn get_refs(&self, referee: &AbsLocation) -> Option<&ModuleIndexValue> {
unsafe { self.0.as_ptr().as_ref().unwrap().get_refs(referee) }
}

pub fn referees(&self) -> Keys<AbsLocation, ModuleIndexValue> {
unsafe { self.0.as_ptr().as_ref().unwrap().members.keys() }
}

pub fn referrers(&self) -> Values<AbsLocation, ModuleIndexValue> {
unsafe { self.0.as_ptr().as_ref().unwrap().members.values() }
pub fn get_refs(&self, referee: &AbsLocation) -> Option<Ref<ModuleIndexValue>> {
if self.0.borrow().get_refs(referee).is_some() {
Some(Ref::map(self.0.borrow(), |index| {
index.get_refs(referee).unwrap()
}))
} else {
None
}
}

pub fn iter(&self) -> Iter<AbsLocation, ModuleIndexValue> {
unsafe { self.0.as_ptr().as_ref().unwrap().members.iter() }
pub fn members(&self) -> Members {
Members(Ref::map(self.0.borrow(), |mi| &mi.members))
}

pub fn initialize(&self) {
Expand Down
Loading

0 comments on commit 8e48139

Please sign in to comment.