jess
/
Graphite
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix faulty contravariance checking (#2025) 

* Fix faulty contravariance checking

* Fix documented soundness issue in type resolution
This commit is contained in:
Dennis Kobert 2024-10-07 21:35:27 +02:00 committed by GitHub
parent fa6b5f298a
commit bb8560e5ad
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 7 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
node-graph/graph-craft/src/proto.rs
View File

@ -712,7 +712,7 @@ impl TypingContext {
// While these two relations aren't a truth about the universe, they are a design decision that we are employing in our language design that is also common in other languages. // While these two relations aren't a truth about the universe, they are a design decision that we are employing in our language design that is also common in other languages.
// For example, Rust implements these same relations as it describes here: <https://doc.rust-lang.org/nomicon/subtyping.html> // For example, Rust implements these same relations as it describes here: <https://doc.rust-lang.org/nomicon/subtyping.html>
// More details explained here: <https://github.com/GraphiteEditor/Graphite/issues/1741> // More details explained here: <https://github.com/GraphiteEditor/Graphite/issues/1741>
(Type::Fn(in1, out1), Type::Fn(in2, out2)) => valid_subtype(out1, out2) && (valid_subtype(in1, in2) || **in1 == concrete!(())), (Type::Fn(in1, out1), Type::Fn(in2, out2)) => valid_subtype(out2, out1) && (valid_subtype(in1, in2) || **in1 == concrete!(())),
// If either the proposed input or the allowed input are generic, we allow the substitution (meaning this is a valid subtype). // If either the proposed input or the allowed input are generic, we allow the substitution (meaning this is a valid subtype).
// TODO: Add proper generic counting which is not based on the name // TODO: Add proper generic counting which is not based on the name
(Type::Generic(_), _) | (_, Type::Generic(_)) => true, (Type::Generic(_), _) | (_, Type::Generic(_)) => true,
@ -724,7 +724,7 @@ impl TypingContext {
// List of all implementations that match the input types // List of all implementations that match the input types
let valid_output_types = impls let valid_output_types = impls
.keys() .keys()
.filter(|node_io| valid_subtype(&input, &node_io.call_argument) && inputs.iter().zip(node_io.inputs.iter()).all(|(p1, p2)| valid_subtype(p1, p2))) .filter(|node_io| valid_subtype(&node_io.call_argument, &input) && inputs.iter().zip(node_io.inputs.iter()).all(|(p1, p2)| valid_subtype(p1, p2)))
.collect::<Vec<_>>(); .collect::<Vec<_>>();
// Attempt to substitute generic types with concrete types and save the list of results // Attempt to substitute generic types with concrete types and save the list of results
@ -772,9 +772,8 @@ impl TypingContext {
let inputs = [&input].into_iter().chain(&inputs).map(|t| t.to_string()).collect::<Vec<_>>().join(", "); let inputs = [&input].into_iter().chain(&inputs).map(|t| t.to_string()).collect::<Vec<_>>().join(", ");
Err(vec![GraphError::new(node, GraphErrorType::InvalidImplementations { inputs, error_inputs })]) Err(vec![GraphError::new(node, GraphErrorType::InvalidImplementations { inputs, error_inputs })])
} }
[(org_nio, output)] => { [(org_nio, _)] => {
// TODO: Fix unsoundness caused by generic parameters not getting cleaned up let node_io = org_nio.clone();
let node_io = NodeIOTypes::new(input, (*output).clone(), inputs);
// Save the inferred type // Save the inferred type
self.inferred.insert(node_id, node_io.clone()); self.inferred.insert(node_id, node_io.clone());
@ -784,16 +783,15 @@ impl TypingContext {
// If two types are available and one of them accepts () an input, always choose that one // If two types are available and one of them accepts () an input, always choose that one
[first, second] => { [first, second] => {
if first.0.call_argument != second.0.call_argument { if first.0.call_argument != second.0.call_argument {
for (org_nio, output) in [first, second] { for (org_nio, _) in [first, second] {
if org_nio.call_argument != concrete!(()) { if org_nio.call_argument != concrete!(()) {
continue; continue;
} }
let node_io = NodeIOTypes::new(input, (*output).clone(), inputs);
// Save the inferred type // Save the inferred type
self.inferred.insert(node_id, node_io.clone()); self.inferred.insert(node_id, org_nio.clone());
self.constructor.insert(node_id, impls[org_nio]); self.constructor.insert(node_id, impls[org_nio]);
return Ok(node_io); return Ok(org_nio.clone());
} }
} }
let inputs = [&input].into_iter().chain(&inputs).map(|t| t.to_string()).collect::<Vec<_>>().join(", "); let inputs = [&input].into_iter().chain(&inputs).map(|t| t.to_string()).collect::<Vec<_>>().join(", ");