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

Remove OVERLAP_SVEC_LEN #12288

Merged
merged 1 commit into from
Jul 24, 2015
Merged

Remove OVERLAP_SVEC_LEN #12288

merged 1 commit into from
Jul 24, 2015

Conversation

yuyichao
Copy link
Contributor

Apparently no one have ever tried it after #10380 because it has been broken since then.

I think we also don't need this risky saving anymore since we don't use SimpleVector all over the place anymore and only for certain internal stuff.

@carnaval
Copy link
Contributor

agreed, there is little point keeping broken code around, maybe only for nostalgia but there is the whole git history for that. I hope that !copy_stacks wont have the same fate though.

@yuyichao
Copy link
Contributor Author

My impression was that it was useful when it was still OVERLAP_TUPLE_LEN. However, after #10380, each tuple have their own types and it isnoe necesary for SimpleVector

@carnaval
Copy link
Contributor

well, useful maybe but did anyone ever build it that way ? Given that we find bugs in the default set of option all the time, I would be quite surprised if a change to the internal representation like that, almost never tested, was even barely functioning. Maybe I'm just pessimistic.

JeffBezanson added a commit that referenced this pull request Jul 24, 2015
@JeffBezanson JeffBezanson merged commit fef1dff into master Jul 24, 2015
@yuyichao yuyichao deleted the yyc/remove-overlap-svec-length branch July 24, 2015 20:51
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