Recent update#10
Hidden character warning
Conversation
| obj_8 | ||
| let {obj_5} | ||
| obj_5 = new D() | ||
| obj_5 |
There was a problem hiding this comment.
Hm, why isn't this instantiation inlined like here?
https://github.com/ChingLongTin/mlscript/pull/10/changes#diff-57aec0daccc22139279b3f2313f4e5c9ed13c42f9e64b17d9314a3000f0b8eeeR8
ChingLongTin
left a comment
ChingLongTin
left a comment
There was a problem hiding this comment.
There is a regression that should be addressed: https://github.com/ChingLongTin/mlscript/pull/10/changes#r3350061642
The remaining are some minor suggestions.
| let x = liftMany([Class(clsSym, [Lit(42)]), Arr([Lit(1), Lit("a")])]) | ||
| let y = liftMany([Lit(1), Lit("a")]) | ||
| selSet(x, y) | ||
| //│ ═══[RUNTIME ERROR] Error: Array out of bound | ||
| //│ ═══[RUNTIME ERROR] TypeError: arg$Class$1$[scrut1].values is not a function |
There was a problem hiding this comment.
| let x = liftMany([Class(clsSym, [Lit(42)]), Arr([Lit(1), Lit("a")])]) | |
| let y = liftMany([Lit(1), Lit("a")]) | |
| selSet(x, y) | |
| //│ ═══[RUNTIME ERROR] Error: Array out of bound | |
| //│ ═══[RUNTIME ERROR] TypeError: arg$Class$1$[scrut1].values is not a function | |
| let x = liftMany([Class(clsSym, [mkLit(42)]), Arr([mkLit(1), mkLit("a")])]) | |
| let y = liftMany([Lit(1), Lit("a")]) | |
| selSet(x, y) | |
| //│ ═══[RUNTIME ERROR] Error: Array out of bound | |
| //│ x = {Arr([{Lit(1)}, {Lit("a")}]),Class(ConcreteClassSymbol("C", fun C { class: class C }, Some([Param(None, Symbol("a"))]), [], false), [{Lit(42)}])} | |
| //│ y = {Lit("a"),Lit(1)} |
| fun empty = ShapeSet(new Map) | ||
|
|
||
| fun lift(s: Shape) = ShapeSet(new Map([[s.hash(), s]])) | ||
| fun makeShapeSet(entries) = |
There was a problem hiding this comment.
| fun makeShapeSet(entries) = | |
| fun mkShapeSet(entries) = |
| import "./Block.mls" | ||
| import "./Option.mls" | ||
| import "./CachedHash.mls" | ||
| import "./ShapeSet.mjs" |
There was a problem hiding this comment.
We might want to put Shape and ShapeSet in a single module in a later commit instead of having both files importing each other.
| Nothing() then p1' | ||
| else new Concat(p1', p2.normalize()) | ||
| fun eq(other) = | ||
| if other is Concat(p1', p2') then p1.eq(p1') and p2.eq(p2') else false |
There was a problem hiding this comment.
| if other is Concat(p1', p2') then p1.eq(p1') and p2.eq(p2') else false | |
| other is Concat(p1', p2') and p1.eq(p1') and p2.eq(p2') |
other eq functions in this file can also be rewritten this way, removing the tail else false block
There was a problem hiding this comment.
somehow, the codegen for this is worse for Exact, adding a match statement on a boolean scrut... but in Concat the extra match statement is always generated
| Class(_, params) then params.every(field => | ||
| let values = field.values() | ||
| values.length is 1 and static(values.0) | ||
| ) |
There was a problem hiding this comment.
| Class(_, params) then params.every(field => | |
| let values = field.values() | |
| values.length is 1 and static(values.0) | |
| ) | |
| Class(_, params) then params.every(_.values() is [v] and static(v)) |
| fun matchImpl(p, s, acc) = | ||
| if len(s) == 0 then | ||
| if p.canBeEmpty() then new Some(acc) | ||
| else new None | ||
| else | ||
| if p is | ||
| Nothing then new None | ||
| else | ||
| let c = s.0 | ||
| if p.startsWith(c) then | ||
| matchImpl(p.derive(c), s.slice(1), acc + c) | ||
| else | ||
| if p.canBeEmpty() then new Some(acc) else new None |
There was a problem hiding this comment.
| fun matchImpl(p, s, acc) = | |
| if len(s) == 0 then | |
| if p.canBeEmpty() then new Some(acc) | |
| else new None | |
| else | |
| if p is | |
| Nothing then new None | |
| else | |
| let c = s.0 | |
| if p.startsWith(c) then | |
| matchImpl(p.derive(c), s.slice(1), acc + c) | |
| else | |
| if p.canBeEmpty() then new Some(acc) else new None | |
| fun matchImpl(p, s, acc) = if | |
| len(s) == 0 and | |
| p.canBeEmpty() then new Some(acc) | |
| else new None | |
| p is Nothing then new None | |
| let c = s.0 | |
| p.startsWith(c) then matchImpl(p.derive(c), s.slice(1), acc + c) | |
| p.canBeEmpty() then new Some(acc) | |
| else new None |
can simplify match(All)Impl with ucs
There was a problem hiding this comment.
I think this is also equivalent? oh, but this uses spread
fun matchImpl(p, s, acc) = if
p is Nothing then new None
s is [c, ...] and p.startsWith(c) then matchImpl(p.derive(c), s.slice(1), acc + c)
p.canBeEmpty() then new Some(acc)
else new None
There was a problem hiding this comment.
The same suggestions in SimpleRegExp should apply here.
| match(Exact("x"), "x") | ||
| //│ = Some("x") | ||
|
|
There was a problem hiding this comment.
Dupe test case
| match(Exact("x"), "x") | |
| //│ = Some("x") |
| let fullBlk = foldl((acc, e) => concat(acc, e.0))(End(), ...evaledArgs) | ||
| let newArgs = evaledArgs.map(e => Arg(if e.1 is Path then e.1 else throw Error("expected path"))) | ||
| [fullBlk, Call(f, newArgs), mkDyn()] // non staged function and some params unknown | ||
| argShapes.every(staticSet) then // non staged function and params known |
There was a problem hiding this comment.
This seems to make the sorUnknownCall fallback inside sorStaticModuleCall unreachable.
2 participants
dynmerging