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

Fix bug 6451 #655

Merged
merged 1 commit into from
Jan 29, 2012
Merged

Fix bug 6451 #655

merged 1 commit into from
Jan 29, 2012

Conversation

mrmonday
Copy link
Contributor

Fix bug 6451 - [64bit] ICE(expression.c:4434): SymbolExp::SymbolExp(Loc,... TOK, int, Declaration*, int): Assertion `var' failed.

…oc, TOK, int, Declaration*, int): Assertion `var' failed.
@WalterBright
Copy link
Member

In the future, please include additions to the test suite when fixing bugs. In this case, the example from the bug report:

void error(...){}

should be written into a file named fail6451.d and added to the directory dmd/test/fail_compilation.

WalterBright added a commit that referenced this pull request Jan 29, 2012
@WalterBright WalterBright merged commit 425a97c into dlang:master Jan 29, 2012
@mrmonday
Copy link
Contributor Author

I wasn't sure how to test this, for several reasons:

  • void error(...){} compiles just fine on 32bit, it only errors on x64 (I guess version could handle that)
  • Testing at as fail compilation doesn't check that there's no ICE

Perhaps something like the following would work (untested):

version (X86_32)
{
    void error(...) {}
}
else version (X86_64)
{
    static assert(!is(typeof(void error(...) {})));
    import core.vararg;
    static assert(is(typeof(void error(...) {})));
}

Though this still doesn't check for what this pull request fixes (the ICE).

@WalterBright
Copy link
Member

Testing does check for seg faults, and those are test failures. The fail tests are supposed to return an exit value of 1, not a seg fault.

Please add your revised test as a pull request.

braddr pushed a commit to braddr/dmd that referenced this pull request Oct 22, 2012
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.

2 participants