Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report binding error for typed LINQ query on a type expression #22412

Merged
merged 2 commits into from
Sep 29, 2017

Conversation

jcouv
Copy link
Member

@jcouv jcouv commented Sep 29, 2017

Customer scenario
Use a type as the collection in a LINQ query with a typed variable (from object x in IEnumerable ...). The compiler should produce an error, but instead it fails when emitting.

Bugs this fixes:
Fixes #21484

Details:
When a LINQ query specifies a type (from object x in y), we will make an invocation for it: y.Cast<object>(). When y is IEnumerable, we find an extension method System.Linq.Enumerable.Cast<object>(this IEnumerable).
After we find it, this PR adds a check and reports a diagnostic, because that is trying to invoke an extension method with a type as its receiver (but an instance is required).

Note, the error is reported from another branch of the same method (MemberGroupFinalValidationAccessibilityChecks) in this scenario:

class C
{
   void Cast<T>() => throw null;
   void M()
   {
       C.Cast<object>(); // error CS0120: An object reference is required for the non-static field, method, or property 'C.Cast<object>()'
    }
}

Workarounds, if any
Fix your code. But the compiler doesn't provide diagnostics pointing to the source of the error :-(

Risk
Performance impact
Low. Adding one more check during binding of queries.
Thanks @AlekseyTs for the help narrowing down a low-risk fix :-)

Is this a regression from a previous update?
No.

How was the bug found?
Reported by customer.

@dotnet/roslyn-compiler for review. Thanks

@jcouv jcouv added this to the 15.5 milestone Sep 29, 2017
@jcouv jcouv self-assigned this Sep 29, 2017
{
// Could not find an implementation of the query pattern for source type '{0}'. '{1}' not found.
diagnostics.Add(ErrorCode.ERR_QueryNoProvider, node.Location, receiverOpt.Type, memberSymbol.Name);
if (receiverOpt?.Kind == BoundKind.QueryClause)
Copy link
Contributor

@AlekseyTs AlekseyTs Sep 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

? [](start = 39, length = 1)

It looks safe dropping the conditional aspect of this access. #Closed

@@ -2523,5 +2523,51 @@ static void Main()
Diagnostic(ErrorCode.ERR_LocalIllegallyOverrides, "a").WithArguments("a").WithLocation(10, 43)
);
}

[Fact, WorkItem(12052, "https://github.com/dotnet/roslyn/issues/12052")]
Copy link
Contributor

@AlekseyTs AlekseyTs Sep 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

12052 [](start = 24, length = 5)

Should this be 21484? #Closed

@AlekseyTs
Copy link
Contributor

AlekseyTs commented Sep 29, 2017

Done with review pass (iteration 1). #Closed

Copy link
Contributor

@AlekseyTs AlekseyTs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (iteration 2).

@jcouv jcouv merged commit 009f92e into dotnet:master Sep 29, 2017
333fred added a commit to 333fred/roslyn that referenced this pull request Sep 29, 2017
* dotnet/master:
  Report binding error for typed LINQ query on a type expression (dotnet#22412)
  clean up
  more refactoring
  Fixed tests
  fix test after rebase
  one more test
  CR feedback
  Allow taking unmanaged/pinned addresses of readonly variables.
  Fix typo in INotifyCompletion API in doc (dotnet#22417)
  Fix serialization of attribute data to account for named params argument (dotnet#22231)
  Create tests to cover test plans dotnet#19216 and dotnet#20127 (dotnet#22284)
  Checked uses of RefKind.RefReadOnly
  String rename
  Check for null before registering incremental analyzer in work coordinator.
  clean up
  Create tests to cover test plans dotnet#19216 and dotnet#20127
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How to reliably crash CSC with Linq
4 participants