Code Review: re

Ruby IronRuby
1

tfpt review “/shelveset:re;REDMOND\sborde”

Comment :
Regexp literal support for /o.
/foo/ should compare equal with /foo/o, so we ensure that
RubyRegexpOptions.Once is passed into the RubyOps runtime helper, but we
then remove it in the RubyRegex constructor
Caching of the RubyRegex (and the underlying
System.Text.RegularExpressions.Regex) for regexp literals so that the
expression is processed only once
Sort the method names in the generated ReflectionCache. This will make
reduce the chances of conflicts during merging. Currently, the order is
whatever System.Reflection choses.

2

ReflectionCacheGenerator: More concise way how to sort an array:

var methods = ReflectMethods(typeof(RubyOps)).Values;
Array.Sort(methods, (m1, m2) => m1.Name.CompareTo(m2.Name));

ScringConstructor:

I would prefer TransformConcatenation to take 2 parameters (Expr
regexOptions, Expr regexCache) rather than params array.

Then this would not be necessary:
List<MSA.Expression> allArgs = new
List<MSA.Expression>(2 + additionalArgs.Length);
allArgs.Add(paramArray);
allArgs.Add(codePage);
allArgs.AddRange(additionalArgs);

and we could do just:
if (regexOptions != null)
opFactory(“N”).OpCall(paramArray, codePage,
regexOptions, regexCache) : …

Other than that looks good.

Tomas

3

Array.Sort(methods, …) does not work because methods is an
IEnumerable, not IList, and so I have to create an auxiliary list. I did
use the inline lambda for the comparision delegate. Changed
TransformConcatenation too.

Thanks,
Shri