Repository: gluon-lang/gluon
Pull request #708 (2019-04-08 19:17:29)

refactor: Limit ModType pollution


Patch:
diff --git a/base/src/types/mod.rs b/base/src/types/mod.rs
index ca6f847c..4ee2d3ee 100644
--- a/base/src/types/mod.rs
+++ b/base/src/types/mod.rs
@@ -74,7 +74,7 @@ macro_rules! forward_eq_hash {
 pub trait TypeEnv: KindEnv {
     type Type;
     /// Returns the type of the value bound at `id`
-    fn find_type(&self, id: &SymbolRef) -> Option<ModType<&Self::Type>>;
+    fn find_type(&self, id: &SymbolRef) -> Option<&Self::Type>;
 
     /// Returns information about the type `id`
     fn find_type_info(&self, id: &SymbolRef) -> Option<&Alias<Symbol, Self::Type>>;
@@ -83,7 +83,7 @@ pub trait TypeEnv: KindEnv {
 impl<'a, T: ?Sized + TypeEnv> TypeEnv for &'a T {
     type Type = T::Type;
 
-    fn find_type(&self, id: &SymbolRef) -> Option<ModType<&Self::Type>> {
+    fn find_type(&self, id: &SymbolRef) -> Option<&Self::Type> {
         (**self).find_type(id)
     }
 
@@ -95,7 +95,7 @@ impl<'a, T: ?Sized + TypeEnv> TypeEnv for &'a T {
 impl TypeEnv for EmptyEnv<Symbol> {
     type Type = ArcType;
 
-    fn find_type(&self, _id: &SymbolRef) -> Option<ModType<&ArcType>> {
+    fn find_type(&self, _id: &SymbolRef) -> Option<&ArcType> {
         None
     }
 
@@ -3759,100 +3759,3 @@ where
         Type::EmptyRow => interner.empty_row(),
     }
 }
-
-#[derive(Clone, Copy, Debug, Eq, PartialEq)]
-pub enum TypeModifier {
-    Wobbly,
-    Rigid,
-}
-
-impl Default for TypeModifier {
-    fn default() -> Self {
-        TypeModifier::Wobbly
-    }
-}
-
-impl ops::BitOr for TypeModifier {
-    type Output = Self;
-
-    fn bitor(mut self, typ: Self) -> Self {
-        self |= typ;
-        self
-    }
-}
-impl ops::BitOrAssign for TypeModifier {
-    fn bitor_assign(&mut self, typ: Self) {
-        match (*self, typ) {
-            (TypeModifier::Rigid, TypeModifier::Rigid) => (),
-            _ => *self = TypeModifier::Wobbly,
-        }
-    }
-}
-
-pub type ModTypeRef<'a> = ModType<&'a ArcType>;
-
-#[derive(Clone, Copy, Debug)]
-pub struct ModType<T = ArcType> {
-    pub modifier: TypeModifier,
-    pub concrete: T,
-}
-
-impl<T> fmt::Display for ModType<T>
-where
-    T: fmt::Display,
-{
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        self.concrete.fmt(f)
-    }
-}
-
-impl<T> Deref for ModType<T> {
-    type Target = T;
-
-    fn deref(&self) -> &Self::Target {
-        &self.concrete
-    }
-}
-
-impl<T> DerefMut for ModType<T> {
-    fn deref_mut(&mut self) -> &mut Self::Target {
-        &mut self.concrete
-    }
-}
-
-impl<T> ops::BitOrAssign for ModType<T> {
-    fn bitor_assign(&mut self, typ: Self) {
-        self.modifier |= typ.modifier;
-        self.concrete = typ.concrete;
-    }
-}
-
-impl<T> ModType<T> {
-    pub fn new(modifier: TypeModifier, typ: T) -> Self {
-        ModType {
-            modifier,
-            concrete: typ,
-        }
-    }
-
-    pub fn rigid(typ: T) -> Self {
-        Self::new(TypeModifier::Rigid, typ)
-    }
-
-    pub fn wobbly(typ: T) -> Self {
-        Self::new(TypeModifier::Wobbly, typ)
-    }
-
-    pub fn as_ref(&self) -> ModType<&T> {
-        ModType::new(self.modifier, &self.concrete)
-    }
-}
-
-impl<'a, T> ModType<&'a T>
-where
-    T: Clone,
-{
-    pub fn to_owned(&self) -> ModType<T> {
-        ModType::new(self.modifier, self.concrete.clone())
-    }
-}
diff --git a/check/src/lib.rs b/check/src/lib.rs
index 5b128616..5d65edfd 100644
--- a/check/src/lib.rs
+++ b/check/src/lib.rs
@@ -89,7 +89,7 @@ mod tests {
     use crate::base::{
         kind::{ArcKind, KindEnv},
         symbol::{Symbol, SymbolModule, SymbolRef, Symbols},
-        types::{Alias, ModTypeRef, TypeEnv},
+        types::{Alias, TypeEnv},
     };
 
     pub struct MockEnv;
@@ -102,7 +102,7 @@ mod tests {
 
     impl TypeEnv for MockEnv {
         type Type = RcType;
-        fn find_type(&self, _id: &SymbolRef) -> Option<ModTypeRef> {
+        fn find_type(&self, _id: &SymbolRef) -> Option<&ArcType> {
             None
         }
         fn find_type_info(&self, _id: &SymbolRef) -> Option<&Alias<Symbol, RcType>> {
diff --git a/check/src/typecheck.rs b/check/src/typecheck.rs
index c0932525..daf756a7 100644
--- a/check/src/typecheck.rs
+++ b/check/src/typecheck.rs
@@ -25,8 +25,8 @@ use crate::base::{
     scoped_map::{self, ScopedMap},
     symbol::{Symbol, SymbolModule, SymbolRef, Symbols},
     types::{
-        self, Alias, AliasRef, AppVec, ArcType, ArgType, Field, Flags, Generic, ModType,
-        ModTypeRef, PrimitiveEnv, Type, TypeCache, TypeContext, TypeEnv, TypeExt, TypeModifier,
+        self, Alias, AliasRef, AppVec, ArcType, ArgType, Field, Flags, Generic, PrimitiveEnv, Type,
+        TypeCache, TypeContext, TypeEnv, TypeExt,
     },
 };
 
@@ -38,12 +38,16 @@ use crate::{
     unify, unify_type, TypecheckEnv,
 };
 
-use self::generalize::TypeGeneralizer;
+use self::{
+    generalize::TypeGeneralizer,
+    mod_type::{ModType, ModTypeRef, TypeModifier},
+};
 
 pub use self::error::{Help, HelpError, SpannedTypeError, TypeError};
 
 mod error;
 mod generalize;
+mod mod_type;
 
 pub(crate) type TcResult<T> = Result<T, TypeError<Symbol, RcType<Symbol>>>;
 
@@ -86,10 +90,10 @@ impl<'a> KindEnv for Environment<'a> {
 impl<'a> TypeEnv for Environment<'a> {
     type Type = RcType;
 
-    fn find_type(&self, id: &SymbolRef) -> Option<ModTypeRef> {
+    fn find_type(&self, id: &SymbolRef) -> Option<&ArcType> {
         self.stack
             .get(id)
-            .map(|bind| bind.typ.as_ref())
+            .map(|bind| &bind.typ.concrete)
             .or_else(|| self.environment.find_type(id))
     }
 
@@ -113,6 +117,15 @@ impl<'a> MetadataEnv for Environment<'a> {
     }
 }
 
+impl Environment<'_> {
+    fn find_mod_type(&self, id: &SymbolRef) -> Option<ModTypeRef> {
+        self.stack
+            .get(id)
+            .map(|bind| bind.typ.as_ref())
+            .or_else(|| self.environment.find_type(id).map(ModType::rigid))
+    }
+}
+
 /// Type returned from the main typecheck function to make sure that nested `type` and `let`
 /// expressions dont overflow the stack
 enum TailCall {
@@ -204,7 +217,7 @@ impl<'a> Typecheck<'a> {
     }
 
     fn find(&mut self, id: &Symbol) -> TcResult<ModType> {
-        match self.environment.find_type(id).map(|t| t.to_owned()) {
+        match self.environment.find_mod_type(id).map(|t| t.to_owned()) {
             Some(typ) => {
                 self.named_variables.clear();
                 debug!("Find {} : {}", self.symbols.string(id), typ);
@@ -3143,7 +3156,7 @@ pub fn translate_projected_type(
             }
             None => Some(
                 env.find_type(&symbol)
-                    .map(|t| t.concrete.clone())
+                    .cloned()
                     .or_else(|| {
                         env.find_type_info(&symbol)
                             .map(|alias| alias.typ(interner).into_owned())
diff --git a/check/src/typecheck/mod_type.rs b/check/src/typecheck/mod_type.rs
new file mode 100644
index 00000000..aa9b5e4a
--- /dev/null
+++ b/check/src/typecheck/mod_type.rs
@@ -0,0 +1,100 @@
+use std::{fmt, ops};
+
+use base::types::ArcType;
+
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+pub enum TypeModifier {
+    Wobbly,
+    Rigid,
+}
+
+impl Default for TypeModifier {
+    fn default() -> Self {
+        TypeModifier::Wobbly
+    }
+}
+
+impl ops::BitOr for TypeModifier {
+    type Output = Self;
+
+    fn bitor(mut self, typ: Self) -> Self {
+        self |= typ;
+        self
+    }
+}
+impl ops::BitOrAssign for TypeModifier {
+    fn bitor_assign(&mut self, typ: Self) {
+        match (*self, typ) {
+            (TypeModifier::Rigid, TypeModifier::Rigid) => (),
+            _ => *self = TypeModifier::Wobbly,
+        }
+    }
+}
+
+pub type ModTypeRef<'a> = ModType<&'a ArcType>;
+
+#[derive(Clone, Copy, Debug)]
+pub struct ModType<T = ArcType> {
+    pub modifier: TypeModifier,
+    pub concrete: T,
+}
+
+impl<T> fmt::Display for ModType<T>
+where
+    T: fmt::Display,
+{
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        self.concrete.fmt(f)
+    }
+}
+
+impl<T> ops::Deref for ModType<T> {
+    type Target = T;
+
+    fn deref(&self) -> &Self::Target {
+        &self.concrete
+    }
+}
+
+impl<T> ops::DerefMut for ModType<T> {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.concrete
+    }
+}
+
+impl<T> ops::BitOrAssign for ModType<T> {
+    fn bitor_assign(&mut self, typ: Self) {
+        self.modifier |= typ.modifier;
+        self.concrete = typ.concrete;
+    }
+}
+
+impl<T> ModType<T> {
+    pub fn new(modifier: TypeModifier, typ: T) -> Self {
+        ModType {
+            modifier,
+            concrete: typ,
+        }
+    }
+
+    pub fn rigid(typ: T) -> Self {
+        Self::new(TypeModifier::Rigid, typ)
+    }
+
+    pub fn wobbly(typ: T) -> Self {
+        Self::new(TypeModifier::Wobbly, typ)
+    }
+
+    pub fn as_ref(&self) -> ModType<&T> {
+        ModType::new(self.modifier, &self.concrete)
+    }
+}
+
+impl<'a, T> ModType<&'a T>
+where
+    T: Clone,
+{
+    pub fn to_owned(&self) -> ModType<T> {
+        ModType::new(self.modifier, self.concrete.clone())
+    }
+}
diff --git a/completion/src/lib.rs b/completion/src/lib.rs
index c7a94ba4..0f24945d 100644
--- a/completion/src/lib.rs
+++ b/completion/src/lib.rs
@@ -1471,7 +1471,7 @@ impl SuggestionQuery {
                         name,
                         typ: Either::Right(
                             env.find_type(SymbolRef::new(module))
-                                .map(|t| t.concrete.clone())
+                                .cloned()
                                 .unwrap_or_else(Type::hole),
                         ),
                     }
diff --git a/vm/src/compiler.rs b/vm/src/compiler.rs
index 360b4863..c6709d2c 100644
--- a/vm/src/compiler.rs
+++ b/vm/src/compiler.rs
@@ -8,7 +8,7 @@ use crate::base::{
     scoped_map::ScopedMap,
     source::Source,
     symbol::{Symbol, SymbolModule, SymbolRef},
-    types::{Alias, ArcType, BuiltinType, ModTypeRef, NullInterner, Type, TypeEnv, TypeExt},
+    types::{Alias, ArcType, BuiltinType, NullInterner, Type, TypeEnv, TypeExt},
 };
 
 use crate::{
@@ -451,7 +451,7 @@ impl<'a> KindEnv for Compiler<'a> {
 impl<'a> TypeEnv for Compiler<'a> {
     type Type = ArcType;
 
-    fn find_type(&self, _id: &SymbolRef) -> Option<ModTypeRef> {
+    fn find_type(&self, _id: &SymbolRef) -> Option<&ArcType> {
         None
     }
 
diff --git a/vm/src/core/interpreter.rs b/vm/src/core/interpreter.rs
index 748bc510..cf828148 100644
--- a/vm/src/core/interpreter.rs
+++ b/vm/src/core/interpreter.rs
@@ -7,7 +7,7 @@ use crate::base::{
     merge::merge_iter,
     scoped_map::ScopedMap,
     symbol::{Symbol, SymbolRef},
-    types::{Alias, ArcType, ModTypeRef, TypeEnv, TypeExt},
+    types::{Alias, ArcType, TypeEnv, TypeExt},
 };
 
 use crate::{
@@ -293,7 +293,7 @@ impl<'a, 'e> KindEnv for Compiler<'a, 'e> {
 impl<'a, 'e> TypeEnv for Compiler<'a, 'e> {
     type Type = ArcType;
 
-    fn find_type(&self, _id: &SymbolRef) -> Option<ModTypeRef> {
+    fn find_type(&self, _id: &SymbolRef) -> Option<&ArcType> {
         None
     }
 
diff --git a/vm/src/types.rs b/vm/src/types.rs
index e9d1e6fb..45131389 100644
--- a/vm/src/types.rs
+++ b/vm/src/types.rs
@@ -2,7 +2,7 @@ use crate::base::{
     fnv::FnvMap,
     kind::{ArcKind, Kind, KindEnv},
     symbol::{Symbol, SymbolRef},
-    types::{Alias, ArcType, ModType, ModTypeRef, Type, TypeEnv, TypeExt},
+    types::{Alias, ArcType, Type, TypeEnv, TypeExt},
 };
 
 pub use self::Instruction::*;
@@ -207,7 +207,7 @@ impl KindEnv for TypeInfos {
 impl TypeEnv for TypeInfos {
     type Type = ArcType;
 
-    fn find_type(&self, id: &SymbolRef) -> Option<ModTypeRef> {
+    fn find_type(&self, id: &SymbolRef) -> Option<&ArcType> {
         let id = id.definition_name();
         self.id_to_type
             .iter()
@@ -217,7 +217,6 @@ impl TypeEnv for TypeInfos {
             })
             .next()
             .map(|field| &field.typ)
-            .map(ModType::rigid)
     }
 
     fn find_type_info(&self, id: &SymbolRef) -> Option<&Alias<Symbol, ArcType>> {
diff --git a/vm/src/value.rs b/vm/src/value.rs
index a5370bf5..dbd3a13f 100644
--- a/vm/src/value.rs
+++ b/vm/src/value.rs
@@ -1610,7 +1610,7 @@ mod tests {
     use crate::base::{
         kind::{ArcKind, KindEnv},
         symbol::{Symbol, SymbolRef},
-        types::{Alias, ArcType, Field, ModTypeRef, Type, TypeEnv},
+        types::{Alias, ArcType, Field, Type, TypeEnv},
     };
 
     struct MockEnv(Option<Alias<Symbol, ArcType>>);
@@ -1624,7 +1624,7 @@ mod tests {
     impl TypeEnv for MockEnv {
         type Type = ArcType;
 
-        fn find_type(&self, _id: &SymbolRef) -> Option<ModTypeRef> {
+        fn find_type(&self, _id: &SymbolRef) -> Option<&ArcType> {
             None
         }
 
diff --git a/vm/src/vm.rs b/vm/src/vm.rs
index ae35f19e..29b8fdc8 100644
--- a/vm/src/vm.rs
+++ b/vm/src/vm.rs
@@ -14,8 +14,8 @@ use crate::base::{
     metadata::{Metadata, MetadataEnv},
     symbol::{Name, Symbol, SymbolRef},
     types::{
-        Alias, AliasData, AppVec, ArcType, Generic, ModType, ModTypeRef, NullInterner,
-        PrimitiveEnv, Type, TypeCache, TypeEnv, TypeExt,
+        Alias, AliasData, AppVec, ArcType, Generic, NullInterner, PrimitiveEnv, Type, TypeCache,
+        TypeEnv, TypeExt,
     },
     DebugLevel,
 };
@@ -229,7 +229,7 @@ impl KindEnv for VmEnv {
 impl TypeEnv for VmEnv {
     type Type = ArcType;
 
-    fn find_type(&self, id: &SymbolRef) -> Option<ModTypeRef> {
+    fn find_type(&self, id: &SymbolRef) -> Option<&ArcType> {
         self.globals
             .get(id.definition_name())
             .map(|g| &g.typ)
@@ -247,7 +247,6 @@ impl TypeEnv for VmEnv {
                     .next()
                     .map(|ctor| ctor)
             })
-            .map(ModType::rigid)
     }
 
     fn find_type_info(&self, id: &SymbolRef) -> Option<&Alias<Symbol, ArcType>> {


Test patch:
diff --git a/base/tests/types.rs b/base/tests/types.rs
index e62b21ea..cd99c29f 100644
--- a/base/tests/types.rs
+++ b/base/tests/types.rs
@@ -347,7 +347,7 @@ impl KindEnv for MockEnv {
 impl TypeEnv for MockEnv {
     type Type = ArcType;
 
-    fn find_type(&self, _id: &SymbolRef) -> Option<ModTypeRef> {
+    fn find_type(&self, _id: &SymbolRef) -> Option<&ArcType> {
         None
     }
 
diff --git a/check/tests/support/mod.rs b/check/tests/support/mod.rs
index 6903d58d..295aeca3 100644
--- a/check/tests/support/mod.rs
+++ b/check/tests/support/mod.rs
@@ -14,10 +14,7 @@ use self::{
         metadata::{Metadata, MetadataEnv},
         pos::{BytePos, Spanned},
         symbol::{Symbol, SymbolModule, SymbolRef, Symbols},
-        types::{
-            self, Alias, ArcType, Field, Generic, ModType, ModTypeRef, PrimitiveEnv, Type,
-            TypeCache, TypeEnv,
-        },
+        types::{self, Alias, ArcType, Field, Generic, PrimitiveEnv, Type, TypeCache, TypeEnv},
     },
     check::{
         metadata, rename,
@@ -125,9 +122,9 @@ impl KindEnv for MockEnv {
 
 impl TypeEnv for MockEnv {
     type Type = ArcType;
-    fn find_type(&self, id: &SymbolRef) -> Option<ModTypeRef> {
+    fn find_type(&self, id: &SymbolRef) -> Option<&ArcType> {
         match id.definition_name() {
-            "False" | "True" => Some(ModType::rigid(&self.bool.as_type())),
+            "False" | "True" => Some(&self.bool.as_type()),
             _ => None,
         }
     }
diff --git a/completion/tests/support/mod.rs b/completion/tests/support/mod.rs
index 5f587df8..8602ffce 100644
--- a/completion/tests/support/mod.rs
+++ b/completion/tests/support/mod.rs
@@ -9,9 +9,7 @@ use crate::base::{
     metadata::{Metadata, MetadataEnv},
     pos::BytePos,
     symbol::{Symbol, SymbolModule, SymbolRef, Symbols},
-    types::{
-        self, Alias, ArcType, Generic, ModType, ModTypeRef, PrimitiveEnv, Type, TypeCache, TypeEnv,
-    },
+    types::{self, Alias, ArcType, Generic, PrimitiveEnv, Type, TypeCache, TypeEnv},
 };
 
 use crate::check::{
@@ -89,11 +87,11 @@ impl KindEnv for MockEnv {
 impl TypeEnv for MockEnv {
     type Type = ArcType;
 
-    fn find_type(&self, id: &SymbolRef) -> Option<ModTypeRef> {
+    fn find_type(&self, id: &SymbolRef) -> Option<&ArcType> {
         match id.definition_name() {
-            "False" | "True" => Some(ModType::rigid(self.bool.as_type())),
+            "False" | "True" => Some(self.bool.as_type()),
             // Just need a dummy type that is not `Type::hole` to verify that lookups work
-            "std.prelude" => Some(ModType::rigid(&self.int)),
+            "std.prelude" => Some(&self.int),
             _ => None,
         }
     }