auto-derived Inspect · contributing

so I have a bunch of work on auto-derived Inspect, but I think I should really focus on documentation and FAQs and such this final week before advent of code - is anyone else able to pick that up?

Richard Feldman (Nov 26 2023 at 16:40):

I really think it'd help people out a lot to have a reliable dbg for Advent of Code, and this is the only nontrivial blocker to that!

Brendan Hansknecht (Nov 26 2023 at 16:51):

Richard Feldman (Nov 26 2023 at 17:07):

Richard Feldman (Nov 26 2023 at 17:36):

if you're interested in it, I'd be happy to chat about it and get you up to speed! :smiley:

Brendan Hansknecht (Nov 26 2023 at 18:16):

Brendan Hansknecht (Nov 28 2023 at 01:49):

thread 'gen_abilities::inspect_bool' panicked at 'assertion failed: `(left == right)`
  left: `0`,
 right: `1`: (LambdaSet([], ^<2128>), Func([<260>EmptyRecord,], <2130=263>LambdaSet([], ^<2128>), <264>Opaque(`Inspect.DbgFormatter`, [], <3300>{ 'data' : Apply(`Str.Str`, []), }<3301>)))', crates/compiler/mono/src/ir.rs:6129:9

thread 'gen_abilities::inspect_num' panicked at 'No lambda set at region 2 found', crates/compiler/types/src/subs.rs:6087:5

Ayaz Hafiz (Nov 28 2023 at 02:15):

Usually this means the specialization of an ability member for a specific type is missing or under defined. Do you what a lambda region is?

Brendan Hansknecht (Nov 28 2023 at 02:17):

Brendan Hansknecht (Nov 28 2023 at 02:25):

Also, at least for a type like Bool.Bool where the inspector just generates a direct call to the immediate Inspect.bool, I don't see a location where were are doing anything complex within the derive code for inspect itself. Totally see how dict, list, record, etc are complex and could hit bigger issues, but unsure why something like just Bool.Bool would hit some sort of issue.

Ayaz Hafiz (Nov 28 2023 at 02:38):

Ayaz Hafiz (Nov 28 2023 at 02:39):

On mobile so I’ll take a deeper look later and explain lambda regions but I can explain broad details

Brendan Hansknecht (Nov 28 2023 at 02:41):

in ability type definition: bool : Bool -> Inspector f where f implements InspectFormatter

dbgBool : Bool -> Inspector DbgFormatter
dbgBool = \b ->
    if b then
        f0 <- custom
        dbgWrite f0 "true"
    else
        f0 <- custom
        dbgWrite f0 "false"

Brendan Hansknecht (Nov 28 2023 at 02:42):

custom : (f -> f) -> Inspector f where f implements InspectFormatter
custom = @Inspector

Ayaz Hafiz (Nov 28 2023 at 02:44):

Brendan Hansknecht (Nov 28 2023 at 02:44):

dbgWrite : DbgFormatter, Str -> DbgFormatter
dbgWrite = \@DbgFormatter { data }, added ->
    @DbgFormatter { data: Str.concat data added }

Ayaz Hafiz (Nov 28 2023 at 02:45):

Ayaz Hafiz (Nov 28 2023 at 02:46):

It’s usually missing when a specialization function corresponding to the same arrow as defined in an ability member is missing. But the specialization here seems to have three arrows, like the ability member

Brendan Hansknecht (Nov 28 2023 at 02:47):

Brendan Hansknecht (Nov 28 2023 at 02:48):

Was trying to fix bool or num tests before turning to the derive implementations that actually generate code rather than just call an existing function.

Ayaz Hafiz (Nov 28 2023 at 02:48):

you might also want to run with ROC_TRACE_COMPACTION=1. It will show some useful specialization traces

Brendan Hansknecht (Nov 28 2023 at 03:36):

Looking at the compaction, the main thing I derive is that the failing match looks to be for Inspect.init. That is the function that generates the initial formatter.

Ayaz Hafiz (Nov 28 2023 at 05:52):

app "test" provides [main] to "./platform"

main =
    Inspect.toInspector Bool.true |> Inspect.apply (Inspect.init {}) |> Inspect.toDbgStr
#   ^^^^^^^^^^^^^^^^^^^ Inspect#Inspect.toInspector(32): Bool -[[]]-> Inspector DbgFormatter

===lambda set compaction===
  concrete type: Opaque(`Inspect.DbgFormatter`, [], <2679>{ 'data' : Apply(`Str.Str`, []), }<115>)
  step 1:
    uls_a = { LambdaSet([] + (<2710>Opaque(`Inspect.DbgFormatter`, [], <2679>{ 'data' : Apply(`Str.Str`, []), }<115>):`Inspect.init`:1), ^<2709>),LambdaSet([] + (<2699>Op
aque(`Inspect.DbgFormatter`, [], <2679>{ 'data' : Apply(`Str.Str`, []), }<115>):`Inspect.bool`:2), ^<2694>),LambdaSet([] + (<2706>Opaque(`Inspect.DbgFormatter`, [], <2679
>{ 'data' : Apply(`Str.Str`, []), }<115>):`Inspect.bool`:1), ^<2688>) }
  step 2:
    uls_a' = { LambdaSet([] + (<2710>Opaque(`Inspect.DbgFormatter`, [], <2679>{ 'data' : Apply(`Str.Str`, []), }<115>):`Inspect.init`:1), ^<2709>),LambdaSet([] + (<2699>O
