fix improper determination of 'return this' status#6330
fix improper determination of 'return this' status#6330dlang-bot merged 1 commit intodlang:scopefrom
Conversation
| struct Foo12 | ||
| { | ||
| int* foo() return @safe; | ||
| } |
There was a problem hiding this comment.
This is not the test case I provided. The original test case was
int* escape () @safe
{
int i;
Foo f;
f.v = &i;
return f.foo;
}
struct Foo
{
int* v;
int* foo () @safe { return this.v; }
}And still compiles with this P.R.
There was a problem hiding this comment.
Well, that was fixed by #6331
The trouble with stacking these PRs is if I don't do it this way there are tedious merge conflicts, especially since the line numbers have to match up.
|
|
||
| int* escape13() @safe | ||
| { | ||
| scope Foo13 f; |
There was a problem hiding this comment.
I would hold off on any scope class test before the fix in stable gets merged into scope. @MartinNowak ?
WalterBright
force-pushed
the
return-this
branch
from
809ffa4 to
25400f0
Compare
WalterBright
force-pushed
the
return-this
branch
2 times, most recently
from
671e642 to
a2cdddf
Compare
|
This is very strange. The error produced is compiling Phobos unittests: which does not change even if I disable the changes in this PR. |
|
#6331 seems to be the culprit. |
WalterBright
force-pushed
the
return-this
branch
3 times, most recently
from
cd21839 to
bf5e0ee
Compare
|
@MartinNowak ready to rock! |
MartinNowak
left a comment
MartinNowak
left a comment
There was a problem hiding this comment.
I'll do another rebase, things got messed up a bit.
| if (global.params.vsafe && tf.isreturn && fd && (ad = fd.isThis()) !is null) | ||
| { | ||
| if (ad.isClassDeclaration()) // this is 'return scope' | ||
| dve.e1.accept(this); |
There was a problem hiding this comment.
Coverage missing, please add a test case.
Also see https://github.com/codecov/browser-extension.
There was a problem hiding this comment.
@WalterBright you may want to install the CodeCov Chrome extension, it nicely shows uncovered lines in red on this page!
| --- | ||
| */ | ||
|
|
||
| #line 1000 |
There was a problem hiding this comment.
It rather seems like a good idea to make tests position independent, what's the problem with it?
test/fail_compilation/retscope.d
Outdated
| --- | ||
| */ | ||
|
|
||
| #line 1000 |
There was a problem hiding this comment.
Should be deleted now if you fixed the test.
| if (global.params.vsafe && tf.isreturn && fd && (ad = fd.isThis()) !is null) | ||
| { | ||
| if (ad.isClassDeclaration()) // this is 'return scope' | ||
| dve.e1.accept(this); |
There was a problem hiding this comment.
@WalterBright you may want to install the CodeCov Chrome extension, it nicely shows uncovered lines in red on this page!
MartinNowak
force-pushed
the
return-this
branch
from
bf5e0ee to
fc12c64
Compare
|
Just removed the noise from the weird rebase, now it's clearer what this PR actually does. |
|
Don't try this at home kids, we're still testing our new auto-merge feature. |
|
@MartinNowak COOL COOL COOL! |
5 participants
No description provided.