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:
- 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.
- 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?
Please sign in to leave a comment.
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!"
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.
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!"
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)
Possibly related: a way to tell R# that a method doesn't enumerate an enumerable.
For example:
The FastCount and TryFastCount don't enumerate the source, but cause the "multiple enumeration" warning in the IndexOf method.
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!"