Suggestion: a way to tell code analysis that an enumerable already undeferred

Consider this code:

        void Func()
        {
            var e = Enumerable.Range(0, 10);
            Test(e.ToList());
        }

        void Test<T>(IEnumerable<T> e)
        {
            if (e.Any())
            {
                Console.WriteLine(e.Count());
            }
        }

R# will flag both usages of 'e' in Test as "Possible multiple enumeration". Yet I want to require that all methods coming into that function already undefer their enums before passing them in. I could put an e = e.ToList() in Test() but what happens if I pass in a Dictionary? It would get undeferred twice and have double the memory usage temporarily.

Proposal: support a new JetBrains.Annotations.UnDeferredAttribute that I can put on the 'e' parameter of Test. This would do two things:

  1. It would treat the incoming parameter as already undeferred, as if it had ToList()/ToArray() called on it. This would mean the unnecessary multiple enumeration warnings would go away, at least until that code runs the enumerable through a new operator.
  2. It would require that all functions calling Test would have undeferred their enumerables being passed in, in advance. So it could flag potential errors of where a function is expecting an undeferred enumerable yet callers are not doing this.


What do you think?

6 comments

Hello Scott,

What about just changing void Test
e)? This will remove ReSharper warning about possible multiple enumerations
and also will produce a compilation error when someone passes an IEnumerable
to that method which forces to convert all IEnumerables to ICollection at
call sites.

Andrey Serebryansky
Senior Support Engineer
JetBrains, Inc
http://www.jetbrains.com
"Develop with pleasure!"

Consider this code:

void Func()
{
var e = Enumerable.Range(0, 10);
Test(e.ToList());
}
void Test<T>(IEnumerable<T> e)
{
if (e.Any())
{
Console.WriteLine(e.Count());
}
}
R# will flag both usages of 'e' in Test as "Possible multiple
enumeration". Yet I want to require that all methods coming into that
function already undefer their enums before passing them in. I could
put an e = e.ToList() in Test() but what happens if I pass in a
Dictionary? It would get undeferred twice and have double the memory
usage temporarily.

Proposal: support a new JetBrains.Annotations.UnDeferredAttribute that
I can put on the 'e' parameter of Test. This would do two things:

1. It would treat the incoming parameter as already undeferred, as if
it had ToList()/ToArray() called on it. This would mean the
unnecessary multiple enumeration warnings would go away, at least
until that code runs the enumerable through a new operator.

2. It would require that all functions calling Test would have
undeferred their enumerables being passed in, in advance. So it could
flag potential errors of where a function is expecting an undeferred
enumerable yet callers are not doing this.

What do you think?

---
Original message URL:
http://devnet.jetbrains.net/message/5308063#5308063



0

Your suggestion would only work on undeferral functions that return an ICollection. Many do not. For example, the RX framework has Memoize and MemoizeAll that cannot act as ICollections because they are built lazily.

Perhaps "undefer" isn't the best word choice. Maybe something like "cached". The point of the new R#6 error is to prevent walking the same enumerable multiple times. MemoizeAll (or our own UnDefer method) guarantees this, but it does it as an IEnumerable, and cannot do it as an ICollection.

0

Hello Scott,

Could you please clarify what kind of problem is with MemoizeAll and such?
Is the problem in the fact that those methods return an IEnumerable? Thank
you!

Andrey Serebryansky
Senior Support Engineer
JetBrains, Inc
http://www.jetbrains.com
"Develop with pleasure!"

Your suggestion would only work on undeferral functions that return an
ICollection. Many do not. For example, the RX framework has Memoize
and MemoizeAll that cannot act as ICollections because they are built
lazily.

Perhaps "undefer" isn't the best word choice. Maybe something like
"cached". The point of the new R#6 error is to prevent walking the
same enumerable multiple times. MemoizeAll (or our own UnDefer method)
guarantees this, but it does it as an IEnumerable, and cannot do it as
an ICollection.

---
Original message URL:
http://devnet.jetbrains.net/message/5308375#5308375



0

Here is a simple repro:

            var q = new[] { 1, 2, 3 }.AsEnumerable().MemoizeAll();
            var x = q.Any();
            var y = q.Any();

Both calls to Any() get a R# complaint about it being a possible multiple enumeration of IEnumerable.

But MemoizeAll(), which returns an IEnumerable<T>, is a function specifically written to avoid multiple enumerations. It caches values into an internal list the first time they are queried.

There are many more caching queries in Microsoft's Reactive Framework like Publish and Replay. These methods are designed to be enumerated multiple times, and will always get a warning from R# when used. None of them return an ICollection, they are all IEnumerable<T>.

So I have a problem. I want the R# detection of multiple enumerations in general, because it is such a useful warning. But I do not want it to happen on operators that are specifically designed for multiple enumeration. Right now I have these choices:

   * Always use ToList or ToArray. This not only may unnecessarily use extra memory but may not even work if run on a generator.
   * Suppress the warning with comments. Unfortunately this would really clutter the code. We are getting these warnings everywhere and in most cases they are invalid due to use of MemoizeAll or UnDefer.
   * Disable the warning completely in options for the whole team. This is what we are doing now.

So what I propose is an additional way to tell R# that an operator is ok for multiple enumeration. Then I can add it to our own operators like UnDefer, and I can add it to ExternalAnnotations for the RX operators.

Does this clear up the problem that I am describing?

(For more info on caching extensions see http://community.bartdesmet.net/blogs/bart/archive/2010/01/07/more-linq-with-system-interactive-functional-fun-and-taming-side-effects.aspx)

0

Possibly related: a way to tell R# that a method doesn't enumerate an enumerable.

For example:

public static bool TryFastCount<T>(this IEnumerable<T> source, out int count)
{
     if (null != source)
     {
          var genericCollection = source as ICollection<T>;
          if (null != genericCollection)
          {
               count = genericCollection.Count;
               return true;
          }
 
          var nonGenericCollection = source as ICollection;
          if (null != nonGenericCollection)
          {
               count = nonGenericCollection.Count;
               return true;
          }
     }
 
     count = -1;
     return false;
}

public static int? FastCount<T>(this IEnumerable<T> source) {      int result;      if (TryFastCount(source, out result)) return result;      return null; }

public static int IndexOf<T>(this IEnumerable<T> source, Func<T, bool> predicate, int startIndex, int count) {      if (null == source) throw new ArgumentNullException("source");      if (null == predicate) throw new ArgumentNullException("predicate");      int itemCount = FastCount(source) ?? -1;      if (-1 != itemCount) ArgumentHelper.ValidateListRange(true, itemCount, startIndex, count);      if (0 == itemCount || 0 == count) return -1;      return IndexOfCore(source, predicate, startIndex, count); }


The FastCount and TryFastCount don't enumerate the source, but cause the "multiple enumeration" warning in the IndexOf method.
Core - Microsoft Visual Studio.png

0

Hello Richard,

You're welcome to vote for the following feature request: http://youtrack.jetbrains.net/issue/RSRP-273689.
Thank you!

Andrey Serebryansky
Senior Support Engineer
JetBrains, Inc
http://www.jetbrains.com
"Develop with pleasure!"

Possibly related: a way to tell R# that a method doesn't enumerate an
enumerable.

For example:
public static bool TryFastCount<T>(this IEnumerable<T> source, out int
count)
{
if (null != source)
{
var genericCollection = source as ICollection<T>;
if (null != genericCollection)
{
count = genericCollection.Count;
return true;
}
var nonGenericCollection = source as ICollection;
if (null != nonGenericCollection)
{
count = nonGenericCollection.Count;
return true;
}
}
count = -1;
return false;
}
public static int? FastCount<T>(this IEnumerable<T> source)
{
int result;
if (TryFastCount(source, out result)) return result;
return null;
}
public static int IndexOf<T>(this IEnumerable<T> source, Func<T, bool>
predicate, int startIndex, int count)
{
if (null == source) throw new ArgumentNullException("source");
if (null == predicate) throw new
ArgumentNullException("predicate");
int itemCount = FastCount(source) ?? -1;
if (-1 != itemCount) ArgumentHelper.ValidateListRange(true,
itemCount, startIndex, count);
if (0 == itemCount || 0 == count) return -1;
return IndexOfCore(source, predicate, startIndex, count); }

The FastCount and TryFastCount don't enumerate the source, but cause
the "multiple enumeration" warning in the IndexOf method.
Image:Core - Microsoft Visual Studio.png
---
Original message URL:
http://devnet.jetbrains.net/message/5308851#5308851



0

Please sign in to leave a comment.