Crash in hash when key is an array with nil

[GoogleCodeExporter]

This code demonstrates an issue that causes ERB to fail (in 
Scanner#regist_scanner) 

$scanner_map = {}
$scanner_map['a'] = 'b'         # <= OK
$scanner_map[['a','b']] = 'c'   # <= OK
$scanner_map[nil] = 'e'         # <= OK
$scanner_map[['d',nil]] = 'e'   # <= Crash
$scanner_map[[nil,'d']] = 'e'   # <= Crash

It appears to be a problem with the nil.

Error dump:
Unhandled Exception: System.Exception: Exception has been thrown by the 
target of an invocation.
   at Ruby.Compiler.AST.SOURCEFILE.ExecuteMain(Assembly Assembly, String[] 
args)
   at Ruby.Compiler.AST.SOURCEFILE.ExecuteMain(PEFile Assembly, String[] 
args)
   at Ruby.Compiler.RubyEntry.Process(String[] args)
   at RubyMain.Main(String[] args) in H:\Play\Ruby.Net\0.8.2
\src\Ruby\Main.cs:line 5

This is with 0.8.2 on Windows XP

Original issue reported on code.google.com by djl_davi...@rifraf.net on 21 Aug 2007 at 11:53

[GoogleCodeExporter]

The simple fix is attached.

Original comment by sanxiyn on 24 Aug 2007 at 11:52

Attachments:

• hash-array-with-nil

[GoogleCodeExporter]

Actually MRI implements this by calling the hash method on each element in the 
array,
and then NUM2LONG. NUM2LONG differs subtly with Hash's key-hashing method
(rb_hash_any) by raising an exception when the return value is not a fixnum. 
Attached
is a patch that follows this behaviour.

It's possible to also check for non-Ruby objects and call GetHashCode instead, 
but
I'm not sure that's the right place to do it (hence the TODO comment in the 
patch).

I'll check this in after writing the test (I'm in fact quite surprised that 
none of
the test suites that we run catch this), unless there are objections.

Original comment by sason...@gmail.com on 24 Aug 2007 at 2:14

• Changed state: Started
• Added labels: Type-Bug, Product-RubyRuntime, CLI-All
• Removed labels: Type-Defect

Attachments:

• hash-array-with-nil.patch

[GoogleCodeExporter]

> It's possible to also check for non-Ruby objects and call GetHashCode instead

I don't think it's necessary to reason about non-ruby objects in the builtins. 
They 
will eventually (but not yet) look just like ordinary ruby objects complete 
with 
methods like "hash".

Original comment by douglas....@gmail.com on 25 Aug 2007 at 12:52

[GoogleCodeExporter]

Attached is the new patch which adds tests for nil elements as well as the 
"hash"
call on each element.  It tests with various return types of hash because 
(surprise
surprise) the type of the exception raised depends on what hash returns.

> > It's possible to also check for non-Ruby objects and call GetHashCode 
instead
> 
> I don't think it's necessary to reason about non-ruby objects in the 
builtins. They 
will eventually (but not yet) look just like ordinary ruby objects complete 
with 
methods like "hash".

Great, that's what I expected too; and I see your latest patch in the mailing 
list
addresses that.

Original comment by sason...@gmail.com on 25 Aug 2007 at 12:34

Attachments:

• hash-array-with-nil-2.patch

[GoogleCodeExporter]

hash-array-with-nil-2.patch commited as r71.

Original comment by sason...@gmail.com on 28 Aug 2007 at 1:20

• Changed state: Fixed