Ruby Issue Tracking System: Issueshttps://redmine.ruby-lang.org/https://redmine.ruby-lang.org/favicon.ico?17113305112011-09-09T23:38:32ZRuby Issue Tracking System
Redmine Ruby master - Bug #5303 (Closed): parse.y relies on $$ = $1 before action routines [PATCH]https://redmine.ruby-lang.org/issues/53032011-09-09T23:38:32Zadgar (Michael Edgar)michael.j.edgar@dartmouth.edu
<p>In perusing parse.y, I came across this pair of action routines:</p>
<p>bv_decls : bvar<br>
/<em>%c%</em>/<br>
/<em>%c<br>
{<br>
$$ = rb_ary_new3(1, $1);<br>
}<br>
%</em>/<br>
| bv_decls ',' bvar<br>
/<em>%c%</em>/<br>
/<em>%c<br>
{<br>
rb_ary_push($$, $3);<br>
}<br>
%</em>/<br>
;</p>
<p>Here, the call to rb_ary_push($$, $3) is relying on the fact that yacc/bison prefaces each routine's execution with $$=$1. It does so in order to implement the default behavior for rules that have no action routine (the implicit $$ = $1 rule), and while I doubt that will ever change, I don't think it should be relied upon. At the very least, it's a bit confusing at first glance (using $$ before it is set). Notably, other instances of this idiom (undef_list -> fitem | undef_list ',' fitem; f_arg -> f_arg_item | f_arg ',' f_arg_item; f_block_optarg; f_optarg....) use this approach.</p>
<p>It is corrected by simply replacing $$ with $1. A patch is included. No behavior changes, so no test is included.</p> Backport193 - Backport #5124 (Closed): foo = [*bar] implies foo.equal?(bar)https://redmine.ruby-lang.org/issues/51242011-07-31T16:35:45Zadgar (Michael Edgar)michael.j.edgar@dartmouth.edu
<p>I just ran into this as a result of some slightly sloppy coding, but it did disagree with my internal assumptions.</p>
<p>Normally, I expect the Array literal syntax to create a new object, every time. So when I rewrote some code and ended up with something similar to the following, my tests broke, and I'm not sure that's how it should be. Here's the reduced test case:</p>
<p>some_ary = [1, 2, 3]<br>
bar = [*some_ary]<br>
bar << 4</p>
<p>p bar</p>
<blockquote>
<blockquote>
<p>[1, 2, 3, 4]<br>
p some_ary<br>
[1, 2, 3, 4]</p>
</blockquote>
</blockquote>
<p>I see it's clearly taking an opportunity for optimization, so I'm more than happy to hear that as a reason for rejecting this. It does warrant documentation somewhere, though, I'd say. Not sure where that documentation would go.</p> Ruby master - Bug #5002 (Closed): Ripper fails to distinguish local vars from vcalls [PATCH]https://redmine.ruby-lang.org/issues/50022011-07-09T12:24:04Zadgar (Michael Edgar)michael.j.edgar@dartmouth.edu
<p>Ripper always parses the <code>variable</code> grammar production (which includes identifiers, {i,c,g}vars, nil, <strong>FILE</strong>, etc) as a <code>var_ref</code> node, whose only child is the token itself.</p>
<p>This is a problem for one huge reason: local variables look exactly like vcalls: no-arg, no-receiver method calls. More importantly, the parse tree defines whether a given bareword identifier is a local variable reference or a method call. Thus, given a ripper parse tree, in order to distinguish local variable references from vcalls, one must reconstruct the parse order, re-implement the local variable introduction rules (local variable assigned in some way, for loops, block arg, rescue exception variable, named regex capture groups, ....), and then relabel those <code>var_ref</code> nodes which are method calls as <code>vcall</code> nodes.</p>
<p>This is quite a nasty workaround. There are a <em>lot</em> of edge cases to mess up. I've implemented it as the <code>ripper-plus</code> gem, but it's a huge pain, I'm not sure it's entirely correct, and is something the parser should be doing anyway.</p>
<p>The funny thing is, the parser already <em>is</em> doing almost all of the work! It's just not looking at the local variable tables when it comes time to generate the Ripper event. The patch I've attached does do so - it's a small change for a huge benefit for Ripper users.</p>
<p>I'd like to see this land in 1.9.3 - it's a small patch, and given the other bug fixes Ripper's had this cycle, would make Ripper pretty much sufficient for an entire Ruby implementation.</p> Ruby master - Feature #4935 (Closed): Quoted Label Form for 1.9 Hasheshttps://redmine.ruby-lang.org/issues/49352011-06-27T11:28:42Zadgar (Michael Edgar)michael.j.edgar@dartmouth.edu
<p>As discussed in the "Shorthand Hash Syntax for Strings" thread/feature request, several (including Matz) expressed interest in a parallel quoted form for the 1.9 label syntax:</p>
<p>{ foo: bar }<br>
{ 'only-19': true }</p>
<p>x = "hello"<br>
{ "#{x}-world": Date.now }</p>
<p>This personally has come up for me with the new syntax: especially when dealing with command-line options, which often use hyphens, the quoted symbol form is necessary. Not being able to use the new syntax results in funny-looking hashes:</p>
<p>{ foo: bar, baz: qux, :'abc-def' => 1 }</p>
<p>I've attached a patch implementing this change. It allows a colon to follow single-quoted or double-quoted string literals to form a symbol key much like the label syntax introduced in 1.9. The examples listed at the start of this post all work fine. It introduces a new token - tLABEL_END - which replaces tSTRING_END when closing a symbol-label-literal. It introduces a single new grammar production for assoc, which is nearly purely syntactic sugar for the dsym grammar production; I've extracted dsym's implementation to a separate helper function, which is called from the dsym production and from the new label-style production.</p>
<p>I will immediately attach a new passing test for test_hash.rb in a followup post, once this has a feature # and ruby-core thread assigned to it.</p>
<p>NB: By removing the quote check on line 6580, one can make this feature work with any string literal (like "{ %q/foo-bar/: baz}", obviously doesn't work with heredocs) - I personally find this to be silly and inconsistent with the existing symbol syntax. Though it was fun to play with for a bit.</p> Ruby master - Feature #4924 (Assigned): mkmf have_header fails with C++ headershttps://redmine.ruby-lang.org/issues/49242011-06-24T12:09:56Zadgar (Michael Edgar)michael.j.edgar@dartmouth.edu
<p>=begin<br>
When a user calls (({have_header('some_cpp_header.h')})), and then header includes a line such as(({ #include })), mkmf will fail.</p>
<p>An example run follows:</p>
<ul>
<li>
<p>extconf.rb</p>
<p>require 'mkmf'<br>
have_library('stdc++')<br>
have_header('BasicBlock.h')<br>
create_makefile('laser/BasicBlock')</p>
</li>
<li>
<p>mkmf.log<br>
have_header: checking for BasicBlock.h... -------------------- no</p>
<p>"gcc -E -I/Users/michaeledgar/.rvm/rubies/ruby-1.9.2-head/include/ruby-1.9.1/x86_64-darwin10.7.0 -I/Users/michaeledgar/.rvm/rubies/ruby-1.9.2-head/include/ruby-1.9.1/ruby/backward -I/Users/michaeledgar/.rvm/rubies/ruby-1.9.2-head/include/ruby-1.9.1 -I. -D_XOPEN_SOURCE -D_DARWIN_C_SOURCE -O3 -ggdb -Wextra -Wno-unused-parameter -Wno-parentheses -Wpointer-arith -Wwrite-strings -Wno-missing-field-initializers -Wshorten-64-to-32 -Wno-long-long -fno-common -pipe conftest.c -o conftest.i"<br>
In file included from conftest.c:3:<br>
./BasicBlock.h:4:18: error: vector: No such file or directory<br>
./BasicBlock.h:5:21: error: exception: No such file or directory<br>
./BasicBlock.h:6:21: error: stdexcept: No such file or directory<br>
checked program was:<br>
/* begin <em>/<br>
1: #include "ruby.h"<br>
2:<br>
3: #include <BasicBlock.h><br>
/</em> end */</p>
</li>
</ul>
<p>The issue here is that the temporary file created to perform the header test is ((%conftest.c%)), not ((%conftest.cc%)) or ((%conftest.cpp%)). Changing the<br>
name of the file and re-running gcc gives success.</p>
<p>In ((%mkmf.rb%)), have_header executes cpp_command, which creates the shell command to run. However, cpp_command uses the constant (({CONFTEST_C = "conftest.c"})). It should use a new constant, (({CONFTEST_CPP = "conftest.cc"})).</p>
<p>I've attached a patch that does this as expected. Tests pass; I'm unsure precisely how to construct<br>
a test case that would be appropriate for the Ruby trunk. There are very few guiding examples in the<br>
existing test suite.</p> Ruby master - Bug #4847 (Closed): Documentation Error for Hash#rejecthttps://redmine.ruby-lang.org/issues/48472011-06-07T03:36:35Zadgar (Michael Edgar)michael.j.edgar@dartmouth.edu
<p>The documentation for Hash#reject fails to note that, when called without a block, an enumerator is returned.</p>
<p>A small patch correcting this is attached.</p> Ruby master - Bug #4716 (Closed): Ripper orders rescue_mod subnodes inconsistently [PATCH]https://redmine.ruby-lang.org/issues/47162011-05-17T08:21:54Zadgar (Michael Edgar)michael.j.edgar@dartmouth.edu
<p>Depending on whether a rescue_mod node is in an assignment context or not, the parse order differs:</p>
<p>pp Ripper.sexp("a rescue b")<br>
[:program,<br>
[[:rescue_mod,<br>
[:var_ref, [:@ident, "b", [1, 9]]],<br>
[:var_ref, [:@ident, "a", [1, 0]]]]]]</p>
<p>pp Ripper.sexp("x = a rescue b")</p>
<p>[:program,<br>
[[:assign,<br>
[:var_field, [:@ident, "x", [1, 0]]],<br>
[:rescue_mod,<br>
[:var_ref, [:@ident, "a", [1, 4]]],<br>
[:var_ref, [:@ident, "b", [1, 13]]]]]]]</p>
<p>I've attached a patch that fixes the issue - a patch containing tests will soon follow.</p> Ruby master - Feature #4553 (Rejected): Add Set#pick and Set#pophttps://redmine.ruby-lang.org/issues/45532011-04-05T08:10:08Zadgar (Michael Edgar)michael.j.edgar@dartmouth.edu
<p>=begin<br>
A very common operation on sets is to take an arbitrary element from them at O(1) cost. I know typically<br>
it's suggested that library additions go out as a gem to see community review. However, to me it seems<br>
to be a glaring omission to lack such an operation on a built-in, fundamental data structure, so I've<br>
gone straight to the bug tracker.</p>
<p>I wasn't too sure which method names to use as I've often heard "take" in lieu of "pop," so I just used the<br>
names Wikipedia uses. Consider them flexible. "Pick" selects an arbitrary element, and "pop" selects and<br>
deletes it.</p>
<p>I've provided a very simple patch that implements the necessary behavior. Thoughts?<br>
=end</p> Backport192 - Backport #4365 (Closed): Ripper.sexp should return an :array node for words/qwordshttps://redmine.ruby-lang.org/issues/43652011-02-04T07:14:46Zadgar (Michael Edgar)michael.j.edgar@dartmouth.edu
<p>=begin<br>
Currently, Ripper has no rule for the words/qwords base rule. It simply returns the array of string literals (and embexprs... etc) contained within the qwords/words expression. Example:</p>
<p>[:program,<br>
[[<br>
[:@tstring_content, "abc", [1, 3]], [:@tstring_content, "def", [1, 7]]]]]</p>
<blockquote>
<blockquote>
<p>pp Ripper.sexp('["abc", "def"]')<br>
[:program,<br>
[[:array,<br>
[[:string_literal, [:string_content, [:@tstring_content, "abc", [1, 2]]]],<br>
[:string_literal,<br>
[:string_content, [:@tstring_content, "def", [1, 9]]]]]]]]</p>
</blockquote>
</blockquote>
<p>Note that in the first example, the root node of the program (after the :program node) does not have a node type at all. I believe this is an error, and makes discovering words/qwords nodes in an AST quite difficult. I feel that for consistency with array literals, it should create an array node, or at least have a node type of its own. I'd prefer that:</p>
<p>['a', 'b', 'c']</p>
<p>and</p>
<p>%w(a b c)</p>
<p>return the same AST, as the latter is merely syntactic sugar for the former. I've attached a small patch to that effect, though if a different node type is desired for words/qwords, that's more than possible by renaming the node in the added calls to dispatch1.<br>
=end</p> Ruby master - Bug #4364 (Closed): Ripper loses MLHS variables in the presence of an LHS splathttps://redmine.ruby-lang.org/issues/43642011-02-04T06:55:07Zadgar (Michael Edgar)michael.j.edgar@dartmouth.edu
<p>=begin<br>
Ripper.sexp currently mis-parses the following LHS forms:</p>
<p>*b, c = ...<br>
*, a = ...<br>
a, *, b = ...</p>
<p>Specifically, in all 3 cases, the variables after the splat are not present in the resulting AST. The cause is simply missing mlhs_add calls in parse.y.</p>
<p>Here is the output of the trunk Ripper.sexp:</p>
<blockquote>
<blockquote>
<p>pp Ripper.sexp('*, b = 1, 2, 3')<br>
[:program,<br>
[[:massign,<br>
[:mlhs_add_star, [], nil],<br>
[:mrhs_new_from_args,<br>
[[:@int, "1", [1, 7]], [:@int, "2", [1, 10]]],<br>
[:@int, "3", [1, 13]]]]]]</p>
</blockquote>
</blockquote>
<blockquote>
<blockquote>
<p>pp Ripper.sexp('*b, c = 1, 2, 3')<br>
[:program,<br>
[[:massign,<br>
[:mlhs_add_star, [], [:@ident, "b", [1, 1]]],<br>
[:mrhs_new_from_args,<br>
[[:@int, "1", [1, 8]], [:@int, "2", [1, 11]]],<br>
[:@int, "3", [1, 14]]]]]]</p>
</blockquote>
</blockquote>
<blockquote>
<blockquote>
<p>pp Ripper.sexp('a, *, c = 1, 2, 3')<br>
[:program,<br>
[[:massign,<br>
[:mlhs_add_star, [[:@ident, "a", [1, 0]]], nil],<br>
[:mrhs_new_from_args,<br>
[[:@int, "1", [1, 10]], [:@int, "2", [1, 13]]],<br>
[:@int, "3", [1, 16]]]]]]</p>
</blockquote>
</blockquote>
<p>And here is it after the patch I am including:</p>
<p>require 'ripper'<br>
require 'pp'</p>
<blockquote>
<blockquote>
<p>pp Ripper.sexp('*, b = 1, 2, 3')<br>
[:program,<br>
[[:massign,<br>
[:mlhs_add_star, [], nil, [[:@ident, "b", [1, 3]]]],<br>
[:mrhs_new_from_args,<br>
[[:@int, "1", [1, 7]], [:@int, "2", [1, 10]]],<br>
[:@int, "3", [1, 13]]]]]]</p>
</blockquote>
</blockquote>
<blockquote>
<blockquote>
<p>pp Ripper.sexp('*b, c = 1, 2, 3, 4')<br>
[:program,<br>
[[:massign,<br>
[:mlhs_add_star, [], [:@ident, "b", [1, 1]], [[:@ident, "c", [1, 4]]]],<br>
[:mrhs_new_from_args,<br>
[[:@int, "1", [1, 8]], [:@int, "2", [1, 11]], [:@int, "3", [1, 14]]],<br>
[:@int, "4", [1, 17]]]]]]</p>
</blockquote>
</blockquote>
<blockquote>
<blockquote>
<p>pp Ripper.sexp('a, *, b, c = 1, 2, 3, 4')<br>
[:program,<br>
[[:massign,<br>
[:mlhs_add_star,<br>
[[:@ident, "a", [1, 0]]],<br>
nil,<br>
[[:@ident, "b", [1, 6]], [:@ident, "c", [1, 9]]]],<br>
[:mrhs_new_from_args,<br>
[[:@int, "1", [1, 13]], [:@int, "2", [1, 16]], [:@int, "3", [1, 19]]],<br>
[:@int, "4", [1, 22]]]]]]</p>
</blockquote>
</blockquote>
<p>I based the new output on the results of the one case that does parse successfully (this is for both versions):</p>
<blockquote>
<blockquote>
<p>pp Ripper.sexp('a, b, *c, d, e = 1, 2, 3, 4, 5, 6')<br>
[:program,<br>
[[:massign,<br>
[:mlhs_add_star,<br>
[[:@ident, "a", [1, 0]], [:@ident, "b", [1, 3]]],<br>
[:@ident, "c", [1, 7]],<br>
[[:@ident, "d", [1, 10]], [:@ident, "e", [1, 13]]]],<br>
[:mrhs_new_from_args,<br>
[[:@int, "1", [1, 17]],<br>
[:@int, "2", [1, 20]],<br>
[:@int, "3", [1, 23]],<br>
[:@int, "4", [1, 26]],<br>
[:@int, "5", [1, 29]]],<br>
[:@int, "6", [1, 32]]]]]]<br>
=end</p>
</blockquote>
</blockquote> Ruby master - Feature #2837 (Closed): Compile-time constant for HEAP_GROWTH_FACTOR (patch attached)https://redmine.ruby-lang.org/issues/28372010-03-05T05:20:29Zadgar (Michael Edgar)michael.j.edgar@dartmouth.edu
<p>=begin<br>
The GC currently increases the size at which newly-created heaps by a factor of 1.8 for each heap. Some find it appropriate to modify this value (REE uses a value of 1 instead of 1.8, for example). In the trunk version of this code, that value is hard-coded in as a constant at 1.8 in gc.c:980.</p>
<p>I've included a patch to expose this as a compile-time constant (HEAP_GROW_FACTOR), and also included getters and setters in the style of the patch I submitted in Issue 1047: <a href="http://redmine.ruby-lang.org/issues/show/1047" class="external">http://redmine.ruby-lang.org/issues/show/1047</a> .<br>
=end</p>