paque(`Inspect.DbgFormatter`, [], <2679>{ 'data' : Apply(`Str.Str`, []), }<115>):`Inspect.bool`:2), ^<2694>),LambdaSet([] + (<2706>Opaque(`Inspect.DbgFormatter`, [], <267
9>{ 'data' : Apply(`Str.Str`, []), }<115>):`Inspect.bool`:1), ^<2688>) }
  step 3:
      SKIP
      =  Func([<74>EmptyRecord,], <2711=77>LambdaSet([], ^<2709>), <78>Opaque(`Inspect.DbgFormatter`, [], <2679>{ 'data' : Apply(`Str.Str`, []), }<115>))

      SKIP
      =  Func([<2682>Opaque(`Inspect.DbgFormatter`, [], <2679>{ 'data' : Apply(`Str.Str`, []), }<115>),], <2693=2685>LambdaSet([], ^<2694>), <2682>Opaque(`Inspect.DbgForm
atter`, [], <2679>{ 'data' : Apply(`Str.Str`, []), }<115>))

      SKIP
      =  Func([<2704>Opaque(`Bool.Bool`, [], <2697>['False' , 'True' , ]<Any(92)>),], <2690=2703>LambdaSet([], ^<2688>), <2702>Opaque(`Inspect.Inspector`, [2692, 2693], <
2694>Func([<2682>Opaque(`Inspect.DbgFormatter`, [], <2679>{ 'data' : Apply(`Str.Str`, []), }<115>),], <2693=2685>LambdaSet([], ^<2694>), <2682>Opaque(`Inspect.DbgFormatte
r`, [], <2679>{ 'data' : Apply(`Str.Str`, []), }<115>))))

Ayaz Hafiz (Nov 28 2023 at 05:52):

Ayaz Hafiz (Nov 28 2023 at 05:58):

Ayaz Hafiz (Nov 28 2023 at 05:59):

Ayaz Hafiz (Nov 28 2023 at 06:00):

Ayaz Hafiz (Nov 28 2023 at 06:01):

app "test" provides [main] to "./platform"

main =
    Inspect.toInspector Bool.true |> Inspect.apply (Inspect.init {}) |> Inspect.toDbgStr
#   ^^^^^^^^^^^^^^^^^^^ Inspect#Inspect.toInspector(32): Bool -[[Inspect.dbgBool(43)]]-> Inspector DbgFormatter

Ayaz Hafiz (Nov 28 2023 at 06:02):

thread 'gen_abilities::inspect_bool' panicked at 'assertion failed: `(left == right)`
  left: `"true, false"`,
 right: `"Bool.true, Bool.false"`: LLVM test failed', crates/compiler/test_gen/src/helpers/llvm.rs:631:13

Ayaz Hafiz (Nov 28 2023 at 06:03):

As a side note, I would put all of the inspect gen tests into a mod inspect like is done for hash, encode, decode, etc. It helps a lot with navigating the file.

Ayaz Hafiz (Nov 28 2023 at 06:03):

Brendan Hansknecht (Nov 28 2023 at 06:04):

Ayaz Hafiz (Nov 28 2023 at 06:05):

Ayaz Hafiz (Nov 28 2023 at 06:06):

And, adding mono tests as well as gen tests. The mono tests will also catch things early on. If mono succeeds, codegen very likely will

Brendan Hansknecht (Nov 28 2023 at 20:13):

custom = @Inspector

custom = \fn -> @Inspector fn

Ayaz Hafiz (Nov 28 2023 at 20:14):

Richard Feldman (Nov 28 2023 at 20:16):

Richard Feldman (Nov 28 2023 at 20:17):

like how you can't do mainForHost = main if main is a function, you have to do mainForHost = \arg -> main arg

Ayaz Hafiz (Nov 28 2023 at 20:17):

Ayaz Hafiz (Nov 28 2023 at 20:23):

Brendan Hansknecht (Nov 28 2023 at 20:50):

Brendan Hansknecht (Nov 28 2023 at 20:55):

Anyway, I think I have autoderived inspect working now. Not wired to dbg, but otherwise working.

Brendan Hansknecht (Nov 28 2023 at 21:04):

── PROC SPECIALIZATION NOT DEFINED ─────────────────────────────────────────────

in toInspector_[A 2] : ({Str, Str}) -> {Str, Str} ((niche {}))

0│  procedure `#Derived.toInspector_[A 2]` (`#Derived.tag`):
1│>     let `#Derived_gen.0` : {Str, Str} = CallByName `Inspect.custom` `#Derived.tag`;
2│      ret `#Derived_gen.0`;

No specialization

