Skip to content

Comments

Fix ICE (issue 8262).#4025

Closed
quickfur wants to merge 2 commits intodlang:masterfrom
quickfur:issue8262
Closed

Fix ICE (issue 8262).#4025
quickfur wants to merge 2 commits intodlang:masterfrom
quickfur:issue8262

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.

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
Contributor

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
Contributor

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
Contributor

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
Contributor

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

Development

Successfully merging this pull request may close these issues.

3 participants