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

Move setindex! with Colon from array.jl to arraymath.jl #11678

Merged
merged 2 commits into from
Jun 12, 2015

Conversation

tkelman
Copy link
Contributor

@tkelman tkelman commented Jun 12, 2015

Reverts #10331

Ignore the branch name, I think this should be a fix for #10331 (comment)

@tkelman tkelman force-pushed the revert-10331-mb/colon-indexing branch from 7dd9716 to 9b24adc Compare June 12, 2015 11:19
hopefully fixes win64 inference freeze
@tkelman tkelman force-pushed the revert-10331-mb/colon-indexing branch from 9b24adc to 2fe0d94 Compare June 12, 2015 11:57
@tkelman tkelman changed the title Revert "RFC: Move Colon translation out of the parser" Move setindex! with Colon from array.jl to arraymath.jl Jun 12, 2015
@mbauman
Copy link
Member

mbauman commented Jun 12, 2015

👍 I like this solution even better. You can totally delete those setindex! methods instead of moving them.

Sorry about the failures -- I did double check that things passed locally, so I thought we were good.

@tkelman
Copy link
Contributor Author

tkelman commented Jun 12, 2015

You can totally delete those setindex! methods instead of moving them.

Thanks. So these get defined elsewhere?

LLVM 3.3 seems to be especially buggy on win32 and win64, and changes to core bits and pieces have a tendency to find those bugs.

@mbauman
Copy link
Member

mbauman commented Jun 12, 2015

Yeah, they're completely redundant with the abstract fallbacks in multidimensional.jl.

@tkelman
Copy link
Contributor Author

tkelman commented Jun 12, 2015

Even better, that'll make this a net deletion of code then, and in the core dependencies of inference too. I'll push that and merge as soon as my tests pass locally.

@mbauman
Copy link
Member

mbauman commented Jun 12, 2015

Thank you Tony!

tkelman added a commit that referenced this pull request Jun 12, 2015
Move setindex! with Colon from array.jl to arraymath.jl
@tkelman tkelman merged commit 064e96b into master Jun 12, 2015
@tkelman tkelman deleted the revert-10331-mb/colon-indexing branch June 12, 2015 12:53
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.

2 participants