Navigation Menu

Skip to content

Commit

Permalink
* IListOps.Difference now uses Object#hash and Object#eql? to check f…
Browse files Browse the repository at this point in the history
…or object equality, this fixes the failing spec "Array#- acts as if using an intermediate hash to collect values"

* Modified IListOps.RecursiveJoin to make it flag the resulting string as tainted if the given array, at least one of its elements or the separator string are tainted.

* Changed IListOps.Repetition to return IList instances of the same type of the given self argument (this fixes also "Array#* with an integer returns subclass instance with Array subclasses")

* Various changes to IListOps.Join to clear all of the remaining tags for the specs of Array#join. The tags marked as critical in join_tags.txt are not related to pending bugs for Array#join.

* Added ArrayOps.ToAry as Array#to_a and Array#to_ary behave differently on subclasses of arrays.

* Cleaning up tags removing expectations that do not fail anymore.

* Changed one overload of IListOps.Equals to make it try to call #to_ary on the object argument of Array#== and use the resulting array as the actual argument for the equality check.

* Changed IListOps.SetElement to make it try to invoke #to_ary on its argument for multi-element sets.

* Changed IListOps.Reverse to return IList instances of the same type of the given self argument (this change also fixes the following failing spec: "Array#reverse returns subclass instance on Array subclasses")

* Fixed IListOps.ReverseIndex as Array#rindex should not fail if elements are removed from the array during the iteration over its elements.

* Slightly modified IListOps.UniqueSelf as suggested upon review.
  • Loading branch information
nrk committed Apr 21, 2009
1 parent a01d87e commit 722dd4d
Show file tree
Hide file tree
Showing 15 changed files with 92 additions and 68 deletions.

This file was deleted.

@@ -1,2 +1 @@
fails:Array#== returns false if any element is not == to the corresponding element in the other the array
fails:Array#== calls to_ary on its argument

This file was deleted.

@@ -1,12 +1,5 @@
critical:Array#join raises a NoMethodError if an element does not respond to #to_s
critical:Array#join raises a NoMethodError if an element does not respond to #to_s
fails:Array#join tries to convert the passed seperator to a String using #to_str
fails:Array#join checks whether the passed seperator responds to #to_str
fails:Array#join handles recursive arrays
fails:Array#join does not consider taint of either the array or the separator when the array is empty
fails:Array#join returns a string which would be infected with taint of the array, its elements or the separator when the array is not empty
fails:Array#join does not process the separator if the array is empty
fails:Array#join raises a TypeError if the passed separator is not a string and does not respond to #to_str
critical:Array#join raises a NoMethodError if an element does not respond to #to_s
critical:Array#join raises a NoMethodError if an element does not respond to #to_s
critical:Array#join raises a NoMethodError if an element does not respond to #to_s
Expand Down

This file was deleted.

