An Atom is recognized as a Primitive when WithWarnings wrap another WithWarnings

[radeusgd]

Run the following code:

from Standard.Base import all
from Standard.Table import Table, Value_Type
from Standard.Database import Database, SQLite, In_Memory

from Standard.Database.Internal.Database_Type_Helpers import type_helpers

my_are_comparable type_1 type_2 =
    find_bucket typ =
        buckets = [["Integer", "Float", "Decimal"], ["Char"], ["Date"], ["Date_Time"], ["Time"]]
        ctor_name = case Meta.meta typ of
            atom : Meta.Atom ->
                atom.constructor.name
            m ->
                IO.println "This should not happen!"
                IO.println m
                IO.println typ
                #Standard.Base.Runtime.Debug.breakpoint
                Nothing 
        buckets.index_of bucket->
            bucket.contains ctor_name

    bucket_1 = find_bucket type_1
    bucket_2 = find_bucket type_2
    bucket_1 == bucket_2

main =
    c = Database.connect (SQLite In_Memory)
    t0 = Table.new [["X", ["a", "bc", "def"]], ["Y", ["a", "bb", "ddd"]]]
    t1 = c.upload_table "Tabela" t0
    t2 = t1.cast "X" (Value_Type.Char size=1)
    t2.print
    IO.println (Warning.get_all t2).length
    
    x = t2.at "X"
    y = t2.at "Y"
    b = my_are_comparable x.value_type (type_helpers.find_argument_type y)
    IO.println b

The output that I'm seeing is:

 X   | Y  
-----+-----
 a   | a
 bc  | bb
 def | ddd

810
This should not happen!
(Primitive.Value (Char Nothing True))
(Char Nothing True)
False

while what I'd expect would be

 X   | Y  
-----+-----
 a   | a
 bc  | bb
 def | ddd

810
True

For some reason, for x = Value_Type.Char Nothing True - a typical Atom instance, Meta.meta x is yielding a Primitive. This is wrong and is breaking my code.

Meta.meta of an atom should always be Meta.Atom.

[radeusgd]

Note that t2 has warnings attached. So the x and y inherit these warnings.

My first hypothesis was that it's the warnings being attached that breaks the Meta.is_atom check. But apparently, a simpler example I tried:

from Standard.Base import all

type My_Atom
    Value x y z

warn x =
    Warning.attach "WARNING" x

main =
    IO.println <| Meta.meta (My_Atom.Value 1 2 3)
    IO.println <| Meta.meta (warn (My_Atom.Value 1 2 3))

    IO.println <| Meta.meta My_Atom.Value
    IO.println <| Meta.meta (warn My_Atom.Value)

    IO.println <| Meta.meta <| My_Atom
    IO.println <| Meta.meta <| warn <| My_Atom

    IO.println <| Meta.meta <| Error.throw (My_Atom.Value "err" "err" "err")
    IO.println <| Meta.meta <| warn <| Error.throw (My_Atom.Value "err" "err" "err")

Actually worked as it should (all paired lines display the same results).

So it is something more subtle.

Through trial and error, I see that if I replaced type_helpers.find_argument_type y with y.value_type, this goes back to normal. So seemingly, something that happens in type_helpers.find_argument_type may be what is making this value behave weirdly.

[radeusgd]

So, the workaround that just removes warnings from the value before doing Meta.meta does fix the issue I had in my code - it again works OK. So this indeed has something to do with warnings.

However, I don't think the solution is to just add Warning.clear to Meta.meta. It would indeed make it work, but it feels like there is some deeper issue with how our warnings are working and I think we should try to figure this out - as otherwise that weird behaviour may manifest in some completely different places too.

[radeusgd]

Warning that once I implement #5159, this and related repros will stop working - because Char size=1 will start being a valid type, so no Inexact_Type_Coercion will be reported anymore. But the fix is simple - we can switch to numeric columns and request a cast to Value_Type.Float Bits.Bits_32 which won't be supported until #6109 which is not planned very soon.

