pkl icon indicating copy to clipboard operation
pkl copied to clipboard

Type constraints on `typealias`es are evaluated eagerly

Open sin-ack opened this issue 10 months ago • 3 comments

Consider the following (the required typealias redirection is another issue):

// foo.pkl
typealias FooMapping = Mapping<String, String>
foo: FooMapping(toMap().every((k, v) -> bar.containsKey(k))

bar: Mapping<String, Boolean>

// bar.pkl
amends "./foo.pkl"

bar {
  "a" = true
  "b" = false
}

foo {
  "a" = "aaa"
}
$ pkl eval bar.pkl
foo {
  ["a"] = "aaa"
}
bar {
  ["a"] = true
  ["b"] = false
}

As expected. However, if we change foo's definition to:

typealias FooMapping1 = Mapping<String, String>
typealias FooMapping = FooMapping1(toMap().every((k, v) -> bar.containsKey(k))
foo: FooMapping

Then I get:

$ pkl eval bar.pkl
–– Pkl Error ––
Type constraint `toMap().every((k, v) -> bar.containsKey(k))` violated.
Value: new Mapping { ["a"] = "aaa" }

2 | typealias FooMapping = FooMapping1(toMap().every((k, v) -> bar.containsKey(k)))
                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
at foo#foo (file:///.../foo.pkl, line 2)

8 | foo {
    ^^^^^
at bar#foo (file:///.../bar.pkl, line 8)

Placing a trace() on bar reveals that bar is a Mapping {} during the evaluation of the constraint, which is unexpected (to me at least). Is this intended behavior?

sin-ack avatar Apr 22 '24 14:04 sin-ack

My understanding is that accessing bar from within a type constraint isn’t allowed because it isn’t const. If you don’t get an error for that it’s probably a bug.

translatenix avatar Apr 22 '24 19:04 translatenix

I can't reproduce this (but also; the snippets you included are not syntactically valid Pkl, so I think something got lost in reduction of the example).

There is a known bug that typealiases resolved names in their definition scope, instead of their use scope. This has been addressed in #373 and will be part of 0.26 (hopefully June).

By the way, toMap() already forces the entire Mapping (keys and values). If you want to keep things lazy, consider keys.intersect(bar.keys) == keys instead.

holzensp avatar Apr 23 '24 15:04 holzensp

I think @sin-ack is using 0.26.0-dev, which reproduces for me.

We currently have an issue in the latest dev version where typealias bodies are not late-bound (changed a fix for a scoping bug in https://github.com/apple/pkl/pull/144). But, at the same time, they aren't required to be const.

We're thinking through whether typealias bodies should be late-bound, or if this is actually the correct behavior. If this is the correct behavior, then we will add an additional rule that references must be const. If not, then we need to adjust the behavior here so that those references are late-bound.

bioball avatar Apr 25 '24 03:04 bioball