Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert the DotVarExp checkPurity check #243

Merged
merged 3 commits into from
Aug 2, 2011
Merged

Revert the DotVarExp checkPurity check #243

merged 3 commits into from
Aug 2, 2011

Conversation

kennytm
Copy link
Contributor

@kennytm kennytm commented Jul 12, 2011

This commit effectively reverts commit 84b4fd1, which adds the check for DotVarExp whether the v in e1.v is accessible from a pure function. This is entirely unnecessary, because e1 is also semantic()-ed, thus purity-checked, and therefore the member v must also be accessible.

This is a more proper solution of bug 6230, and also solves bug 6284 (due to the implicit (*__withSym).a), bug 6293 and other bugs where ethis is not a VarExp or DotVarExp. This also supersedes pull #242 (bug 6295) which is introduced by pull #222.

kennytm added 3 commits July 13, 2011 03:27
…cting weak purity.

This commit effectively reverts commit 84b4fd1, which adds the check for DotVarExp whether
the `v` in `e1.v` is accessible from a pure function. This is entirely unnecessary, because
`e1` is also `semantic()`-ed, thus purity-checked, and because of weak-purity the member `v`
must also be accessible.

This is a more proper solution of bug 6230, and also solves bug 6284 (due to the implicit
`(*__withSym).a`), bug 6293 and other bugs where `ethis` is not a VarExp or DotVarExp. This
also supersedes pull #242 (bug 6295) which is introduced by pull #222.
WalterBright added a commit that referenced this pull request Aug 2, 2011
@WalterBright WalterBright merged commit 1dac08b into dlang:master Aug 2, 2011
@braddr
Copy link
Member

braddr commented Aug 2, 2011

One of the new tests is failing on all platforms when building with -O.

Error: variable x0 used before set

Two issues, both dmd bugs:

  1. no line number!
  2. x0 is set before it's used, so, um.. say what?

@kennytm
Copy link
Contributor Author

kennytm commented Aug 2, 2011

@braddr: This is a bug in the back-end. A reduced test case is:

void test6293() {
    int x;
    int y = *(1 + &x);
}
$ dmd -O -c xtest46.d
Error: variable x used before set

The back-trace on Linux x64 is

#0  verror(Loc, const char *, typedef __va_list_tag __va_list_tag *) (loc=..., format=0x54d838 "variable %s used before set", ap=0x7fffffffdba0) at mars.c:159
#1  0x00000000004afa4b in error (filename=0x0, linnum=0, format=<value optimized out>) at mars.c:146
#2  0x000000000049be77 in chkrd (n=0x7e41dc, rdlist=<value optimized out>) at backend/gother.c:428
#3  0x000000000049c51b in conpropwalk (n=0x7e41dc, IN=0x84bd08) at backend/gother.c:342
#4  0x000000000049c384 in conpropwalk (n=0x7e435c, IN=0x84bd08) at backend/gother.c:230
#5  0x000000000049c4f7 in conpropwalk (n=0x7e43bc, IN=0x84bd08) at backend/gother.c:273
#6  0x000000000049c6df in rd_compute () at backend/gother.c:169
#7  constprop () at backend/gother.c:109
#8  0x000000000049ad05 in optfunc () at backend/go.c:293
#9  0x00000000004ccd0c in writefunc2 (sfunc=0x7e39c8) at backend/out.c:1177
#10 writefunc (sfunc=0x7e39c8) at backend/out.c:913
#11 0x0000000000498d1d in FuncDeclaration::toObjFile (this=<value optimized out>, multiobj=<value optimized out>) at glue.c:1026
#12 0x000000000049995b in Module::genobjfile (this=0x7e67c0, multiobj=0) at glue.c:279
#13 0x00000000004b101c in main (argc=7, argv=0x7d78b0) at mars.c:1320

This is actually bug 5790, but seems only triggered on Linux and FreeBSD.

@WalterBright
Copy link
Member

This is not a regression. It exists in older dmc compilers, too. It is also a correct error message.

@kennytm
Copy link
Contributor Author

kennytm commented Aug 2, 2011

@braddr: I think the tester can pass by commenting out line 3365:

    assert(x0 is (p1 - 1).token);

@braddr
Copy link
Member

braddr commented Aug 2, 2011

The reduced test case doesn't match the currently failing test case though, since it's removed the setting of the value. The checked in code is setting the value.

@kennytm
Copy link
Contributor Author

kennytm commented Aug 2, 2011

@braddr: The test case is reduced from

auto x0 = a[0].token;
auto p1 = &x0 + 1;
assert(x0 is (p1 - 1).token);

The error is raised from the *(p1 - 1) in the assert, which the cause is dereferencing a pointer with a nonzero offset. Is it possible that this code has hit an undefined behavior?

@braddr
Copy link
Member

braddr commented Aug 3, 2011

There's a bug report I made some time ago that's related:
http://d.puremagic.com/issues/show_bug.cgi?id=5239

@braddr
Copy link
Member

braddr commented Aug 3, 2011

So, another unexplained anomaly.. osx32 and win32 pass. linux* and freebsd* fail.

Regardless.. forwards with disabling this test or backwards by reverting the checkin. Staying where we are is unacceptable.

@braddr
Copy link
Member

braddr commented Aug 3, 2011

or (of course) fixing the underlying bug(s)

@kennytm
Copy link
Contributor Author

kennytm commented Aug 3, 2011

@braddr: Of course I prefer fix the bug or disable the test. The cause of the bug has nothing related to this pull request. The test only checks whether BinaryExp can appear as e1 of a DotVarExp in a (weakly) pure function.

@WalterBright
Copy link
Member

fixed

@braddr
Copy link
Member

braddr commented Aug 3, 2011

On Wednesday, August 03, 2011 9:54:34 AM, WalterBright wrote:

fixed

Nice.. the best solution. I'll close my pull request that disables the
test.

braddr pushed a commit to braddr/dmd that referenced this pull request Sep 15, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants