diff --git a/libflux/flux-core/src/semantic/infer.rs b/libflux/flux-core/src/semantic/infer.rs index 507afc900c..edf105c5d9 100644 --- a/libflux/flux-core/src/semantic/infer.rs +++ b/libflux/flux-core/src/semantic/infer.rs @@ -158,7 +158,7 @@ pub fn constrain( ) -> Result<(), Located> { log::debug!("Constraint::Kind {:?}: {} => {}", loc.source, exp, act); act.apply_cow(sub) - .constrain(exp, sub.cons()) + .constrain(exp, sub) .map_err(|error| Located { location: loc.clone(), error, @@ -182,6 +182,28 @@ pub fn equal( }) } +pub fn subsume( + exp: &MonoType, + act: &MonoType, + loc: &SourceLocation, + sub: &mut Substitution, +) -> Result>> { + log::debug!( + "Constraint::Subsume {:?}: {} <===> {}", + loc.source, + exp, + act + ); + exp.try_subsume(act, sub).map_err(|error| { + log::debug!("Unify error: {} <=> {} : {}", exp, act, error); + + Located { + location: loc.clone(), + error, + } + }) +} + /// Generalizes `t` without modifying the substitution. pub(crate) fn temporary_generalize( env: &Environment, @@ -248,7 +270,7 @@ pub(crate) fn temporary_generalize( pub fn generalize(env: &Environment, sub: &mut Substitution, t: MonoType) -> PolyType { struct Generalize<'a> { env_free_vars: Vec, - sub: &'a Substitution, + sub: &'a mut Substitution, vars: Vec<(Tvar, Tvar)>, } diff --git a/libflux/flux-core/src/semantic/nodes.rs b/libflux/flux-core/src/semantic/nodes.rs index 011316830d..8577118a99 100644 --- a/libflux/flux-core/src/semantic/nodes.rs +++ b/libflux/flux-core/src/semantic/nodes.rs @@ -167,6 +167,51 @@ impl InferState<'_, '_> { } } + fn subsume(&mut self, exp: &MonoType, act: &MonoType, loc: &ast::SourceLocation) -> MonoType { + match infer::subsume(exp, act, loc, self.sub) { + Ok(typ) => typ, + Err(err) => { + self.errors + .extend(err.error.into_iter().map(|error| Located { + location: loc.clone(), + error: error.into(), + })); + MonoType::Error + } + } + } + + fn subsume_function( + &mut self, + call_expr: &CallExpr, + exp: &Function, + act: Function<(MonoType, &ast::SourceLocation)>, + ) { + log::debug!( + "Subsume {:?}: {} <===> {}", + call_expr.callee.loc().source, + exp, + act.clone().map(|(typ, _)| typ), + ); + if let Err(err) = exp.try_subsume_with( + &act, + self.sub, + |typ| (typ.clone(), call_expr.callee.loc()), + |error| Located { + location: call_expr.loc.clone(), + error, + }, + ) { + log::debug!( + "Unify error: {} <=> {} : {}", + exp, + act.clone().map(|(typ, _)| typ), + err + ); + self.errors.extend(err.into_iter().map(Error::from)); + } + } + fn error(&mut self, loc: ast::SourceLocation, error: ErrorKind) { self.errors.push(located(loc, error)); } @@ -1019,7 +1064,7 @@ impl FunctionExpr { infer.solve(&ncons); - infer.equal(&exp, &default_func, &self.loc); + infer.subsume(&exp, &default_func, &self.loc); Ok(()) } @@ -1292,18 +1337,25 @@ impl CallExpr { self.callee.infer(infer)?; let mut req = MonoTypeMap::new(); let mut pipe = None; + for arg in &mut self.arguments { + arg.value.infer(infer)?; + } + for Property { - key: ref mut id, - value: ref mut expr, + key: id, + value: expr, .. - } in &mut self.arguments + } in &self.arguments { - expr.infer(infer)?; // Every argument is required in a function call. req.insert(id.name.to_string(), (expr.type_of(), expr.loc())); } - if let Some(ref mut p) = &mut self.pipe { + + if let Some(p) = &mut self.pipe { p.infer(infer)?; + } + + if let Some(p) = &self.pipe { pipe = Some(types::Property { k: "<-".to_string(), v: (p.type_of(), p.loc()), @@ -1319,23 +1371,7 @@ impl CallExpr { match &*self.callee.type_of().apply_cow(infer.sub) { MonoType::Fun(func) => { - if let Err(err) = func.try_subsume_with( - &act, - infer.sub, - |typ| (typ.clone(), self.callee.loc()), - |error| Located { - location: self.loc.clone(), - error, - }, - ) { - log::debug!( - "Unify error: {} <=> {} : {}", - func, - act.map(|(typ, _)| typ), - err - ); - infer.errors.extend(err.into_iter().map(Error::from)); - } + infer.subsume_function(self, func, act); } callee => { let act = act.map(|(typ, _)| typ); diff --git a/libflux/flux-core/src/semantic/sub.rs b/libflux/flux-core/src/semantic/sub.rs index ec85d967d2..2ca73e0ee8 100644 --- a/libflux/flux-core/src/semantic/sub.rs +++ b/libflux/flux-core/src/semantic/sub.rs @@ -3,7 +3,7 @@ use std::{borrow::Cow, cell::RefCell, collections::BTreeMap, fmt, iter::FusedIte use crate::semantic::{ fresh::Fresher, - types::{union, Error, MonoType, PolyType, SubstitutionMap, Tvar, TvarKinds}, + types::{union, Error, Kind, MonoType, PolyType, SubstitutionMap, Tvar, TvarKinds}, }; use ena::unify::UnifyKey; @@ -62,7 +62,7 @@ type UnificationTable = ena::unify::InPlaceUnificationTable; impl From for Substitution { /// Derive a substitution from a hash map. fn from(values: SubstitutionMap) -> Substitution { - let sub = Substitution::default(); + let mut sub = Substitution::default(); for (var, typ) in values { // Create any variables referenced in the input map while var.0 >= sub.table.borrow().len() as u64 { @@ -75,6 +75,11 @@ impl From for Substitution { } impl Substitution { + /// Return a new empty substitution. + pub fn new() -> Substitution { + Substitution::default() + } + /// Return a new empty substitution. pub fn empty() -> Substitution { Substitution::default() @@ -107,6 +112,25 @@ impl Substitution { self.cons.get_mut() } + /// Returns the real type or the root variable of `typ` if it is an variable. + /// Returns `typ` itself if it isn't a variable + pub(crate) fn real<'a>(&self, typ: &'a MonoType) -> Cow<'a, MonoType> { + match *typ { + MonoType::Var(var) => self + .try_apply(var) + .map(Cow::Owned) + .unwrap_or_else(|| Cow::Borrowed(typ)), + _ => Cow::Borrowed(typ), + } + } + + pub(crate) fn satisfies(&self, v: Tvar, kind: Kind) -> bool { + self.cons + .borrow() + .get(&v) + .map_or(false, |kinds| kinds.contains(&kind)) + } + /// Apply a substitution to a type variable. pub fn apply(&self, tv: Tvar) -> MonoType { self.try_apply(tv).unwrap_or(MonoType::Var(tv)) @@ -143,22 +167,21 @@ impl Substitution { /// Unifies as a `Tvar` and a `MonoType`, recording the result in the substitution for later /// lookup - pub fn union_type(&self, var: Tvar, typ: MonoType) -> Result<(), Error> { + pub fn union_type(&mut self, var: Tvar, typ: MonoType) -> Result<(), Error> { match typ { MonoType::Var(r) => self.union(var, r), _ => { self.table.borrow_mut().union_value(var, Some(typ.clone())); - let mut cons = self.cons.borrow_mut(); - if let Some(kinds) = cons.remove(&var) { + if let Some(kinds) = self.cons().remove(&var) { for kind in &kinds { // The monotype that is being unified with the // tvar must be constrained with the same kinds // as that of the tvar. - typ.clone().constrain(*kind, &mut cons)?; + typ.clone().constrain(*kind, self)?; } if matches!(typ, MonoType::BoundVar(_)) { - cons.insert(var, kinds); + self.cons().insert(var, kinds); } } } diff --git a/libflux/flux-core/src/semantic/tests/labels.rs b/libflux/flux-core/src/semantic/tests/labels.rs index 79bdda038d..e77832ab3d 100644 --- a/libflux/flux-core/src/semantic/tests/labels.rs +++ b/libflux/flux-core/src/semantic/tests/labels.rs @@ -274,3 +274,58 @@ fn optional_label_undefined() { "#]], } } + +#[test] +fn default_arguments_do_not_try_to_treat_literals_as_strings_when_they_must_be_a_label() { + test_infer! { + config: AnalyzerConfig{ + features: vec![Feature::LabelPolymorphism], + ..AnalyzerConfig::default() + }, + env: map![ + "max" => r#"(<-tables: stream[{ A with L: B }], ?column: L) => stream[{ A with L: B }] + where A: Record, + B: Comparable, + L: Label"#, + ], + src: r#" + f = ( + column="_value", + tables=<-, + ) => + tables + // `column` would be treated as `string` instead of a label when checking the + // default arguments + |> max(column: column) + "#, + exp: map![ + "f" => r#"(<-tables:stream[{C with A:B}], ?column:A) => stream[{C with A:B}] where A: Label, B: Comparable, C: Record"#, + ], + } +} + +#[test] +fn constraints_propagate_fully() { + test_infer! { + config: AnalyzerConfig{ + features: vec![Feature::LabelPolymorphism], + ..AnalyzerConfig::default() + }, + env: map![ + "aggregateWindow" => r#"(fn:(<-:stream[B], column:C) => stream[D], ?column:C) => stream[E]"#, + "max" => r#"(<-tables: stream[{ A with L: B }], ?column: L) => stream[{ A with L: B }] + where A: Record, + B: Comparable, + L: Label"#, + ], + src: r#" + x = aggregateWindow( + fn: max, + column: "_value", + ) + "#, + exp: map![ + "x" => "stream[E]", + ], + } +} diff --git a/libflux/flux-core/src/semantic/types.rs b/libflux/flux-core/src/semantic/types.rs index fd19b79728..84f86c7dd4 100644 --- a/libflux/flux-core/src/semantic/types.rs +++ b/libflux/flux-core/src/semantic/types.rs @@ -31,6 +31,8 @@ pub type SemanticMap = BTreeMap; pub type SemanticMapIter<'a, K, V> = std::collections::btree_map::Iter<'a, K, V>; trait Matcher { + fn name(&self) -> &'static str; + fn match_types( &self, unifier: &mut Unifier<'_, E>, @@ -42,6 +44,10 @@ trait Matcher { struct Unify; impl Matcher for Unify { + fn name(&self) -> &'static str { + "Unify" + } + fn match_types( &self, unifier: &mut Unifier<'_, Error>, @@ -65,6 +71,10 @@ impl Matcher for Unify { struct Subsume; impl Matcher for Subsume { + fn name(&self) -> &'static str { + "Subsume" + } + fn match_types( &self, unifier: &mut Unifier<'_, Error>, @@ -78,14 +88,8 @@ impl Matcher for Subsume { maybe_label: &'a MonoType, maybe_var: &'a MonoType, ) -> Cow<'a, MonoType> { - match maybe_var { - MonoType::Var(v) - if !unifier - .sub - .cons() - .get(v) - .map_or(false, |kinds| kinds.contains(&Kind::Label)) => - { + match *maybe_var { + MonoType::Var(v) if !unifier.sub.satisfies(v, Kind::Label) => { struct ReplaceLabels; impl Substituter for ReplaceLabels { fn try_apply(&mut self, _: Tvar) -> Option { @@ -106,8 +110,12 @@ impl Matcher for Subsume { _ => Cow::Borrowed(maybe_label), } } - let actual = translate_label(unifier, actual, expected); - let expected = translate_label(unifier, expected, &actual); + + let actual = unifier.sub.real(actual); + let expected = unifier.sub.real(expected); + + let actual = translate_label(unifier, &actual, &expected); + let expected = translate_label(unifier, &expected, &actual); match (&*expected, &*actual) { // Labels should be accepted anywhere that we expect a string (&MonoType::STRING, MonoType::Label(_)) => MonoType::STRING, @@ -964,7 +972,7 @@ impl MonoType { actual: &Self, unifier: &mut Unifier<'_>, ) -> MonoType { - log::debug!("Unify {} <=> {}", self, actual); + log::debug!("{} {} <=> {}", unifier.matcher.name(), self, actual); unifier.matcher.match_types(unifier, self, actual) } @@ -1027,7 +1035,7 @@ impl MonoType { } /// Validates that the current type meets the constraints of the specified kind. - pub fn constrain(&self, with: Kind, cons: &mut TvarKinds) -> Result<(), Error> { + pub fn constrain(&self, with: Kind, sub: &mut Substitution) -> Result<(), Error> { match self { MonoType::Error => Ok(()), MonoType::Builtin(typ) => typ.constrain(with), @@ -1047,14 +1055,11 @@ impl MonoType { exp: with, }), }, - MonoType::Var(tvr) => { - tvr.constrain(with, cons); - Ok(()) - } - MonoType::Collection(app) => app.constrain(with, cons), - MonoType::Dict(dict) => dict.constrain(with, cons), - MonoType::Record(obj) => obj.constrain(with, cons), - MonoType::Fun(fun) => fun.constrain(with, cons), + MonoType::Var(tvr) => tvr.constrain(with, sub), + MonoType::Collection(app) => app.constrain(with, sub), + MonoType::Dict(dict) => dict.constrain(with, sub), + MonoType::Record(obj) => obj.constrain(with, sub), + MonoType::Fun(fun) => fun.constrain(with, sub), } } @@ -1212,15 +1217,21 @@ impl Tvar { } } - fn constrain(&self, with: Kind, cons: &mut TvarKinds) { - match cons.get_mut(self) { - Some(kinds) => { - if !kinds.contains(&with) { - kinds.push(with); - } - } + fn constrain(&self, with: Kind, sub: &mut Substitution) -> Result<(), Error> { + match sub.try_apply(*self) { + Some(typ) => typ.constrain(with, sub), None => { - cons.insert(*self, vec![with]); + match sub.cons().get_mut(self) { + Some(kinds) => { + if !kinds.contains(&with) { + kinds.push(with); + } + } + None => { + sub.cons().insert(*self, vec![with]); + } + } + Ok(()) } } } @@ -1247,16 +1258,16 @@ impl Collection { self.arg.unify(&with.arg, unifier); } - fn constrain(&self, with: Kind, cons: &mut TvarKinds) -> Result<(), Error> { + fn constrain(&self, with: Kind, sub: &mut Substitution) -> Result<(), Error> { match self.collection { CollectionType::Array | CollectionType::Stream => match with { - Kind::Equatable => self.arg.constrain(with, cons), + Kind::Equatable => self.arg.constrain(with, sub), _ => Err(Error::CannotConstrain { act: MonoType::app(self.clone()), exp: with, }), }, - CollectionType::Vector => self.arg.constrain(with, cons), + CollectionType::Vector => self.arg.constrain(with, sub), } } @@ -1287,7 +1298,7 @@ impl Dictionary { self.val.unify(&actual.val, unifier); } - fn constrain(&self, with: Kind, _: &mut TvarKinds) -> Result<(), Error> { + fn constrain(&self, with: Kind, _: &mut Substitution) -> Result<(), Error> { Err(Error::CannotConstrain { act: MonoType::dict(self.clone()), exp: with, @@ -1552,16 +1563,16 @@ impl Record { } } - fn constrain(&self, with: Kind, cons: &mut TvarKinds) -> Result<(), Error> { + fn constrain(&self, with: Kind, sub: &mut Substitution) -> Result<(), Error> { match with { Kind::Record => Ok(()), Kind::Equatable => { let mut fields = self.fields(); for head in &mut fields { - head.v.constrain(with, cons)?; + head.v.constrain(with, sub)?; } match fields.tail() { - Some(t) => t.constrain(with, cons), + Some(t) => t.constrain(with, sub), None => Ok(()), } } @@ -2265,7 +2276,7 @@ impl Function { }) } - fn constrain(&self, with: Kind, _: &mut TvarKinds) -> Result<(), Error> { + fn constrain(&self, with: Kind, _: &mut Substitution) -> Result<(), Error> { Err(Error::CannotConstrain { act: MonoType::from(self.clone()), exp: with, @@ -2988,11 +2999,13 @@ mod tests { Kind::Stringable, ]; for c in allowable_cons { - MonoType::INT.constrain(c, &mut TvarKinds::new()).unwrap(); + MonoType::INT + .constrain(c, &mut Substitution::new()) + .unwrap(); } let sub = MonoType::INT - .constrain(Kind::Record, &mut TvarKinds::new()) + .constrain(Kind::Record, &mut Substitution::new()) .map(|_| ()); assert_eq!( Err(Error::CannotConstrain { @@ -3005,7 +3018,7 @@ mod tests { #[test] fn constrain_rows() { Record::Empty - .constrain(Kind::Record, &mut TvarKinds::new()) + .constrain(Kind::Record, &mut Substitution::new()) .unwrap(); let unallowable_cons = vec![ @@ -3018,7 +3031,7 @@ mod tests { ]; for c in unallowable_cons { let sub = Record::Empty - .constrain(c, &mut TvarKinds::new()) + .constrain(c, &mut Substitution::new()) .map(|_| ()); assert_eq!( Err(Error::CannotConstrain { @@ -3045,7 +3058,7 @@ mod tests { for c in allowable_cons_int { let vector_int = MonoType::vector(MonoType::INT); - vector_int.constrain(c, &mut TvarKinds::new()).unwrap(); + vector_int.constrain(c, &mut Substitution::new()).unwrap(); } // kind constraints not allowed for Vector(MonoType::STRING) @@ -3053,7 +3066,7 @@ mod tests { for c in unallowable_cons_string { let vector_string = MonoType::vector(MonoType::STRING); let sub = vector_string - .constrain(c, &mut TvarKinds::new()) + .constrain(c, &mut Substitution::new()) .map(|_| ()); assert_eq!( Err(Error::CannotConstrain { @@ -3068,7 +3081,9 @@ mod tests { let unallowable_cons_time = vec![Kind::Subtractable, Kind::Divisible, Kind::Numeric]; for c in unallowable_cons_time { let vector_time = MonoType::vector(MonoType::TIME); - let sub = vector_time.constrain(c, &mut TvarKinds::new()).map(|_| ()); + let sub = vector_time + .constrain(c, &mut Substitution::new()) + .map(|_| ()); assert_eq!( Err(Error::CannotConstrain { act: MonoType::TIME, @@ -3089,7 +3104,7 @@ mod tests { for c in allowable_cons_time { let vector_time = MonoType::vector(MonoType::TIME); - vector_time.constrain(c, &mut TvarKinds::new()).unwrap(); + vector_time.constrain(c, &mut Substitution::new()).unwrap(); } } #[test] diff --git a/libflux/go/libflux/buildinfo.gen.go b/libflux/go/libflux/buildinfo.gen.go index 9ad0df5dcb..933ac95d42 100644 --- a/libflux/go/libflux/buildinfo.gen.go +++ b/libflux/go/libflux/buildinfo.gen.go @@ -46,11 +46,11 @@ var sourceHashes = map[string]string{ "libflux/flux-core/src/semantic/fresh.rs": "97238fbc317e7c51836a6ba3441d641d9f4f8c7f637bde4bccbd0e09146129d0", "libflux/flux-core/src/semantic/fs.rs": "f7f609bc8149769d99b737150e184a2d54029c0b768365dbcf08ff193b0e1f6f", "libflux/flux-core/src/semantic/import.rs": "184e955211db1ceb1be782b4daf75584b86907b1428e50015497909cfc2dd89a", - "libflux/flux-core/src/semantic/infer.rs": "fcbe1bb017c3caf7935a1d29ec88b60b8f54defa047f6a915a8d941b2db154be", + "libflux/flux-core/src/semantic/infer.rs": "9d4293f2471a90cc89c1e45cdc72082e0da1a484981b803aea05856e6b4d722f", "libflux/flux-core/src/semantic/mod.rs": "3cec76c645e411592898a3e153d571787157bdfdf8552c21748c5e8c5e41d48d", - "libflux/flux-core/src/semantic/nodes.rs": "ee3865bb9edf67dcc3baa8b74fcbd132a00f7eb2cf937d9f6bbcb3d3bccce454", - "libflux/flux-core/src/semantic/sub.rs": "792b4a81d514e991d93998f407aa90527812c7c107ee827554b902b0af40aba3", - "libflux/flux-core/src/semantic/types.rs": "bd49bcf2da204a4e68a8612df68c499a4eaf65988a424a12bd71f5be4dd1be6f", + "libflux/flux-core/src/semantic/nodes.rs": "300de1a0f78f303b97736add050d9c8e5d90ad9554eaed6d24b79f9c39991f67", + "libflux/flux-core/src/semantic/sub.rs": "a989e50c113ca899fe02f8d49a4744a420580a3f803f656db25beb2d0c2a247b", + "libflux/flux-core/src/semantic/types.rs": "dc444143773bf261fb11520543964f6e03dadcec4a8f4b239e151a37710a5b1a", "libflux/flux-core/src/semantic/vectorize.rs": "6ce2dc4e6ff572abc0146a220291322ea88557ce674ae16220a2d67420e9fa0d", "libflux/flux-core/src/semantic/walk/_walk.rs": "198e6c546f73f78b1de4b963084033a06a6d71e72b0294b16c5e4de874f97a38", "libflux/flux-core/src/semantic/walk/mod.rs": "a0eab3f225dc294569217da295d44f31073e643073e6b00fec5846cde2382ade",