I was meaning to update these repros right now, but I will not go ahead of myself - once I do #5159 I will revisit these.

[JaroslavTulach]

I am trying to reproduce the problem on 6404332 - if I past the initial program into t.enso and do:

sbt:enso> runEngineDistribution --run t.enso
In module t:
Compiler encountered errors:ibution 3s
t.enso[5:1-5:73]: The module Standard.Database.Internal.Database_Type_Helpers does not exist.
t.enso[36:41-36:52]: The name `type_helpers` could not be found.
Aborting due to 2 errors and 0 warnings.
Execution finished with an error: Compilation aborted due to errors.
        at <java> org.graalvm.sdk/org.graalvm.polyglot.Value.invokeMember(Value.java:971)
        at <java> org.enso.polyglot.Module.getAssociatedType(Module.scala:19)
        at <java> org.enso.runner.Main$.runMain(Main.scala:802)
        at <java> org.enso.runner.Main$.runSingleFile(Main.scala:734)
        at <java> org.enso.runner.Main$.run(Main.scala:616)
        at <java> org.enso.runner.Main$.main(Main.scala:1111)
        at <java> org.enso.runner.Main.main(Main.scala)

Can you merge with recent develop and give me a commit ID to try this upon? Or just give me proper commit ID to checkout.

[radeusgd]

Can you merge with recent develop and give me a commit ID to try this upon? Or just give me proper commit ID to checkout.

Sorry, when writing this issue, I probably assumed my pending PR #6298 will be merged very soon - but I did not anticipate the many other obstacles.

The latest commit on #6298 shall work, but I cannot guarantee that something else will not break it as it's WIP - the safest commit to try this on should be d96d159 - it is done right after I reported this bug and it must have been reproducible at that point. Please let me know if there are any issues with this and I can try to double check the repro if needed.

[JaroslavTulach]

Thanks. I can reproduce the problem now. It is caused by WithWarnings wrapping another WithWarnings:

We should probably avoid such a WithWarnings nesting somewhere. CCing @hubertp. The wrapping currently happens in Table.enso:620. Which is handled by CaseNode:doWarnings:86.

[radeusgd]

Glad the repro works.

We should probably avoid such a WithWarnings nesting somewhere. CCing @hubertp. The wrapping currently happens in Table.enso:620.

Indeed, I'd even propose adding an assert !(value instanceof WithWarnings) to the WithWarnings constructors, what do you think?

or having a similar assert on the result of WarningsLibrary::removeWarnings.

[JaroslavTulach]

adding an assert !(value instanceof WithWarnings) to the WithWarnings constructors

I have it locally on my disk to debug the situation ;-)

[radeusgd]

adding an assert !(value instanceof WithWarnings) to the WithWarnings constructors

I have it locally on my disk to debug the situation ;-)

Then I think it's worth keeping :)

[enso-bot]

Jaroslav Tulach reports a new STANDUP for yesterday (2023-04-18):

Progress: - investigating Warnings: #6258 (comment)

• discussion, planning, processing issues, backlog
• finalizers: Removing need for asynchronous thread to execute ResourceManager finalizers #6335
• review: NaN can be used as a key in Map #6301
• review UnresolvedSymbol is now accepted by Vector.sort #6334 (review) It should be finished by 2023-04-24.

Next Day: Multiple warnings issues

[enso-bot]

Jaroslav Tulach reports a new STANDUP for yesterday (2023-04-19):

Progress: - Avoid double wrapping: #6348

• BookClub: https://discord.com/channels/401396655599124480/1097905211554680874
• standup & 360 review with Pavel
• Explaining Type.to_text: Type.to_text shall invoke instance method #6300 (comment)
• Final ResourceManager improvement: Removing need for asynchronous thread to execute ResourceManager finalizers #6335 (comment) It should be finished by 2023-04-24.

Next Day: <| functions

Discord

Discord - A New Way to Chat with Friends & Communities

Discord is the easiest way to communicate over voice, video, and text. Chat, hang out, and stay close with your friends and communities.