-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Fix go to definition for conversions on invalid constructor overloads #78514
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
Fix go to definition for conversions on invalid constructor overloads #78514
Conversation
We need to have direct tests for SemanticModel API, plus an explanation what was the old behavior, why it was wrong, etc.
@AlekseyTs this is ready for review
plus an explanation what was the old behavior, why it was wrong, etc.
Was the explanation provided? Basically we need a clear description of the problem with the SemanticModel API. The reason why you think the present behavior is wrong, a description of the expected behavior with a reason behind the expectation.
Was the explanation provided?
I added an explanation here: https://github.com/dotnet/roslyn/pull/78514/files#diff-d901fc0a7ce2d67b0336c3708a45962ee31ae1ce7c9c67e79d3def4fc8f947bfR1913-R1916
I can reword it if you feel like it doesn't provide enough information
I think this isn't quite what I was looking for. I was looking for a description of your intent in this PR and the reason behind the intent. Something like: "I intend to change behavior *** API for the scenarios ***. The current behavior of the API is ***. I think it should be *** instead, because ***." Obviously your ultimate goal is to get some IDE scenarios working, but simply stating that is not sufficient for changing how compiler API behaves. In order for the Compiler team to review the PR, we need the intent stated in "Compiler API" terms.
@AlekseyTs updated the description and brought the PR up to date, ptal
Uh oh!
There was an error while loading. Please reload this page.
Closes #73498
Closes #77545
With this PR, the
GetSymbolInfoAPI and all the corresponding APIs using it will be affected for the scenarios where an object creation expression used a constructor overload that doesn't exist, whilst the expression was being implicitly converted to another type. The previous behavior would resolve to the target type of the conversion, since the binding result would be an overload resolution, whereas now it has been changed to resolve to the existing overload resolution result.The goal of this change is also reflected in GTD which will now correctly navigate to the type whose constructor was not found, instead of the target conversion type, or the implicit conversion method.
This also adds tests for explicit conversions on object creation expressions, covering the above scenaria.