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 ICE (issue 8262). #4025

Closed
wants to merge 2 commits into from
Closed

Fix ICE (issue 8262). #4025

wants to merge 2 commits into from

Conversation

quickfur
Copy link
Member

FIxes: https://issues.dlang.org/show_bug.cgi?id=8262

This PR is still incomplete, as it still doesn't quite implement the alias this in the expected manner (the output of the test program in the bug is S(0) as opposed to 0, which would be the case if the extra level of indirection via Seq!s was removed), and the error appears to be swallowed. At least the compiler won't segfault anymore, but probably this isn't the correct fix for the problem.

@@ -0,0 +1,16 @@
// TBD: define expected output here.
Copy link
Member

Choose a reason for hiding this comment

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

The trouble with putting each issue in its own file is the test suite then takes forever to run. Please add it into one of the existing files, then the incremental testing time is pretty much zero.

Copy link
Member Author

Choose a reason for hiding this comment

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

Where's the best place to put this particular test?

Copy link
Member

Choose a reason for hiding this comment

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

Usually they go in xtest46 if they don't have and special compiler flag requirements. And the test should be reduced to not use phobos.

Copy link
Member Author

Choose a reason for hiding this comment

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

What does the 46 stand for?

Copy link
Member

Choose a reason for hiding this comment

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

Haha who knows. It's the megatest.

Copy link
Member Author

Choose a reason for hiding this comment

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

Shouldn't it be called test42 then?

Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't it be called test42 then?

Actually come to think of it there is a test42 that is also ginormous.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm. There are a bunch of testxxx's that appear to be named after their respective bug numbers. So I looked up issue 42 and 46 on bugzilla... seems to be some random issues long since fixed.

Copy link
Member

Choose a reason for hiding this comment

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

Those test files most likely predate bugzilla.

@WalterBright
Copy link
Member

I suspect that if the alias resolves to a tuple, then the tuple must be one element, and it is resolved to that one element, otherwise an error.

@quickfur
Copy link
Member Author

Righto, I'll take a look in that direction. Thanks for the feedback!

@quickfur quickfur force-pushed the issue8262 branch 2 times, most recently from 0d50ae8 to 9a4e884 Compare September 26, 2014 20:01
@quickfur
Copy link
Member Author

quickfur commented Oct 7, 2014

Don't have time to work on this right now... plus, this will mostly be superceded by the multiple alias this PR, so I'm closing this for now.

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