custom : ({Str, Str}) -> {Str, Str} ((niche {}))

was found

The following specializations of Inspect.custom were built:custom : () ->
{} ((niche {}))

custom : () -> {} ((niche {}))

custom : () -> {} ((niche {}))', crates/compiler/test_mono/src/tests.rs:177:5

Richard Feldman (Nov 28 2023 at 21:05):

Luke Boswell (Nov 28 2023 at 21:09):

Brendan Hansknecht (Nov 28 2023 at 21:17):

Also, I think the plan is to submit #5775, make another PR for dbg changes, and then add some post fixes as needed.

Ayaz Hafiz (Nov 28 2023 at 21:50):

I'll review this tonight. Do you guys mind holding off merging until I have a chance to look at this?

Brendan Hansknecht (Nov 28 2023 at 21:56):

Brendan Hansknecht (Nov 28 2023 at 21:57):

Brendan Hansknecht (Nov 28 2023 at 22:03):

Wouldn't mind some help with them if you have any ideas. Just updated tests to show the errors:

Ayaz Hafiz (Nov 28 2023 at 22:22):

Ayaz Hafiz (Nov 28 2023 at 22:25):

Regarding (2), this is a bit of a problem. In the future I'd like to get rid of that panic and instead have the "unspecialized..." thing return an error function, because if the type variable is unbound, we know that the specialization function will never be called. But as you've noticed, in the meantime it finds actual errors. So I am inclined to say that debugging a list with non concrete type should not be supported for now, or we should generate the synthetic error function only in non-debug builds.

Brendan Hansknecht (Nov 28 2023 at 23:02):

(1) I don't think it is Frac missing in the switch. Num and Integer are also not in the switch, but they work just fine. Frac is a wrapping type, so we want to unwrap it and get to the underlying Dec/F32/F64. Unspecified number types work because when we unwrap, it results in an I64. For Frac, it should unwrap to Dec by default if the type is unspecfied. So I think this is an issue with default number handling logic elsewhere.

Brendan Hansknecht (Nov 28 2023 at 23:03):

For (2), sounds reasonable to ignore for now. Long term can either pipeline through a nicer error or a special exception.

Ayaz Hafiz (Nov 28 2023 at 23:14):

Ayaz Hafiz (Nov 28 2023 at 23:15):

So what you want is to check if it's a Frac with an unbound type variable (FlexVar - Rigid means it's a bug, because it's trying to specialize a generalized type, which is nonsense). In that case, you know it will compile to the default Dec, so you can use the Dec specialization.

Brendan Hansknecht (Nov 28 2023 at 23:16):

Brendan Hansknecht (Nov 28 2023 at 23:22):

Brendan Hansknecht (Nov 28 2023 at 23:30):

Brendan Hansknecht (Nov 29 2023 at 00:15):

Interesting wasm failure. Is this a case where we are passing a wrapping type to NumToStr, but it has the same underlying layout as a Numeric Type. As such, it theoretically can just be treated as the underlying numeric type? Not exactly sure the correct fix on the wasm side, but I assume it is something like instead of checking if the type is valid, check if the underlying layout is valid.

thread 'gen_abilities::inspect::list' panicked at 'NumToStr is not defined for LambdaSet(LambdaSet { set: [( 15.295, [InLayout(DEC)])], args: [InLayout(25)], ret: InLayout(25), representation: InLayout(DEC), full_layout: InLayout(81) })', crates/compiler/gen_wasm/src/low_level.rs:2272:18

Brendan Hansknecht (Nov 29 2023 at 00:39):

So yeah, using the runtime representation is a fix in the wasm backend. That said, I wonder if that means tons of low levels for wasm should be using the runtime representation to extract the inner layout from lambdasets. For now, just changing the single location to fix a test, but feels like probably a symptom of a bigger issue.

Brendan Hansknecht (Nov 29 2023 at 00:48):

For using Inspect with dbg. That will need to be handled at the mono or higher level, correct? Cause dbg no longer has enough information to just be dealt with by the backends. Instead, dbg val essentially has to desugar into Inspect.inspect val |> Inspect.toDbgStr |> roc_dbg, right? Then that new set of commands will get lowered by the backends.

Richard Feldman (Nov 29 2023 at 00:49):

Brendan Hansknecht (Nov 29 2023 at 00:52):

At what level do we normally desugar these things? Any specific code location? I assume either can or mono somewhere.

Brendan Hansknecht (Nov 29 2023 at 00:52):

Richard Feldman (Nov 29 2023 at 00:53):

Richard Feldman (Nov 29 2023 at 00:54):

Brendan Hansknecht (Nov 29 2023 at 00:57):

Oh, should debug still automatically add on [examples/helloWorld.roc 7:9], I guess that is not covered by inspect. Though in a number of platforms, I assume it is not wanted. Maybe dbg should pass two strings, location and message?

Richard Feldman (Nov 29 2023 at 00:58):

Brendan Hansknecht (Nov 29 2023 at 01:10):

Richard Feldman (Nov 29 2023 at 01:13):

