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

Issue 10341 - ICE on case-range statement without an associated switch statement #2167

Merged
merged 1 commit into from Jun 13, 2013
Merged

Issue 10341 - ICE on case-range statement without an associated switch statement #2167

merged 1 commit into from Jun 13, 2013

Conversation

ghost
Copy link

@ghost ghost commented Jun 12, 2013

if (sw == NULL)
{
error("case range not in switch statement");
return statement;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't seem to return this here, it just segfaults at a different location. For expressions I can usually return new ErrorExp(), but we don't seem to have an equivalent for statements?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I plan to eventually create an ErrorStatement, but haven't done so yet.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could possibly use an empty CompoundStatement as a no-op statement.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like so?:

if (sw == NULL)
{
    error("case range not in switch statement");
    return new CompoundStatement(loc, new Statements());
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so. Alternatively, some statements return NULL if they are removed by semantic, like static ifs/asserts that are not triggered. This should work at least in theory.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returning NULL seems to work too. I'll give it a go and see what the tester says.

@9rnsr
Copy link
Contributor

9rnsr commented Jun 13, 2013

LGTM.

9rnsr added a commit that referenced this pull request Jun 13, 2013
Issue 10341 - ICE on case-range statement without an associated switch statement
@9rnsr 9rnsr merged commit 384d833 into dlang:master Jun 13, 2013
@9rnsr
Copy link
Contributor

9rnsr commented Jun 13, 2013

Mmm, Currently, the following error has occurred frequently on Linux platform in Auto-tester.

Creating output directory: test_results
Building d_do_test tool
OS: posix
Error: cannot find source code for runtime library file 'object.d'
       dmd might not be correctly installed. Run 'dmd -man' for installation instructions.
Specify path to file 'object.d' with -I switch
make: *** [test_results/d_do_test] Error 1
Building d_do_test tool
OS: posix
Error: cannot find source code for runtime library file 'object.d'
       dmd might not be correctly installed. Run 'dmd -man' for installation instructions.
Specify path to file 'object.d' with -I switch
make: *** [test_results/d_do_test] Error 1

I'm really not sure what's wrong, but from the circumstance merging this pull request is the most doubtful.
@AndrejMitrovic, sorry but I'll revert this change from master branch.

9rnsr added a commit that referenced this pull request Jun 13, 2013
This reverts commit 384d833, reversing
changes made to af70ceb.

-----

Currently auto-tester has random failures in Linux platform. From the circumstance, this change is the most doubtful. So I revert it conservatively.
@9rnsr
Copy link
Contributor

9rnsr commented Jun 13, 2013

The snapshot of failures which had been displayed by auto tester:
failures

@ghost
Copy link
Author

ghost commented Jun 13, 2013

@9rnsr I see no relationship between case range statement and dmd's inability to find object.d. You have reverted the pull, but failures still occur.

@9rnsr
Copy link
Contributor

9rnsr commented Jun 13, 2013

@mxfm Right...

@ghost
Copy link
Author

ghost commented Jun 13, 2013

@9rnsr: Could this be the problem?: cf03d8e

I didn't think about MSYS when I implemented that pull.

@braddr
Copy link
Member

braddr commented Jun 13, 2013

Sorry abut those tester errors.. entirely my fault. Ignore them. I've deprecated all the tests from that build host so those tests are being re-run.

@ghost
Copy link
Author

ghost commented Jun 13, 2013

@9rnsr: Should I reopen this pull?

9rnsr added a commit that referenced this pull request Jun 14, 2013
Issue 10341 - ICE on case-range statement without an associated switch statement

-----

I confirmed that the change was entirely unrelated to the auto-tester failures. So I re-merge this.
@9rnsr
Copy link
Contributor

9rnsr commented Jun 14, 2013

@braddr Thanks, I confirmed.
@AndrejMitrovic Unfortunately, we cannot re-open the pull request which has once merged. I pushed the merge commit to upstream once again (095b58a).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants