Charlie:
Ah. Makes sense. Thanks for taking a look.
I can take another pass if you like.
Cheers,
Gerald
On 2009-08-02, at 5:20 PM, Charles Oliver Nutter <[email protected]>
wrote:
Ok, I think I see the problem. The new spec that's failing does an
hash.each {} and inside the each does repeated hash.merge! calls.
Previously, since we weren't raising errors for changes during
iteration, this passed ok. But now, it fails, because each sees that
the contents of the hash table are changing.
I think that in your patch, the raise inside iteratorVisitAll may not
be necessary. Since rehash itself checks if an iteration is in
progress, that would fulfill the contract of the spec from JRUBY-3745
without breaking the new spec.
I also noticed that your patch does iterator entry/exit without a
try/finally pair. This could lead to a hash getting stuck in
"iterating" mode if an error is raised during iteration, and never
being rehashable.
I'll make a few tweaks to your patch and see how it runs.
On Sun, Aug 2, 2009 at 4:34 PM, Gerald Boersma<[email protected]>
wrote:
Charlie:
Thanks.
I was holding off on posting it there since it is not fully
working. But I
can put it there if you think that works better.
Cheers,
Gerald
On 2009-08-02, at 11:01 AM, Charles Oliver Nutter <[email protected]
>
wrote:
Ok, I'll have a look at it. You can also just attach patches to the
associated bugs in the future, so they're centrally tracked.
- Charlie
On Sat, Aug 1, 2009 at 3:14 AM, Gerald Boersma<[email protected]>
wrote:
Here's an updated patch that is closer to the Ruby implementation
approach.
Now passes the specific rspec tests, but fails on another:
[java] Hash#merge! shouldn't raise spurious RuntimeErrors FAILED
[java] Expected to not get RuntimeError
[java]
/Users/gerald/dev/jruby/spec/mspec/lib/mspec/expectations/
expectations.rb:15:in
`fail_with'
[java]
/Users/gerald/dev/jruby/spec/mspec/lib/mspec/expectations/
should.rb:19:in
`should_not'
[java] /Users/gerald/dev/jruby/spec/ruby/core/hash/
merge_spec.rb:69
No idea on what is causing this one. Seems like a side effect of
the fix.
Any idea on what could cause this one? Any insight will be much
appreciated.
In the meantime, I'll move on to another bug to expand my
understanding
of
the JRuby core.
Cheers,
Gerald
On 31-Jul-09, at 11:44 AM, Gerald Boersma wrote:
ΩSo I have an approach, but I am not happy with it. Attached
is a patch.
- It does not work for Hash.sort_by.
- It adds an instance variable to Hash, which doesn't smell
right to me.
Basically:
- I set an instance variable on RubyHash which holds the block
passed
into
each, each_pair, etc.
- In rehash, I check the stack for the current thread to see if
the
iterator block appears. If so, I throw the appropriate exception.
This works for all cases except sort_by, since it seems that the
RubyHash.rehash is not even called in this case (?!?), and the
implementation lives somewhere else (RubyEnumerable).
I would prefer an approach similar to the Ruby C code, where the
function
hash_foreach_iter in hash.c is called from every context where the
iterator
is built, and it just compares an initial state to a final state
of the
hash. But I do not see a corresponding pattern in the Java code.
Any suggestions welcome.
Cheers,
Gerald
<rs3745.patch>
On 31-Jul-09, at 8:33 AM, Charles Oliver Nutter wrote:
On Thu, Jul 30, 2009 at 7:29 PM, Gerald Boersma<[email protected]
>
wrote:
Where can I look in the code base to understand how to
determine in
what
context the Hash.rehash method is called, i.e. if it is called
from an
iterator over the Hash or not? I see from the Hash.each method
how the
yield
to the block is performed and how the reference to the block
to be
used
is
called. I am not sure how to use that to safely determine that a
rehash
to
the same Hash is being called from that same block (or even if
that is
a
good approach. It doesn't smell quite right to me...).
Any suggestions?
If you think this defect is too big a bite to start with (i.e.
first
requires deep understanding of JRuby architecture / design), any
suggestions
for easier ones are welcome. But I am willing to start
anywhere...
A quick look at RubyHash.rehash shows it to be pretty
straightforward.
I don't think this is too difficult to start with. Why don't
you have
a look and feel free to post any questions back here. We're
standing
by to help you help us :)
BTW, searching through the dev mail archive using CodeHaus is
not
working
very well for me. Trying a search on "3745" or "RUBY-3745"
gives me
"Unfortunately, there was an unexpected issue processing your
request.
"
(after I prove I am human... ;-)
Yeah, we're pretty disappointed with the Codehaus mail
archives. I
don't think they've ever worked. Try nabble or markmail or
something.
- Charlie
---
---
---------------------------------------------------------------
To unsubscribe from this list, please visit:
http://xircles.codehaus.org/manage_email
---
------------------------------------------------------------------
To unsubscribe from this list, please visit:
http://xircles.codehaus.org/manage_email
---
------------------------------------------------------------------
To unsubscribe from this list, please visit:
http://xircles.codehaus.org/manage_email
---------------------------------------------------------------------
To unsubscribe from this list, please visit:
http://xircles.codehaus.org/manage_email
---------------------------------------------------------------------
To unsubscribe from this list, please visit:
http://xircles.codehaus.org/manage_email
---------------------------------------------------------------------
To unsubscribe from this list, please visit:
http://xircles.codehaus.org/manage_email