Agus Zubiaga (Nov 29 2023 at 01:25):

The Expr version sounds like elm’s Debug.log which returns the value it logs. I thought dbg couldn’t do that but I’d love it if it did :grinning:

Agus Zubiaga (Nov 29 2023 at 01:26):

It’s very nice when you have some code already written and want to “sprinkle” some logging for debug purposes

Brendan Hansknecht (Nov 29 2023 at 01:28):

dbg "some value"

continuation

Brendan Hansknecht (Nov 29 2023 at 01:30):

Aside, I guess since desugaring still generates an Expr, I have to actually make an expr that is Expr::LowLevelDbg.

Brendan Hansknecht (Nov 29 2023 at 01:30):

Richard Feldman (Nov 29 2023 at 02:14):

I just wanted to start with the "statement" version because I know from Elm that if that doesn't exist, people will do _ = dbg ... and I wanted to see if there was still demand for the other style if we had the statement version

Richard Feldman (Nov 29 2023 at 02:14):

Agus Zubiaga (Nov 29 2023 at 02:27):

Brendan Hansknecht (Nov 29 2023 at 02:35):

For dbg, do we want to attempt to wire through a file name or should we just give a module name (or other I guess)?

Richard Feldman (Nov 29 2023 at 03:23):

Brendan Hansknecht (Nov 29 2023 at 04:02):

I mean we currently use filename, just need to figure out wiring it through differently. Cause now we need it at compile time instead of runtime.

Brian Carroll (Nov 29 2023 at 07:28):

No, there are only 5 existing low-level ops that can actually take lambdasets as operands in the first place! It's just List.map and the other "higher order low levels". In the wasm backend, those lambda sets are converted to their runtime representations here.

Brian Carroll (Nov 29 2023 at 07:49):

Apart from low-levels, there are about 4 or 5 other places where it's needed. Converting IR symbol layout to WasmLayout, inserting data into structs, etc.

Pearce Keesling (Nov 29 2023 at 14:30):

If I understand correctly that means behaving like dbg! In rust where I get the value back out so I can just add the debug macro into a bigger expression. Yeah I'm a big fan of that. I'd extra love it if I didn't need to add parentheses cause that is cumbersome in rust because I'm potentially wrapping a huge chunk of code

Richard Feldman (Nov 29 2023 at 15:05):

Brendan Hansknecht (Nov 29 2023 at 16:24):

Somehow the inspect ability lead to a lambdaset getting past to Num.toStr in the wasm backend. The Lambdaset was just a Dec under the hood.

Brendan Hansknecht (Nov 29 2023 at 16:27):

Ayaz Hafiz (Nov 29 2023 at 16:51):

Ayaz Hafiz (Nov 29 2023 at 16:52):

Lambda sets should compile to an int, struct, or union. And they should only appear where function types are expected

Ayaz Hafiz (Nov 29 2023 at 16:52):

Brendan Hansknecht (Nov 29 2023 at 16:55):

gen_abilities::inspect::list and gen_abilities::inspect::num, but only for test-gen-wasm

Brendan Hansknecht (Nov 29 2023 at 16:55):

Not sure why, but the incorrect parameters only happen with the dev wasm backend.

Brendan Hansknecht (Nov 29 2023 at 16:57):

The llvm backend uses match layout_interner.get_repr(num_layout) without handling lambasets and just works. So maybe the wasm backend has a bug with mapping layouts to args?

Ayaz Hafiz (Nov 29 2023 at 17:06):

Brendan Hansknecht (Nov 29 2023 at 17:18):

I am not sure, but I recall it resolving to a Dec. So probably Dec or Frac case of some sort.

Brendan Hansknecht (Nov 29 2023 at 17:28):

For symbols like roc_alloc in the llvm backend, where do the external function headers for them get defined. Searched around a bit, but not finding it except for testing.

Trying to do let function = self.module.get_function("roc_dbg").unwrap();, but unwrapping will result in a None.

Ayaz Hafiz (Nov 29 2023 at 17:29):

diff --git a/crates/compiler/mono/src/layout/intern.rs b/crates/compiler/mono/src/layout/intern.rs
index 9891c59f2..138d602cb 100644
--- a/crates/compiler/mono/src/layout/intern.rs
+++ b/crates/compiler/mono/src/layout/intern.rs
@@ -341,9 +341,10 @@ pub trait LayoutInterner<'a>: Sized {
                 }
                 doc
             }
