Skip to content

Conversation

@smoothdeveloper
Copy link
Owner

No description provided.

…cted by dotnet#6596

the baseline files are empty on purpose, expecting CI failure reporting those tests, I intend to update the baseline and clean up the comments / xml tags from fsharpqa syntax in later commit, and then remove those specific tests altogether from fsharpqa if this is OK with the maintainers.
… trigger this other overload related error message
…of callerArgsCount, uncurriedCallerArgs and other variants in the overload resolution logic happening in ConstraintSolver & TypeChecker

* [TypeChecker.fs] pass CallerArgs instace at call sites for overload resolution + some commented code as we'll be building a list of given argument types (probably moving as a CallerArgs method or property)
* [ConstraintSolver.fs/fsi] pipe the overload resolution traced callback to `trace.CollectThenUndoOrCommit` as that expression is long and more important in that context
…load resolution

[ConstraintSolver.fsi]

* define OverloadInformation and OverloadResolutionFailure, those replace workflow where `string * ((CalledMeth<_> * exn) list)` values were created at call site mixed with message building and matched later in non obvious fashion in `failOverloading` closure
* adjust UnresolvedOverloading and PossibleOverload exceptions

[ConstraintSolver.fs]

* get rid of `GetPossibleOverloads`, this was used only in `failOverloading` closure, and just being passed the same parameters at call site
* give `failOverloading` a more meaningful signature and consolidate the prefix message building logic there
* fix `failOverloading` call sites

[CompilerOps.fs] adjust pattern matching

Expecting behaviour of this code to be identical as before PR

`
I'm trying to identify why we get `(CalledMeth<Expr> * exn list)`, I'm making a cartesian product through getMethodSlotsAndErrors for now, trying to figure out if we can get other than 0 or 1 exception in the list for a given method slot.

I may revert that one if it doesn't make sense make from a reviewer standpoint and/or breaks fsharpqa tests surounding overload resolution error messages.
…, almost like I want in VS2019, this is for the simple non generic method overload case.

I want to check if user experience changes wrt dotnet#2503 and put some time to add tests.
* [MethodCalls.fs] Make `CallerArg` a record rather than single entry DU with properties
* [ConstraintSolver.fs] fix `CallerArg` call sites
* [TypeChecker.fs]
  * fix `CallerArg` call sites
  * un-nest DU deconstruction into simpler argument passing
  * simplify calls to `coerceExpr` by passing individual `CallerArg` rather than it's fields

Notes regarding changing to a record:
* the DU had properties, in debug, you'd see the unnamed items + the properties exposing the same values, this kind of change (+ naming fields) may have overall impact on debugging experience, along same line as naming DU fields (we remove half of the work of evaluating values when using inspector)
* fixing the call sites to the constructor is done by exposing a `make` function taking a tuple in same order as the original DU

Notes regarding passing CallerArg rather than it's fields:
* there seem to be overlap of optional and out argument concepts in this code path, but the changes are intended to be 1-to-1 refactor
… PossibleOverload.

It consolidates some more of the string building logic, and it now shows as a single compact and exhaustive error message in VS

Thinking I'll change PossibleOverload to not be an exception if going this way is OK
[update.base.line.with.actuals.fsx]
* add few directories
* error handling (when tests are running, the .err files are locked
smoothdeveloper and others added 28 commits January 31, 2020 20:24
we keep the fsharpqa test around (but removing the overload error messages from what is asserted out of it) in the meantime
* renaming tys and returnTy fields to better match convention (`tys` is used, so no underscore prefix)
* minimizing diff
@smoothdeveloper smoothdeveloper force-pushed the overload-list-error-message branch 2 times, most recently from bd7edfc to 4141681 Compare February 15, 2020 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants