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 Issue 19954 - ICE: Casting AliasSeq to array and passing to a function #10036

Merged
merged 1 commit into from Jun 15, 2019
Merged

Fix Issue 19954 - ICE: Casting AliasSeq to array and passing to a function #10036

merged 1 commit into from Jun 15, 2019

Conversation

JinShil
Copy link
Contributor

@JinShil JinShil commented Jun 14, 2019

Best viewed without whitespace changes.

@thewilsonator
Copy link
Contributor

Stable?

@JinShil JinShil changed the base branch from master to stable June 14, 2019 13:32
@dlang-bot
Copy link
Contributor

dlang-bot commented Jun 14, 2019

Thanks for your pull request and interest in making D better, @JinShil! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the annotated coverage diff directly on GitHub with CodeCov's browser extension
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close Bugzilla Severity Description
19954 normal ICE: Casting AliasSeq to array and passing to a function

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "stable + dmd#10036"

@JinShil JinShil changed the base branch from stable to master June 14, 2019 13:34
@JinShil JinShil requested a review from Geod24 as a code owner June 14, 2019 13:48
@JinShil JinShil changed the base branch from master to stable June 14, 2019 13:48
@JinShil
Copy link
Contributor Author

JinShil commented Jun 14, 2019

rebased to stable


void main()
{
auto a = cast(char[][]) "";
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this really compile? auto a = cast(int[]) 1; doesn't.

Copy link
Contributor Author

@JinShil JinShil Jun 14, 2019

Choose a reason for hiding this comment

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

According to this (run "All dmd com"), yes. But I think "" is more analogous to [] than to 1, and auto a = cast(int[])[]; compiles.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, a regression. cast(void[][]) [] does compile too, so all good.

@jacob-carlborg
Copy link
Contributor

Should there be a test that actually casts an AliasSeq to an array, as described in the issue?

@ghost
Copy link

ghost commented Jun 14, 2019

Yes please add the test with an alias Seq. If that does not pass then you can add this:

+        if (exp.e1.type.ty == Ttuple)
+        {
+            TupleExp te = exp.e1.isTupleExp();
+            if (te.exps.dim == 1)
+                exp.e1 = (*te.exps)[0];
+        }

around line 6650, just after you're sure that exp.e1.type is a thing.
Theorically tuples with not one element are rejected when implicit conversion is tried.

@JinShil
Copy link
Contributor Author

JinShil commented Jun 15, 2019

Should there be a test that actually casts an AliasSeq to an array, as described in the issue?

Yes, good call. It didn't pass.

If that does pass then you can add this

Thanks, that worked.

@dlang-bot dlang-bot merged commit 385c1d4 into dlang:stable Jun 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants