1380990 - Return a more descriptive error message than "|this| used uninitialized in ... constructor"

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P3)

Description

[Soledad Penades [:sole] [:spenades]]

If you try to use this in a class that inherits from another before calling super, you get a somewhat obscure error: "|this| used uninitialized in [insert class name here] constructor"

This caught me off guard and confused me to no end for a while.

Perhaps we should add something like "Please call super() before accessing |this| on a constructor"?

Examples -- the following causes the error.

```
class A extends B {
  constructor() {
    this.someVariable = 'some value'; // fails
  }
}
```

and the following doesn't.

```
class A extends B {
  constructor() {
    super(); // ☜☜☜ ❗️❗️❗️
    this.someVariable = 'some value'; // works!
  }
}
```

Comment 1

[Jan de Mooij [:jandem]]

FWIW in Safari I get: ReferenceError: Cannot access uninitialized variable.

And Chrome: ReferenceError: Must call super constructor in derived class before accessing 'this' or returning from derived constructor

Comment 2

[Masaya Hayashi]

I'm working on this!

Comment 3

[Masaya Hayashi]

Attachment: Improve error message for accessing this of uninitialized

When you execute following code, you will get a confusing error message saying "|this| used uninitialized in A class constructor".
In this patch, you will get a clearer one saying "Must call super constructor before using |this| in A class constructor".

> class B {};
> 
> class A extends B {
>   constructor() {
>     // You need to call super(); here.
>     this.someVariable = 'some value'; // fails
>   }
> }
> 
> new A();

Also, when you execute following code, you will get a confusing error message saying "|this| used uninitialized in arrow function in class constructor".
In this patch, you will get a clearer one saying "Must call super constructor before using |this| in arrow function in class constructor".

> class B {};
> 
> class A extends B {
>   constructor() {
>     // You need to call super(); here.
>     (() => {
>         this.someVariable = 'some value'
>     })()
>   }
> }
> 
> new A();

Comment 4

[Tooru Fujisawa [:arai]]

Comment on attachment 8924424 [details] [diff] [review]
Improve error message for accessing this of uninitialized

Review of attachment 8924424 [details] [diff] [review]:
-----------------------------------------------------------------

Thank you for your patch :)

::: js/src/js.msg
@@ +102,4 @@
>  MSG_DEF(JSMSG_INVALID_ARG_TYPE,        3, JSEXN_TYPEERR, "Invalid type: {0} can't be a{1} {2}")
>  MSG_DEF(JSMSG_TERMINATED,              1, JSEXN_ERR, "Script terminated by timeout at:\n{0}")
>  MSG_DEF(JSMSG_CANT_CALL_CLASS_CONSTRUCTOR, 0, JSEXN_TYPEERR, "class constructors must be invoked with |new|")
> +MSG_DEF(JSMSG_UNINITIALIZED_THIS,      1, JSEXN_REFERENCEERR, "Must call super constructor before using |this| in {0} class constructor")

can you start with lowercase "m"?
most other error messages start with lowercase, so it should be better trying to keep consistent
(if we should start with upper case, it might be nice to file a dedicated bug for it)

@@ +102,5 @@
>  MSG_DEF(JSMSG_INVALID_ARG_TYPE,        3, JSEXN_TYPEERR, "Invalid type: {0} can't be a{1} {2}")
>  MSG_DEF(JSMSG_TERMINATED,              1, JSEXN_ERR, "Script terminated by timeout at:\n{0}")
>  MSG_DEF(JSMSG_CANT_CALL_CLASS_CONSTRUCTOR, 0, JSEXN_TYPEERR, "class constructors must be invoked with |new|")
> +MSG_DEF(JSMSG_UNINITIALIZED_THIS,      1, JSEXN_REFERENCEERR, "Must call super constructor before using |this| in {0} class constructor")
> +MSG_DEF(JSMSG_UNINITIALIZED_THIS_ARROW, 0, JSEXN_REFERENCEERR, "Must call super constructor before using |this| in arrow function in class constructor")

"... in derived class constructor", so that it's clear that the arrow function is inside derived class constructor

Comment 5

[Masaya Hayashi]

Attachment: Improve error message for accessing this of uninitialized

Thank you for your review!
I modified error messages.

Comment 6

[Tooru Fujisawa [:arai]]

Comment on attachment 8924432 [details] [diff] [review]
Improve error message for accessing this of uninitialized

Review of attachment 8924432 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks again!

here's try run.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=46109b6a267136e28e46925dbae4061d3be50acb

Comment 7

[Masaya Hayashi]

Attachment: Improve error message for accessing this of uninitialized

I also modified jit test for error message which is changed in this patch.

Comment 8

[Tooru Fujisawa [:arai]]

Comment on attachment 8924516 [details] [diff] [review]
Improve error message for accessing this of uninitialized

Review of attachment 8924516 [details] [diff] [review]:
-----------------------------------------------------------------

thanks!

here's another try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=789e306865f73c75f9df104a499d8e0934a13002

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fdb23b22f19a
Improve error message for accessing this of uninitialized. r=arai

Comment 10

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/fdb23b22f19a