-            LambdaSet(lambda_set) => {
-                self.to_doc(lambda_set.runtime_representation(), alloc, seen_rec, parens)
-            }
+            LambdaSet(lambda_set) => alloc
+                .text("LambdaSet(")
+                .append(self.to_doc(lambda_set.runtime_representation(), alloc, seen_rec, parens))
+                .append(")"),
             RecursivePointer(rec_layout) => {
                 if seen_rec.contains(&rec_layout) {
                     alloc.text("*self")
diff --git a/crates/compiler/test_gen/src/gen_abilities.rs b/crates/compiler/test_gen/src/gen_abilities.rs
index 2bc4791b7..4d099c386 100644
--- a/crates/compiler/test_gen/src/gen_abilities.rs
+++ b/crates/compiler/test_gen/src/gen_abilities.rs
@@ -2222,21 +2222,7 @@ mod inspect {
             app "test" provides [main] to "./platform"

             main = [
-                Inspect.inspect 0,              # Num a
                 Inspect.inspect 1u8,            # U8
-                Inspect.inspect 2i8,            # I8
-                Inspect.inspect 3u16,           # U16
-                Inspect.inspect 4i16,           # I16
-                Inspect.inspect 5u32,           # U32
-                Inspect.inspect 6i32,           # I32
-                Inspect.inspect 7u64,           # U64
-                Inspect.inspect 8i64,           # I64
-                Inspect.inspect 9u128,          # U128
-                Inspect.inspect 10i128,         # I128
-                Inspect.inspect 0.5,            # Frac a
-                Inspect.inspect 1.5f32,         # F32
-                Inspect.inspect 2.2f64,         # F64
-                Inspect.inspect (1.1dec + 2.2), # Dec
             ] |> List.map Inspect.toDbgStr |> Str.joinWith ", "
             "#
             ),

Ayaz Hafiz (Nov 29 2023 at 17:29):

❯ ROC_PRINT_IR_AFTER_RESET_REUSE=1 ROC_CHECK_MONO_IR=1 cargo test-gen-wasm inspect::num

Ayaz Hafiz (Nov 29 2023 at 17:30):

procedure : `Inspect.256` Str
procedure = `Inspect.256` (`Inspect.f0`: Str, `Inspect.num`: LambdaSet(U8)):
    let `Inspect.318` : Str = CallByName `Num.toStr` `Inspect.num`;
    let `Inspect.317` : Str = CallByName `Inspect.dbgWrite` `Inspect.f0` `Inspect.318`;
    ret `Inspect.317`;

Ayaz Hafiz (Nov 29 2023 at 17:31):

procedure : `Inspect.inspect` Str
procedure = `Inspect.inspect` (`Inspect.val`: U8):
    let `Inspect.312` : LambdaSet(U8) = CallByName `Inspect.dbgU8` `Inspect.val`;
    let `Inspect.309` : {} = Struct {};
    let `Inspect.308` : Str = CallByName `Inspect.dbgInit` `Inspect.309`;
    let `Inspect.307` : Str = CallByName `Inspect.256` `Inspect.308` `Inspect.312`;
    ret `Inspect.307`;

Ayaz Hafiz (Nov 29 2023 at 17:32):

But that Inspect.256 is definitely wrong, which I think you're right is that dbg* function

Ayaz Hafiz (Nov 29 2023 at 17:32):

It's almost certainly a problem in the chosen specialization, since after the specialization is applied it's not type-checked again

Brendan Hansknecht (Nov 29 2023 at 18:31):

$ cargo run examples/platform-switching/main.roc
    Finished dev [unoptimized + debuginfo] target(s) in 0.50s
     Running `target/debug/roc examples/platform-switching/main.roc`
🔨 Rebuilding platform...
[TODO: add filepath here] {num: 32, str: "test"}
Which platform am I running on now?

Brendan Hansknecht (Nov 29 2023 at 18:34):

Also, in all of our platform switching examples, roc_panic is technically wrong. It only works with small strings cause it assume a char*. instead of a RocStr*.

Brendan Hansknecht (Nov 30 2023 at 00:46):

It is still missing a lot, but is nearly at a place where we could submit it just to enable a better version of dbg for advent of code. At a minimum, I need to update all the platforms to include roc_dbg, but I listed a number of other goals in the PR. Not sure how many we will get to, but I think even without the extra goals, it should be submittable soon.

Brendan Hansknecht (Nov 30 2023 at 06:48):

Ok. So I think this is at a state where I want to submit it and do anything else in a follow up PR (sans fixing tests). That way, it hopefully can be merged tomorrow and ready for advent of code on friday.

There are still a number of changes listed in the PR and a suggestion by Ayaz that I want to make, but I want to push that all to follow ups in order to get this in.

Brendan Hansknecht (Nov 30 2023 at 06:50):

Brendan Hansknecht (Nov 30 2023 at 06:52):

@Anton, if you get the chance, can you merge and cut a release for those 2 PRs? Basic CLI being the more important one for advent of code. Should be fine to cut a release before the new roc version is out.

Luke Boswell (Nov 30 2023 at 07:07):

Massive effort Brendan!! This will be such a nice improvement for everyone, even if most people never see this work. :octopus:

Brendan Hansknecht (Nov 30 2023 at 15:57):

Can I get a review on #6116 to submit it? All test are green now (had to disable 2 that have unrelated bugs to be fixed later related to wrong exit codes)

Brendan Hansknecht (Nov 30 2023 at 16:17):

As a note, though the PR is large, it is mostly fluff from platform changes to roc_dbg and roc_panic. The crates/compiler folder being the main changes that matter.

Brendan Hansknecht (Nov 30 2023 at 22:28):

Ok, now we just need to cut new releases of platforms that will be used with AOC such that they have a roc_dbg impl.

Richard Feldman (Nov 30 2023 at 22:28):

Brendan Hansknecht (Nov 30 2023 at 22:31):

This two PRs are both ultra simple if someone wants to review and merge them. For basic-cli and basic-webserver.

Richard Feldman (Nov 30 2023 at 22:34):

also in order to get the new basic-cli into the tutorial, we need to double check that it still works after the breaking Stdin change

Luke Boswell (Nov 30 2023 at 22:34):

If we need anything updated I have time today to do that. Should I test basic-cli with a branch?

Richard Feldman (Nov 30 2023 at 22:35):

Brendan Hansknecht (Nov 30 2023 at 22:37):

With what was just merged and the new compiler, basic-cli should just work with dbg.

Brendan Hansknecht (Nov 30 2023 at 22:38):

So I think we just need to cut a new release (and update the examples in the repo to use the new release).

Brendan Hansknecht (Nov 30 2023 at 22:38):

Luke Boswell (Nov 30 2023 at 22:39):

Luke Boswell (Nov 30 2023 at 22:40):

Brendan Hansknecht (Nov 30 2023 at 22:43):

Brendan Hansknecht (Nov 30 2023 at 22:44):

Luke Boswell (Nov 30 2023 at 22:56):

We have a bug with dbg for opaque types. That should probably be resolved before cutting a new nightly. I don't think this blocks platform release, but the platform releases should stay in testing until we have a new nightly.

Brendan Hansknecht (Nov 30 2023 at 23:07):

So for the opaque bug, it is an issue with not creating a correct specialization when dealing with inspect an opaque value with an automatically derived impl.

Essentially, Inspect.opaque is a * -> Inspector f, but we need to specialize it to the opaque type. So for Utc := ... we want the final version of the function to be Utc -> Inspector f. I think that specialized version is not being generating and we are hitting issues. That is at least my guess.

Brendan Hansknecht (Nov 30 2023 at 23:09):

toInspector = \u ->
    fmt <- Inspect.custom
    Inspect.opaque u
    |> Inspect.apply fmt

This value is a declared specialization of type:

    * -> Inspector f where f implements InspectFormatter

But the type annotation on toInspector says it must match:

    val ->
    Inspector f where val implements Inspect, f implements InspectFormatter

toInspector : Utc -> Inspect.Inspector f where f implements Inspect.InspectFormatter

Brendan Hansknecht (Nov 30 2023 at 23:10):

So maybe I can change the auo derivation to add a type and that will fix the issue.

Brendan Hansknecht (Dec 01 2023 at 03:05):

@Ayaz Hafiz any chance you can take a look at this? Repro is just to try and run dbg on an opaque type that doesn't implement Inspect at all. So something like:

Op := {}

dbg (@Op {})

Brendan Hansknecht (Dec 01 2023 at 03:06):

Ayaz Hafiz (Dec 01 2023 at 03:40):

Luke Boswell (Dec 01 2023 at 03:41):

Ayaz Hafiz (Dec 01 2023 at 03:42):

Luke Boswell (Dec 01 2023 at 03:42):

Ayaz Hafiz (Dec 01 2023 at 03:42):

Ayaz Hafiz (Dec 01 2023 at 03:43):

Does it fail with "expected to know a specialization for #UserApp.Op#Inspect.toInspector, but it wasn't found"?

Luke Boswell (Dec 01 2023 at 03:44):

$ roc dev examples/platform-switching/rocLovesZig.roc
An internal compiler expectation was broken.
This is definitely a compiler bug.
Please file an issue here: https://github.com/roc-lang/roc/issues/new/choose
thread '<unnamed>' panicked at 'expected to know a specialization for `17.IdentId(1)`#`15.IdentId(32)`, but it wasn't found', /Users/luke/Documents/GitHub/roc/crates/compiler/solve/src/specialize.rs:744:33
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Luke Boswell (Dec 01 2023 at 03:44):

# examples/platform-switching/rocLovesZig.roc
app "rocLovesZig"
    packages { pf: "zig-platform/main.roc" }
    imports []
    provides [main] to pf

main =
    dbg (@Op {})

    "Roc <3 Zig!\n"

Op := {}

Luke Boswell (Dec 01 2023 at 03:46):

Ayaz Hafiz (Dec 01 2023 at 04:24):

Brendan Hansknecht (Dec 01 2023 at 07:30):

Brian Carroll (Dec 01 2023 at 07:38):

It's failing CI: Only builtin abilities can be derived.
That suggests it's using the previous version of the compiler?

Luke Boswell (Dec 01 2023 at 07:41):

We need to cut a nightly release before updating the platforms after Ayaz's change.

Brendan Hansknecht (Dec 01 2023 at 07:51):

Anton (Dec 01 2023 at 10:05):

Brendan Hansknecht (Dec 01 2023 at 15:27):

Anton (Dec 01 2023 at 15:54):

Richard Feldman (Dec 01 2023 at 16:53):

yeah it's really great that people today have been able to use it to get unblocked from dbg bugs - huge thanks to both of you for shipping it in time! :smiley:

Pearce Keesling (Dec 01 2023 at 16:57):

I currently don't get any output from dbg when I am running a test. Is there some flag I need to enable for that to happen? The results appear just fine in an actual run

Brendan Hansknecht (Dec 01 2023 at 16:58):

Hmm, I wonder which roc_dbg that hooks into. I guess whichever one the compiler itself exposes. I guess we need to update that.

Anton (Dec 01 2023 at 16:58):

Anton (Dec 01 2023 at 16:59):

I think roc build myfile.roc will still drop dbg like it used to before the new dbg

Pearce Keesling (Dec 01 2023 at 17:04):

Anton (Dec 01 2023 at 17:13):

Pearce Keesling (Dec 01 2023 at 17:17):

Sure thing :thumbs_up: . I do need to verify that I actually understand the issue though. I noticed just now when I was running the test that it marked it as a "warning" which makes me think it is interpreting stderr output as warning on a test?

Anton (Dec 01 2023 at 17:30):

Ayaz Hafiz (Dec 01 2023 at 22:50):

Undefined symbols for architecture arm64:
  "_roc_dbg", referenced from:
      _#UserApp_determineCalibration_3bbacd33228bca14fe5573efe7278cde33c78fe9028ba98810cff368dece in roc_appGIhqSC.o
ld: symbol(s) not found for architecture arm64

Brendan Hansknecht (Dec 01 2023 at 22:54):

Brendan Hansknecht (Dec 02 2023 at 03:23):

So 1 thing I think we missed with dbg that might be quite nice. Do we also want to send the source to the platform. So we send location in the form path:line, the src, and the dbg str. As a result, this:

#App.roc
x = 27
dbg x

[path/App.roc:2] x = 27

Thoughts? The one pain about this is it requires adding one more parameter to all roc_debug impls and it is another breaking change that requires some update shuffling.

Currently, we already have location, so we can get [path/App.roc:2] 27 without the context of the src.

Brendan Hansknecht (Dec 02 2023 at 03:24):

Richard Feldman (Dec 02 2023 at 03:25):

I definitely think it's worth the breaking change assuming we like the idea, although we don't need to rush it out this time :sweat_smile:

Brendan Hansknecht (Dec 02 2023 at 03:29):

Richard Feldman (Dec 02 2023 at 03:35):

Brendan Hansknecht (Dec 02 2023 at 04:22):

[../roc-basic-cli/examples/time.roc:12] (x * 2) = 44

Brendan Hansknecht (Dec 02 2023 at 04:22):

Agus Zubiaga (Dec 02 2023 at 04:28):

This is pretty cool. Question, will this print the entire expression always? What if it’s multiline?

Brendan Hansknecht (Dec 02 2023 at 04:33):

Brendan Hansknecht (Dec 02 2023 at 04:38):

[../roc-basic-cli/examples/time.roc:13] (x
        *
        2) = 44

[crates/compiler/can/src/operator.rs:661] arena.alloc(Loc {
        value: LowLevelDbg(arena.alloc(format!("{}:{}", module_path,
                    line_col.line)), arena.alloc(dbg_src), dbg_str,
            desugared_continuation),
        region: loc_expr.region,
    }) = // Elide for brevty

Brendan Hansknecht (Dec 02 2023 at 04:39):

Anton (Dec 02 2023 at 09:50):

Brendan Hansknecht (Dec 02 2023 at 16:07):

So one thing I realized though haven't done yet....I can technically make this a non breaking change by putting the args in a weird order.

Brendan Hansknecht (Dec 02 2023 at 16:10):

The args are currently (loc, msg) If I make the new args be (loc, msg, src), all of the old functions will just access the first two args and ignore the third. They would have the exact same behaviour as current.

Richard Feldman (Dec 02 2023 at 16:11):

could do that for now and then switch the order to a more logical one when doing the breaking abi change

Brendan Hansknecht (Dec 02 2023 at 16:13):

That said, currently on the branch, the args are in the order (loc, src, msg) cause I like that order better. This could be merged without totally breaking dbg, but with non-updated platforms, the dbg statements would be useless. For:

Brendan Hansknecht (Dec 02 2023 at 21:21):

I think #6143 is ready for review. Should be passing tests now, we'll see if all of CI is green in a bit. @Richard Feldman

Would you prefer for me to reorder all the args to (loc, msg, src) such that current platforms with the old version of roc_dbg just keep working instead of printing out as I mentioned in the comment directly above this? Or should I just leave it as is accepting the mildly breaking change until other platforms are updated?

Richard Feldman (Dec 02 2023 at 21:28):

I think reordering for backwards compatibility is worth it for now, to avoid disrupting advent of code :big_smile:

Stream: contributing

Topic: auto-derived Inspect

Richard Feldman (Nov 26 2023 at 16:40):

Richard Feldman (Nov 26 2023 at 16:40):

Brendan Hansknecht (Nov 26 2023 at 16:51):

Richard Feldman (Nov 26 2023 at 17:07):

Richard Feldman (Nov 26 2023 at 17:36):

Brendan Hansknecht (Nov 26 2023 at 18:16):

Brendan Hansknecht (Nov 28 2023 at 01:49):

Ayaz Hafiz (Nov 28 2023 at 02:15):

Brendan Hansknecht (Nov 28 2023 at 02:17):

Brendan Hansknecht (Nov 28 2023 at 02:25):

Ayaz Hafiz (Nov 28 2023 at 02:38):

Ayaz Hafiz (Nov 28 2023 at 02:39):

Brendan Hansknecht (Nov 28 2023 at 02:41):

Brendan Hansknecht (Nov 28 2023 at 02:42):

Ayaz Hafiz (Nov 28 2023 at 02:44):

Brendan Hansknecht (Nov 28 2023 at 02:44):

Ayaz Hafiz (Nov 28 2023 at 02:45):

Ayaz Hafiz (Nov 28 2023 at 02:45):

Ayaz Hafiz (Nov 28 2023 at 02:46):

Ayaz Hafiz (Nov 28 2023 at 02:46):

Brendan Hansknecht (Nov 28 2023 at 02:47):

Brendan Hansknecht (Nov 28 2023 at 02:48):

Ayaz Hafiz (Nov 28 2023 at 02:48):

Brendan Hansknecht (Nov 28 2023 at 03:36):

Ayaz Hafiz (Nov 28 2023 at 05:52):

Ayaz Hafiz (Nov 28 2023 at 05:52):

Ayaz Hafiz (Nov 28 2023 at 05:58):

Ayaz Hafiz (Nov 28 2023 at 05:58):

Ayaz Hafiz (Nov 28 2023 at 05:59):

Ayaz Hafiz (Nov 28 2023 at 06:00):

Ayaz Hafiz (Nov 28 2023 at 06:01):

Ayaz Hafiz (Nov 28 2023 at 06:02):

Ayaz Hafiz (Nov 28 2023 at 06:03):

Ayaz Hafiz (Nov 28 2023 at 06:03):

Brendan Hansknecht (Nov 28 2023 at 06:04):

Ayaz Hafiz (Nov 28 2023 at 06:05):

Ayaz Hafiz (Nov 28 2023 at 06:06):

Brendan Hansknecht (Nov 28 2023 at 20:13):

Ayaz Hafiz (Nov 28 2023 at 20:14):

Richard Feldman (Nov 28 2023 at 20:16):

Richard Feldman (Nov 28 2023 at 20:17):

Ayaz Hafiz (Nov 28 2023 at 20:17):

Ayaz Hafiz (Nov 28 2023 at 20:23):

Brendan Hansknecht (Nov 28 2023 at 20:50):

Brendan Hansknecht (Nov 28 2023 at 20:55):

Brendan Hansknecht (Nov 28 2023 at 21:04):

Richard Feldman (Nov 28 2023 at 21:05):

Richard Feldman (Nov 28 2023 at 21:05):

Luke Boswell (Nov 28 2023 at 21:09):

Brendan Hansknecht (Nov 28 2023 at 21:17):

Ayaz Hafiz (Nov 28 2023 at 21:50):

Brendan Hansknecht (Nov 28 2023 at 21:56):

Brendan Hansknecht (Nov 28 2023 at 21:57):

Brendan Hansknecht (Nov 28 2023 at 22:03):

Ayaz Hafiz (Nov 28 2023 at 22:22):

Ayaz Hafiz (Nov 28 2023 at 22:25):

Brendan Hansknecht (Nov 28 2023 at 23:02):

Brendan Hansknecht (Nov 28 2023 at 23:03):

Ayaz Hafiz (Nov 28 2023 at 23:14):

Ayaz Hafiz (Nov 28 2023 at 23:15):

Brendan Hansknecht (Nov 28 2023 at 23:16):

Brendan Hansknecht (Nov 28 2023 at 23:22):

Brendan Hansknecht (Nov 28 2023 at 23:30):

Brendan Hansknecht (Nov 29 2023 at 00:15):

Brendan Hansknecht (Nov 29 2023 at 00:39):

Brendan Hansknecht (Nov 29 2023 at 00:48):

Richard Feldman (Nov 29 2023 at 00:49):

Brendan Hansknecht (Nov 29 2023 at 00:52):

Brendan Hansknecht (Nov 29 2023 at 00:52):

Richard Feldman (Nov 29 2023 at 00:53):

Richard Feldman (Nov 29 2023 at 00:54):

Brendan Hansknecht (Nov 29 2023 at 00:57):

Richard Feldman (Nov 29 2023 at 00:58):

Brendan Hansknecht (Nov 29 2023 at 01:10):

Richard Feldman (Nov 29 2023 at 01:13):

Agus Zubiaga (Nov 29 2023 at 01:25):

Agus Zubiaga (Nov 29 2023 at 01:26):

Brendan Hansknecht (Nov 29 2023 at 01:28):