Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
improve compiler error message for failed overload resolution (#6596)
* migrate (as a separate copy) fsharpqa tests that are going to be impacted by #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. * update the .bsl files * remove comments that are handled by baseline files, update baseline files * remove the migrated tests from fsharpqa tests * need to be more careful when migrating those * testing if running the test with .fs instead of .fsx makes them work on .netcore * exclude migrated fsharpqa from dotnet core run * sample test in fsharpqa (can't run locally for now) * trying to make it green now. * checking if this path is covered by a test, trying to identify how to trigger this other overload related error message * * [MethodCalls.fs] Defining CallerArgs<'T> in, this replaces passing 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 * bit of refactoring of error message building logic during failed overload 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 ` * (buildfix) harmonizing .fsi/.fs right before commit doesn't always work with simple copy paste. * trying to check what kind of things break loose when I change this 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. * (minor) [ConstraintSolver.fs] revert space mishapps to reduce diff, put back comments where they were * toward displaying the argument names properly (may fail some fsharpqa tests for now) * missing Resharper's Ctrl+Alt+V "introduce variable" refactoring @auduchinok :) * pretty print unresolved overloads without all the type submsumption per overload verbose messages * Overload resolution error messages: things display like I want in fsi, 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/fsharp#2503 and put some time to add tests. * adjust message for candidates overload * Hijack the split phase for UnresolvedOverloading, remove the match on 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 * updating existing failing baseline files that looks correct to me * quickfix for update.base.line.with.actuals.fsx so that it skips missing base line files in the loop * (minor) minimize diff, typos, comments * fix vsintegration tests affected by overload error message changes * merge issue unused variable warning * update the 12 fsharpqa migrated baseline with new error messages * (minor) baseline update script [update.base.line.with.actuals.fsx] * add few directories * error handling (when tests are running, the .err files are locked * move System.Convert.ToString and System.Threading.Tasks.Task.Run tests from QA * * moving 3 fsharpqa tests to new test suite * bring back neg_System.Convert.ToString.OverloadList.fsx which I mistakenly removed during merge * consolidate all string building logic in CompileOps.fs, fix remaining cases with missing \n (the end result code is less whack-a-mole-ish) * update base lines * remove the migrated tests from fsharpqa * fix vstest error message * fix env.lst, removed wrong one... * update baselines of remaining tests * adding one simple test with many overloads * appropriate /// comments on `CalledMeth` constructor arguments * minimize diff / formatting * trim unused code * add simple test, one message is interesting, mentioning parameter count for possible overloads * comment flaky test for now * code review on the `coerceExpr` function from @TIHan, we can discard first `isOutArg` argument as the only hardcoded usage was guarded by `error` when the value is true, and passing an explicit false which is now guaranteed equivalent to `callerArg.IsOptional` * code formatting remarks on ConstraintSolver.fs/fsi * (minor) formatting * format known argument type / updating .bsl * update missing baseline * update missing baselines * update missing baseline * minor: make TTrait better in debugger display * [wip] pass TraitConstraintInfo around failed overload resolution error, and try to display some of it's info * minimize diff * signature file mismatch * removing duplicate in fscomp.txt * surfacing initial bits of TraitConstraintInfo, roughly for now * formatting types of known argument in one go, the formatting is still wrong: * unamed type arguments aren't relabelled (still show their numerical ids) * SRTP constraints are dismissed, not sure how they should be lined up to pass to SimplifyTypes.CollectInfo * rework of overload failure message to prettify *ALL* types in a single go, not only argument types, but return types and known generic argument types (this info was never displayed originally but is useful for scenario similar to FSharpPlus implementation) added `PrettifyDiscriminantAndTypePairs` in TastOps, this allow to weave extra information with the complete list of types, to help reconstituting the several types from their origin (in the usage spot: argument types, return type, generic parameter types) * fixup the tests and add two tests * one checking on the new information of known return type and generic parameter types * one checking we don't leak internal unammed Typar and that they don't all get named 'a despite being different * updating baselines that got tighter. * simplify handling of TraitConstraintInfo * naming couple of fields and turning a property to a methods as it triggers an assert in debugger when inspecting variables * comments in the assembling of overload resolution error message * Add information about which argument doesn't match Add couple of tests Updating bunch of baselines * minor updates to testguide and devguide * fix PrimitiveConstraints.``Invalid object constructor`` test * fix(?) salsa tests * minimize diff * put back tests under !FSHARP_SUITE_DRIVES_CORECLR_TESTS * missing updated message * minor adjustments to TESTGUIDE.md * return type was missing prettifying in prettyLayoutsOfUnresolvedOverloading * address code review nits / minimize diff / add comment on PrettifyDiscriminantAndTypePairs * minimize diff * proposed work around the flaky error message until dotnet/fsharp#6725 has a fix we keep the fsharpqa test around (but removing the overload error messages from what is asserted out of it) in the meantime * fixing baselines of new tests from master * sisyphus round of baseline update * removing type which isn't in use and popped back up after rebase * minimize diff * tidy inconsistent tuple literal * * removing TTrait properties that end up not being used * renaming tys and returnTy fields to better match convention (`tys` is used, so no underscore prefix) * minimizing diff * minimize diff * minimize diff * minimize diff * link to usage example in same file * revert converting CallerArg single cased DU into Record * minimize diff * minimize diff * minimize diff * fix rebase glitches * fix rebase glitch * update baseline * fix base lines / new tests base lines * edge case: needs a new line after "A unique overload for method '%s' could not be determined based on type information prior to this program point. A type annotation may be needed." if there are no optional parts. * updating base line for edge case of missing new line * missing baseline * removing comment
- Loading branch information
1 parent
37575d6
commit 4842127