Bug 15839 – [REG2.071-b1] this.outer is of wrong type

Status
RESOLVED
Resolution
FIXED
Severity
regression
Priority
P1
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2016-03-27T10:30:00Z
Last change time
2016-10-01T11:46:52Z
Keywords
pull
Assigned to
nobody
Creator
doob

Comments

Comment #0 by doob — 2016-03-27T10:30:18Z
Compiling the code below will result in the following error: main.d(15): Error: constructor main.GC.this (AnimatedProgress ap) is not callable using argument types (void*) interface Runnable {} class GC { this(AnimatedProgress ap) {} } class AnimatedProgress { void start() { auto r = new class Runnable { void run() { GC gc = new GC(this.outer); } }; } }
Comment #1 by k.hara.pg — 2016-03-27T12:24:25Z
This is an intentional bugfix introduced by fixing issue 14442. Note that there was accepts-invalid bug. The spec page http://dlang.org/spec/class said: > The property .outer used in a nested class gives the this pointer to its enclosing class. If the enclosing context is not a class, the .outer will give the pointer to it as a void* type. It means `this.outer` in `run()` member function should represent the frame of 'start' function. Actually the generated code is following the spec, but front-end had incorrectly typed 'this.outer' like an AnimatedProgress class.
Comment #2 by jack — 2016-03-27T15:17:13Z
If this is the result of a breaking change, then it should be in the changelog.
Comment #3 by doob — 2016-03-27T20:33:56Z
(In reply to Kenji Hara from comment #1) > > The property .outer used in a nested class gives the this pointer to its enclosing class. If the enclosing context is not a class, the .outer will give the pointer to it as a void* type. > > It means `this.outer` in `run()` member function should represent the frame > of 'start' function. Actually the generated code is following the spec, but > front-end had incorrectly typed 'this.outer' like an AnimatedProgress class. So it's not representing the this reference of the outer class anymore? Is it possible to get that behavior somehow? This is a huge regression for DWT.
Comment #4 by k.hara.pg — 2016-03-28T14:06:58Z
(In reply to Jacob Carlborg from comment #3) > So it's not representing the this reference of the outer class anymore? Until 2.070, the builtin .outer property sometimes returned correct class reference, sometimes invalid one. Test following code with 2.070 or earlier. import core.stdc.stdio : printf; interface Runnable {} class GC { this(AnimatedProgress ap) { printf("GC.ctor, ap = %p\n", ap); } } class AnimatedProgress { void start() { printf("start, this = %p\n", this); version(ng) int a; auto r = new class Runnable { void run() { printf("run, this.outer = %p\n", this.outer); GC gc = new GC(this.outer); version(ng) int b = a; } }; r.run(); } } void main() { auto ap = new AnimatedProgress(); printf("main, ap = %p\n", ap); ap.start(); } Without -version=ng, the class reference (instance address) are same, so the code would work as the author's expected. But with -version=ng, the start() member function will become a closure, and this.outer will suddenly return a pointer to the closure environment - of course it's invalid as a class reference. Note that, compiler cannot determine whether the start() makes a closure environment or not at the place where 'this.outer' is used. Because of that, the .outer property should have void* type, to be consistent with the lexical scope nesting. > Is > it possible to get that behavior somehow? > > This is a huge regression for DWT. Theoretically the chain of 'outer' can reach to AnimatedProgress class instance. Actually in my example code, following code can work in run() function with -version=ng case. version(ng) printf("run, *cast(void**)this.outer = %p\n", *cast(void**)this.outer); //GC gc = new GC(this.outer); GC gc = new GC(cast(AnimatedProgress)*cast(void**)this.outer); However... of course it's not stable. And as far as I know, there's no way to know the number of chains up to reach wanting enclosing scope.
Comment #5 by doob — 2016-03-28T19:05:32Z
(In reply to Kenji Hara from comment #4) > However... of course it's not stable. And as far as I know, there's no way > to know the number of chains up to reach wanting enclosing scope. So the best approach would be to pass in the outer reference to the constructor when creating the anonymous class and use that instead of "this.outer"?
Comment #6 by code — 2016-03-29T22:33:31Z
(In reply to Kenji Hara from comment #4) > Note that, compiler cannot determine whether the start() makes a closure > environment or not at the place where 'this.outer' is used. Because of that, > the .outer property should have void* type, to be consistent with the > lexical scope nesting. But this.outer always refers to the enclosing function even if no closure is required, right? A this.outer that is typed as void* and either refers to an enclosing function or an enclosing class would be highly confusing.
Comment #7 by doob — 2016-03-31T08:28:56Z
I tried modifying my code to explicitly pass in the outer this reference. That works until you have nested anonymous classes. For example: interface A {} interface B {} class Foo { void foo() { auto a = new class(this) A { Foo outer; this(Foo outer) { this.outer = outer; this.outer.bar(); auto b = new class(this) B { A outer; this(A outer) { this.outer = outer; this.outer.outer.bar(); // line 20 } }; } }; } void bar() {} } The above will of course fail to compile since there's no "bar" in "A": main.d(20): Error: no property 'outer' for type 'main.A' Any suggestions?
Comment #8 by k.hara.pg — 2016-03-31T15:28:36Z
https://github.com/D-Programming-Language/dmd/pull/5613 If a nested class has a parent class over some nested functions, '.outer' will properly point instance of the parent class.
Comment #9 by doob — 2016-03-31T19:36:46Z
(In reply to Kenji Hara from comment #8) > https://github.com/D-Programming-Language/dmd/pull/5613 > > If a nested class has a parent class over some nested functions, '.outer' > will properly point instance of the parent class. So, this pull request will fix the issues I'm having?
Comment #10 by github-bugzilla — 2016-04-01T10:13:42Z
Commits pushed to stable at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/bb5f550edd77ad780878b8fba89820b001b43554 fix Issue 15839 - this.outer is of wrong type Tweak built-in `.outer` property behavior. If there's some nested functions between a nested class and parent class, `.outer` can point the parent class instance. In other words, the immediate parent function frame of the nested class will be inaccessible by using `.outer`. https://github.com/D-Programming-Language/dmd/commit/1aa8a1e8ebe9893f04d689319dd0606bede83fac Merge pull request #5613 from 9rnsr/fix15839 [REG2.071-b1] Issue 15839 - this.outer is of wrong type
Comment #11 by github-bugzilla — 2016-04-09T06:01:09Z
Comment #12 by github-bugzilla — 2016-05-09T01:53:48Z
Commits pushed to master at https://github.com/dlang/dmd https://github.com/dlang/dmd/commit/a117c87a8a9ffbccc78b2c28ae3643b5e5b02794 Fix invalid AST produced by issue 15839 fix The fix for 15839 (accessing this.outer from a member function inside a nested class) in bb5f550edd7 produces a ThisExp that refers to the 'this' VarDecl in an outer scope, leading to the latter being referenced from the nested scope without that being added to its nestedRefs. Subsequently, the VarDecl is also not present in the outer FuncDecls closureVars. This is a weird construct, and outside the AST invariants that would previously hold. This commit properly at least properly registers the nested reference, but using a ThisExp in this manner might not be the cleanest way in the first place. https://github.com/dlang/dmd/commit/d178c8578e615c3e68702905b72c3e89bccbecd6 Merge pull request #5741 from klickverbot/fixup-15839-ast Fix invalid AST produced by issue 15839 fix
Comment #13 by github-bugzilla — 2016-05-18T08:56:05Z
Commit pushed to master at https://github.com/dlang/dlang.org https://github.com/dlang/dlang.org/commit/a3d804bf84f70b6f6e73cd010c8c6fb7374375c1 Update spec to follow .outer property behavior change introduced in 2.071 See: https://issues.dlang.org/show_bug.cgi?id=15839 Issue 15839 - this.outer is of wrong type
Comment #14 by github-bugzilla — 2016-10-01T11:44:43Z
Commit pushed to stable at https://github.com/dlang/dlang.org https://github.com/dlang/dlang.org/commit/a3d804bf84f70b6f6e73cd010c8c6fb7374375c1 Update spec to follow .outer property behavior change introduced in 2.071
Comment #15 by github-bugzilla — 2016-10-01T11:46:52Z
Commits pushed to stable at https://github.com/dlang/dmd https://github.com/dlang/dmd/commit/a117c87a8a9ffbccc78b2c28ae3643b5e5b02794 Fix invalid AST produced by issue 15839 fix https://github.com/dlang/dmd/commit/d178c8578e615c3e68702905b72c3e89bccbecd6 Merge pull request #5741 from klickverbot/fixup-15839-ast