These are chat archives for opal/opal

3rd
Jun 2018
Jan Biedermann
@janbiedermann
Jun 03 2018 07:41

@iliabylich on master a

module Beauty
  class Flower

and a

Beauty::Flower.new.name

prints Flower, and not Beauty::Flower
$$full_name is set in this line:
Object.setPrototypeOf(klass, Opal.Class.prototype);
https://github.com/opal/opal/blob/a7f2efc0b05dd9fe24527e33f0cea10077750623/opal/corelib/runtime.js#L431
but at his time $$base_module is not set, thats why its missing from $$full_name
I dont understand why setPrototypeOf sets $$full_name, whats happening here?

Jan Biedermann
@janbiedermann
Jun 03 2018 08:07
or @elia
Jan Biedermann
@janbiedermann
Jun 03 2018 08:19
that should be above Beauty::Flower.name
or Beauty::Flower.new.class.name
Jan Biedermann
@janbiedermann
Jun 03 2018 08:56
actually it appears i was mislead by the chrome debugger, so above setPrototypeOf may not be the cause, but still the problem is there
Jan Biedermann
@janbiedermann
Jun 03 2018 09:40

so i correct myself:
$$full_name is set here:
https://github.com/opal/opal/blob/a7f2efc0b05dd9fe24527e33f0cea10077750623/opal/corelib/runtime.js#L510

when i add
delete klass.$$full_name; after that, i get correct values.

Elia Schito
@elia
Jun 03 2018 10:21
@janbiedermann I'm not sure I understood, here's my attempt to reproduce:
opal:master ⤑ bin/opal -e 'class A; class B; end; end; p A::B.name'
"A::B"
opal:master ⤑ bin/opal -e 'class A; class B; end; end; p A::B.new.class.name'
"A::B"
@mistergibson I'll try 0.11.1 on my prod app and if everything runs smoothly I'll release
Jan Biedermann
@janbiedermann
Jun 03 2018 10:43
your test case works for me, but in browsers i get just B, but not for all classes
i dont mean to block the relase
Jan Biedermann
@janbiedermann
Jun 03 2018 11:49
i just tried myself with master, it brakes a hyperloop app, same symptom
works with 0.11, brakes with master, no other changes
this may well be a hyperloop issue, i was just unable to pinpoint it so far
Jan Biedermann
@janbiedermann
Jun 03 2018 15:18

2 issues i have here with master:

  1. $$full_name sometimes gets set before $$base_module is set, thus is incorrect
  2. Opal::Is::Cool() behavour has changed.

both brake hyperloop, seems like hyperloop has to adapt to 2.
why 1. happens is still not clear to me

Jan Biedermann
@janbiedermann
Jun 03 2018 17:53

@elia
when i change the end of Object.klass = function etc. in runtime.js to look like this, everything works:

    if (bridged) {
      Opal.bridge(bridged);
      klass = bridged;
      Opal.const_set(scope, name, klass);
    } else {
      // Create the class object (instance of Class)
      klass = Opal.allocate_class(name, superclass, constructor);
      Opal.const_set(scope, name, klass);
      // Call .inherited() hook with new class on the superclass
      if (superclass.$inherited) {
        superclass.$inherited(klass);
      }
    }

    return klass;

  }

i moved the const_set above the inherited

Jan Biedermann
@janbiedermann
Jun 03 2018 18:11
this change fixes 1. and 2.
Elia Schito
@elia
Jun 03 2018 21:00
@janbiedermann Interesting, I'll check if MRI has the constant already set when inherited is called, if yes that's perfect, let me know if you prefer to send a PR, otherwise I'll do it directly
G. Gibson
@mistergibson
Jun 03 2018 21:23
@elia and @janbiedermann : thank you for pushing so hard to put in the finishing touches to 11.1 -- I look forward to coding on it. Cheers :beer:
Elia Schito
@elia
Jun 03 2018 21:24
@janbiedermann looks like that fix is good as it is:
>> class A; def self.inherited(b); p(A::B) == p(b); end; end
=> :inherited
>> class A::B < A; end
A::B
A::B
=> nil
>>
Jan Biedermann
@janbiedermann
Jun 03 2018 21:25
pr is already there
Elia Schito
@elia
Jun 03 2018 21:26
:+1:
@mistergibson just to be clear, 0.11.1 is not directly off master, but rather 0.11.0 plus some backports, master will end up in 1.0 which will take some more time I think
G. Gibson
@mistergibson
Jun 03 2018 21:28
Ah ok - no worries.