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 14889 & 14900 & [REG2.068.0] 14911 - Reimplement fix for issue 1215 #4876

Closed
wants to merge 5 commits into from

Conversation

9rnsr
Copy link
Contributor

@9rnsr 9rnsr commented Aug 10, 2015

Issue 14889 - ICE: Assertion `o->dyncast() == DYNCAST_DSYMBOL' failed.
Issue 14900 - 2.068.0 change log example does not compile
Issue 14911 - Compiler found indexing in code "new MyStruct[2].ptr"

It was implemented in #4516, but unfortunately the we didn't get enough chance to test the new feature.

@9rnsr
Copy link
Contributor Author

9rnsr commented Aug 10, 2015

I've just started working yesterday for issue 14889, so this change would have some risk (in particular the refactoring in TypeQualified::resolveHelper is dangerous against stability). I'll continue to verify this change, but I'm not sure it should go into stable branch.

@MartinNowak and @WalterBright How do you think about issue 14900?

@MartinNowak
Copy link
Member

@MartinNowak and @WalterBright How do you think about issue 14900?

Let's change the changelog for now and then we'll see how to complete the feature.

@9rnsr 9rnsr force-pushed the fix14889 branch 2 times, most recently from 9184736 to b01b687 Compare August 11, 2015 07:39
@9rnsr
Copy link
Contributor Author

9rnsr commented Aug 11, 2015

OK, I removed excessive refactoring in TypeQualified::resolveHelper. Ready to review.

@9rnsr 9rnsr changed the title Issue 14889 & 14900 - Reimplement fix for issue 1215 Issue 14889 & 14900 & [REG2.068.0] 14911 - Reimplement fix for issue 1215 Aug 13, 2015
@9rnsr
Copy link
Contributor Author

9rnsr commented Aug 13, 2015

I append one more commit for the the 2.068 regression 14911. It's a workaround for the parser.

@schveiguy
Copy link
Member

Not able to pull/review, but shouldn't this target stable?

@9rnsr
Copy link
Contributor Author

9rnsr commented Aug 14, 2015

I don't know. master branch will be converted to ddmd, so I'm not sure the stable branch will be released.

@schveiguy
Copy link
Member

It's a good question, @MartinNowak what happens to stable when ddmd is applied to the master? It seems like we will have to have at least one release where stable changes must target both master and stable, one in C++, one in D.

@MartinNowak
Copy link
Member

stable changes should still only address the stable branch, i.e. they'll be written in C++.
The conversion has to happen when merging into master as with any other merge conflict, hopefully as automated as possible.

@MartinNowak
Copy link
Member

I'm not sure the stable branch will be released.

Yes, the stable branch will be used for 2.068.1 in about 3 weeks.

@MartinNowak
Copy link
Member

LGTM, though it's a pretty big change for this tiny feature.
Please rebase+reopen against stable.

@9rnsr
Copy link
Contributor Author

9rnsr commented Aug 22, 2015

@MartinNowak See #4918.

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