@@ -1,8 +1,4 @@
fails:Array#* with an integer returns subclass instance with Array subclasses
fails:Array#* with an integer copies the taint status of the original array even if the array is empty
fails:Array#* with an integer copies the taint status of the original array if the passed count is not 0
critical:Array#* with a string raises a NoMethodError if an element does not respond to #to_s
fails:Array#* with a string returns a string which would be infected with taint of the array, its elements or the separator when the array is not empty
critical:Array#* with a string raises a NoMethodError if an element does not respond to #to_s
critical:Array#* with a string raises a NoMethodError if an element does not respond to #to_s
critical:Array#* with a string raises a NoMethodError if an element does not respond to #to_s
Expand Down
@@ -1,2 +1 @@
fails:Array.new with (size, object=nil) raises an ArgumentError if size is too large
fails:Array.new with (size, object=nil) uses the block value instead of using the default value

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Expand Up @@ -16,12 +16,8 @@ core\argf\path_tags.txt:0:critical:ARGF.path it sets the $FILENAME global variab
core\argf\rewind_tags.txt:0:critical:ARGF.rewind goes back to beginning of current file
core\argf\to_i_tags.txt:0:critical:ARGF.to_i returns the current file number on each file
core\argf\to_io_tags.txt:0:critical:ARGF.to_io returns the IO of the current file
core\array\intersection_tags.txt:0:critical:Array#& properly handles recursive arrays
core\array\join_tags.txt:0:critical:Array#join raises a NoMethodError if an element does not respond to #to_s
core\array\join_tags.txt:0:critical:Array#join does not separates elements when the passed separator is nil
core\array\union_tags.txt:0:critical:Array#| properly handles recursive arrays
core\array\uniq_tags.txt:0:critical:Array#uniq properly handles recursive arrays
core\array\uniq_tags.txt:0:critical:Array#uniq! properly handles recursive arrays
core\kernel\sleep_tags.txt:3:critical:Kernel#sleep pauses execution indefinitely if not given a duration
core\process\times_tags.txt:0:unstable:Process.times returns current cpu times
core\string\process\wait_tags.txt:0:critical:Process.wait
Expand Down
Expand Up @@ -171,11 +171,15 @@ public static class ArrayOps {
#endregion

[RubyMethod("to_a")]
[RubyMethod("to_ary")]
public static RubyArray/*!*/ ToArray(RubyArray/*!*/ self) {
return self is RubyArray.Subclass ? new RubyArray(self) : self;
}

[RubyMethod("to_ary")]
public static RubyArray/*!*/ ToAry(RubyArray/*!*/ self) {
return self;
}

#region class FormatDirective is used by Array.pack and String.unpack

internal struct FormatDirective {
Expand Down
Expand Up @@ -186,14 +186,20 @@ public static class IListOps {
#region *, +, concat

[RubyMethod("*")]
public static RubyArray/*!*/ Repetition(IList/*!*/ self, int repeat) {
public static IList/*!*/ Repetition(CallSiteStorage<Func<CallSite, RubyClass, object>>/*!*/ allocateStorage, IList/*!*/ self, int repeat) {
if (repeat < 0) {
throw RubyExceptions.CreateArgumentError("negative argument");
}
RubyArray result = new RubyArray(self.Count * repeat);

IList result = CreateResultArray(allocateStorage, self);
if (result is RubyArray) {
(result as RubyArray).Capacity = self.Count * repeat;
}
for (int i = 0; i < repeat; ++i) {
AddRange(result, self);
}

allocateStorage.Context.TaintObjectBy<IList>(result, self);
return result;
}

Expand All @@ -203,10 +209,10 @@ public static class IListOps {
}

[RubyMethod("*")]
public static object Repetition(ConversionStorage<MutableString>/*!*/ tosConversion, IList/*!*/ self, [DefaultProtocol]Union<MutableString, int> repeat) {
public static object Repetition(CallSiteStorage<Func<CallSite, RubyClass, object>>/*!*/ allocateStorage, ConversionStorage<MutableString>/*!*/ tosConversion, IList/*!*/ self, [DefaultProtocol]Union<MutableString, int> repeat) {

if (repeat.IsFixnum()) {
return Repetition(self, repeat.Fixnum());
return Repetition(allocateStorage, self, repeat.Fixnum());
} else {
return Repetition(tosConversion, self, repeat.String());
}
Expand All @@ -230,13 +236,25 @@ public static class IListOps {
}

[RubyMethod("-")]
public static RubyArray/*!*/ Difference(BinaryOpStorage/*!*/ equals, IList/*!*/ self, [DefaultProtocol, NotNull]IList/*!*/ other) {
public static RubyArray/*!*/ Difference(RubyContext/*!*/ context, IList/*!*/ self, [DefaultProtocol, NotNull]IList/*!*/ other) {
// MRI follows hashing semantics here, so doesn't actually call eql?/hash for Fixnum/Symbol
IEqualityComparer<object> comparer = context.EqualityComparer;
RubyArray result = new RubyArray();

// TODO: optimize this
foreach (object element in self) {
if (!Include(equals, other, element)) {
result.Add(element);
foreach (object selfElement in self) {
bool found = false;
foreach (object otherElement in other) {
bool sameHashcode = comparer.GetHashCode(selfElement) == comparer.GetHashCode(otherElement);
bool isEql = comparer.Equals(selfElement, otherElement);
if (sameHashcode && isEql) {
found = true;
break;
}
}

if (!found) {
result.Add(selfElement);
}
}

Expand All @@ -248,8 +266,9 @@ public static class IListOps {
#region ==, <=>, eql?, hash

[RubyMethod("==")]
public static bool Equals(IList/*!*/ self, object other) {
return false;
public static bool Equals(ConversionStorage<IList>/*!*/ arrayTryCast, BinaryOpStorage/*!*/ equals, IList/*!*/ self, object other) {
IList otherAsArray = Protocols.TryCastToArray(arrayTryCast, other);
return otherAsArray != null ? Equals(equals, self, otherAsArray) : false;
}

[MultiRuntimeAware]
Expand Down Expand Up @@ -409,8 +428,9 @@ public static class IListOps {
}

[RubyMethod("[]=")]
public static object SetElement(RubyContext/*!*/ context, IList/*!*/ self, [DefaultProtocol]int index, [DefaultProtocol]int length, object value) {
RubyUtils.RequiresNotFrozen(context, self);
public static object SetElement(ConversionStorage<IList>/*!*/ arrayTryCast, IList/*!*/ self,
[DefaultProtocol]int index, [DefaultProtocol]int length, object value) {
RubyUtils.RequiresNotFrozen(arrayTryCast.Context, self);

if (length < 0) {
throw RubyExceptions.CreateIndexError(String.Format("negative length ({0})", length));
Expand All @@ -422,12 +442,15 @@ public static class IListOps {
}

IList valueAsList = value as IList;
if (valueAsList == null) {
valueAsList = Protocols.TryCastToArray(arrayTryCast, value);
}

if (value == null || (valueAsList != null && valueAsList.Count == 0)) {
DeleteItems(self, index, length);
} else {
if (valueAsList == null) {
Insert(context, self, index, value);
Insert(arrayTryCast.Context, self, index, value);

if (length > 0) {
RemoveRange(self, index + 1, Math.Min(length, self.Count - index - 1));
Expand Down Expand Up @@ -459,7 +482,8 @@ public static class IListOps {
}

[RubyMethod("[]=")]
public static object SetElement(ConversionStorage<int>/*!*/ fixnumCast, IList/*!*/ self, [NotNull]Range/*!*/ range, object value) {
public static object SetElement(ConversionStorage<IList>/*!*/ arrayTryCast, ConversionStorage<int>/*!*/ fixnumCast,
IList/*!*/ self, [NotNull]Range/*!*/ range, object value) {
RubyUtils.RequiresNotFrozen(fixnumCast.Context, self);

int begin = Protocols.CastToFixnum(fixnumCast, range.Begin);
Expand All @@ -473,7 +497,7 @@ public static class IListOps {
end = end < 0 ? end + self.Count : end;

int count = range.ExcludeEnd ? end - begin : end - begin + 1;
return SetElement(fixnumCast.Context, self, begin, Math.Max(count, 0), value);
return SetElement(arrayTryCast, self, begin, Math.Max(count, 0), value);
}

#endregion
Expand Down Expand Up @@ -1010,10 +1034,13 @@ public static class IListOps {

[RubyMethod("rindex")]
public static object ReverseIndex(BinaryOpStorage/*!*/ equals, IList/*!*/ self, object item) {
for (int i = self.Count - 1; i >= 0; i--) {
for (int originalSize = self.Count, i = originalSize - 1; i >= 0; i--) {
if (Protocols.IsEqual(equals, self[i], item)) {
return i;
}
if (self.Count < originalSize) {
i = originalSize - i - 1 + self.Count;
}
}
return null;
}
Expand Down Expand Up @@ -1083,6 +1110,7 @@ public static class IListOps {
IList/*!*/ list, MutableString/*!*/ separator, MutableString/*!*/ result, Dictionary<object, bool>/*!*/ seen) {

Assert.NotNull(list, separator, result, seen);
RubyContext context = tosConversion.Context;
// TODO: can we get by only tracking List<> ?
// (inspect needs to track everything)
bool found;
Expand All @@ -1109,6 +1137,12 @@ public static class IListOps {
}
}

if (!result.IsTainted){
if (context.IsObjectTainted(list) || context.IsObjectTainted(item) || separator.IsTainted) {
result.IsTainted = true;
}
}

if (i < list.Count - 1) {
result.Append(separator);
}
Expand All @@ -1124,9 +1158,16 @@ public static class IListOps {
}

[RubyMethod("join")]
public static MutableString/*!*/ Join(ConversionStorage<MutableString>/*!*/ tosConversion, IList/*!*/ self, MutableString separator) {
public static MutableString/*!*/ Join(ConversionStorage<MutableString>/*!*/ tosConversion, ConversionStorage<MutableString>/*!*/ tostrConversion, IList/*!*/ self, Object separator) {
if (self.Count == 0) {
return MutableString.Empty;
}
return Join(tosConversion, self, separator != null ? Protocols.CastToString(tostrConversion, separator) : MutableString.Empty);
}

public static MutableString/*!*/ Join(ConversionStorage<MutableString>/*!*/ tosConversion, IList/*!*/ self, MutableString/*!*/ separator) {
MutableString result = MutableString.CreateMutable();
RecursiveJoin(tosConversion, self, separator ?? MutableString.Empty, result,
RecursiveJoin(tosConversion, self, separator ?? MutableString.Empty, result,
new Dictionary<object, bool>(ReferenceEqualityComparer<object>.Instance)
);
return result;
Expand Down Expand Up @@ -1270,34 +1311,36 @@ public static class IListOps {
#region slice!

[RubyMethod("slice!")]
public static object SliceInPlace(RubyContext/*!*/ context, IList/*!*/ self, [DefaultProtocol]int index) {
RubyUtils.RequiresNotFrozen(context, self);
public static object SliceInPlace(ConversionStorage<IList>/*!*/ arrayTryCast, IList/*!*/ self, [DefaultProtocol]int index) {
RubyUtils.RequiresNotFrozen(arrayTryCast.Context, self);
index = index < 0 ? index + self.Count : index;
if (index >= 0 && index < self.Count) {
object result = self[index];
SetElement(context, self, index, 1, null);
SetElement(arrayTryCast, self, index, 1, null);
return result;
} else {
return null;
}
}

[RubyMethod("slice!")]
public static object SliceInPlace(ConversionStorage<int>/*!*/ fixnumCast,
public static object SliceInPlace(ConversionStorage<IList>/*!*/ arrayTryCast,
ConversionStorage<int>/*!*/ fixnumCast,
CallSiteStorage<Func<CallSite, RubyClass, object>>/*!*/ allocateStorage,
IList/*!*/ self, [NotNull]Range/*!*/ range) {
RubyUtils.RequiresNotFrozen(fixnumCast.Context, self);
object result = GetElement(fixnumCast, allocateStorage, self, range);
SetElement(fixnumCast, self, range, null);
SetElement(arrayTryCast, fixnumCast, self, range, null);
return result;
}

[RubyMethod("slice!")]
public static IList/*!*/ SliceInPlace(CallSiteStorage<Func<CallSite, RubyClass, object>>/*!*/ allocateStorage,
public static IList/*!*/ SliceInPlace(ConversionStorage<IList>/*!*/ arrayTryCast,
CallSiteStorage<Func<CallSite, RubyClass, object>>/*!*/ allocateStorage,
IList/*!*/ self, [DefaultProtocol]int start, [DefaultProtocol]int length) {
RubyUtils.RequiresNotFrozen(allocateStorage.Context, self);
IList result = GetElements(allocateStorage, self, start, length);
SetElement(allocateStorage.Context, self, start, length, null);
SetElement(arrayTryCast, self, start, length, null);
return result;
}

Expand Down Expand Up @@ -1342,8 +1385,11 @@ public static class IListOps {
#region reverse, reverse!, transpose, uniq, uniq!

[RubyMethod("reverse")]
public static RubyArray/*!*/ Reverse(IList/*!*/ self) {
RubyArray reversedList = new RubyArray(self.Count);
public static IList/*!*/ Reverse(CallSiteStorage<Func<CallSite, RubyClass, object>>/*!*/ allocateStorage, IList/*!*/ self) {
IList reversedList = CreateResultArray(allocateStorage, self);
if (reversedList is RubyArray) {
(reversedList as RubyArray).Capacity = self.Count;
}
for (int i = 0; i < self.Count; i++) {
reversedList.Add(self[self.Count - i - 1]);
}
Expand Down Expand Up @@ -1411,7 +1457,6 @@ public static class IListOps {
[RubyMethod("uniq!")]
public static IList UniqueSelf(RubyContext/*!*/ context, IList/*!*/ self) {
var seen = new Dictionary<object, bool>(context.EqualityComparer);
bool frozen = context.IsObjectFrozen(self);
bool modified = false;
int i = 0;
while (i < self.Count) {
Expand All @@ -1420,7 +1465,7 @@ public static class IListOps {
seen.Add(key, true);
i++;
} else {
if (frozen) {
if (context.IsObjectFrozen(self)) {
throw RubyExceptions.CreateTypeError("can't modify frozen array");
}
self.RemoveAt(i);
Expand Down

0 comments on commit 722dd4d

Please sign in to comment.