Branch: refs/heads/master
Revision: 1183999
Author: bradcray
Log Message:
Merge pull request #16969 from bradcray/squash-bad-candidates
Avoid printing candidates in resolution error messages that represent a method / standalone function mismatch
[reviewed by @mppf]
This implements an idea proposed in issue #16928 in which we don’t print out resolution candidates by default if they are standalone functions and the call is a method; or the call is to a standalone function and the candidate is a method. This was motivated by a user error reported in issue #16917 in which a bad call to write() printed out four unrelated method-based implementations of write().
As an example, prior to this change, calling write("hi", int); resulted in:
badWrite.chpl:1: error: unresolved call 'write("hi", type int(64))'
$CHPL_HOME/modules/standard/ChapelIO.chpl:837: note: this candidate did not match: write(const args ...?n)
badWrite.chpl:1: note: because actual argument #2 is a type
$CHPL_HOME/modules/standard/ChapelIO.chpl:837: note: but is passed to non-type varargs formal 'args'
$CHPL_HOME/modules/internal/Atomics.chpl:237: note: candidates are: AtomicBool.write(value: bool, param order: memoryOrder = memoryOrder.seqCst)
$CHPL_HOME/modules/internal/Atomics.chpl:422: note: AtomicT.write(value: T, param order: memoryOrder = memoryOrder.seqCst)
note: and 2 other candidates, use --print-all-candidates to see them
where this PR changes it to:
badWrite.chpl:1: error: unresolved call 'write("hi", type int(64))'
$CHPL_HOME/modules/standard/ChapelIO.chpl:837: note: this candidate did not match: write(const args ...?n)
badWrite.chpl:1: note: because actual argument #2 is a type
$CHPL_HOME/modules/standard/ChapelIO.chpl:837: note: but is passed to non-type varargs formal 'args'
note: there are also 4 other candidates, use --print-all-candidates to see them
Resolves #16928 to my satisfaction (that issue also proposed that we trim method calls to classes not in the same class sub-hierarchy, but I currently think that this happens rarely enough to not be worth the additional effort until it becomes a true pain point).
One interesting side effect of this is that in some existing tests, the “best” match whose failed resolution we choose to describe sometimes changed between a standalone function and a method. For example, given foo(1) and choices between proc R.foo(x:int) and proc foo(), perhaps we used to choose the method and describe the missing receiver, but now choose the standalone function and describe the extraneous argument. This is arguably a “half-empty/half-full” type of issue where there’s arguably no right answer (e.g., did the programmer forget to mention the object on which they intended to dispatch? or did they think there was an int-taking standalone version that doesn’t exist?). But I think the benefits are sufficiently strong in cases like the one motivating that this tradeoff is worth taking as well.
Added two new tests:
- one that captures a version of the original motivating issue
- one that tests a number of method vs. other class method vs. standalone function overload errors to make sure they behaved as expected
While here, I made some other minor adjustments:
- changed “1 arguments” to “1 argument” in a few error messages because it started to bug me
- changed “candidates are:” line after an explanation about the first bad match to “other candidates include” because the previous phrasing seemed confusing (since it wasn’t listing all candidates, simply those that hadn’t been explained)
- moved these “other candidates” to their own lines with no indentation rather than indenting them as before, as suggested by @mppf and @vasslitvinov
- specialized the message when there are no other candidates that meet the standalone/method choice to say something like “there are also 4 other candidates” to make it read more clearly (to my eyes)
- refactored some of the compiler logic that generates these error messages in hopes of making it a bit more straightforward
Modified Files:
A test/functions/resolution/errors/badWrite.chpl
A test/functions/resolution/errors/badWrite.good
A test/functions/resolution/errors/testBadCallCandidates.1-all.good
A test/functions/resolution/errors/testBadCallCandidates.1.good
A test/functions/resolution/errors/testBadCallCandidates.10-all.good
A test/functions/resolution/errors/testBadCallCandidates.10.good
A test/functions/resolution/errors/testBadCallCandidates.11-all.good
A test/functions/resolution/errors/testBadCallCandidates.11.good
A test/functions/resolution/errors/testBadCallCandidates.12-all.good
A test/functions/resolution/errors/testBadCallCandidates.12.good
A test/functions/resolution/errors/testBadCallCandidates.13-all.good
A test/functions/resolution/errors/testBadCallCandidates.13.good
A test/functions/resolution/errors/testBadCallCandidates.14-all.good
A test/functions/resolution/errors/testBadCallCandidates.14.good
A test/functions/resolution/errors/testBadCallCandidates.15-all.good
A test/functions/resolution/errors/testBadCallCandidates.15.good
A test/functions/resolution/errors/testBadCallCandidates.16-all.good
A test/functions/resolution/errors/testBadCallCandidates.16.good
A test/functions/resolution/errors/testBadCallCandidates.17-all.good
A test/functions/resolution/errors/testBadCallCandidates.17.good
A test/functions/resolution/errors/testBadCallCandidates.18-all.good
A test/functions/resolution/errors/testBadCallCandidates.18.good
A test/functions/resolution/errors/testBadCallCandidates.19-all.good
A test/functions/resolution/errors/testBadCallCandidates.19.good
A test/functions/resolution/errors/testBadCallCandidates.2-all.good
A test/functions/resolution/errors/testBadCallCandidates.2.good
A test/functions/resolution/errors/testBadCallCandidates.20-all.good
A test/functions/resolution/errors/testBadCallCandidates.20.good
A test/functions/resolution/errors/testBadCallCandidates.21-all.good
A test/functions/resolution/errors/testBadCallCandidates.21.good
A test/functions/resolution/errors/testBadCallCandidates.3-all.good
A test/functions/resolution/errors/testBadCallCandidates.3.good
A test/functions/resolution/errors/testBadCallCandidates.4-all.good
A test/functions/resolution/errors/testBadCallCandidates.4.good
A test/functions/resolution/errors/testBadCallCandidates.5-all.good
A test/functions/resolution/errors/testBadCallCandidates.5.good
A test/functions/resolution/errors/testBadCallCandidates.6-all.good
A test/functions/resolution/errors/testBadCallCandidates.6.good
A test/functions/resolution/errors/testBadCallCandidates.7-all.good
A test/functions/resolution/errors/testBadCallCandidates.7.good
A test/functions/resolution/errors/testBadCallCandidates.8-all.good
A test/functions/resolution/errors/testBadCallCandidates.8.good
A test/functions/resolution/errors/testBadCallCandidates.9-all.good
A test/functions/resolution/errors/testBadCallCandidates.9.good
A test/functions/resolution/errors/testBadCallCandidates.chpl
A test/functions/resolution/errors/testBadCallCandidates.compopts
M compiler/resolution/ResolutionCandidate.cpp
M compiler/resolution/functionResolution.cpp
M test/classes/deitz/inherit-class-record/inherit_mod1.good
M test/classes/deitz/inherit-class-record/inherit_mod2.good
M test/classes/deitz/inherit-class-record/inherit_mod3.good
M test/classes/deitz/method/method_call2.good
M test/classes/delete-free/owned/owned-record-instantiation-types-user-init-owns-error.good
M test/classes/generic/typeOfGenericField-fromTypeVariable.bad
M test/classes/initializers/promotion/copyInitLookalike.good
M test/classes/lydia/staticMethodErrorMessage.bad
M test/classes/nilability/multiple-errors.1.good
M test/classes/nilability/multiple-errors.2.good
M test/constrained-generics/basic/set1/error-concrete-arg.good
M test/distributions/ferguson/dist-assoc-bulkadd.bad
M test/functions/bradc/intUint3.good
M test/functions/bradc/paramThis/funParamThis.good
M test/functions/bradc/resolution/badEnumDispatch.good
M test/functions/bradc/resolution/intChoicesDupTypes-errors.good
M test/functions/bradc/resolution/smallIntBoolTests/boolInt-testenum.good
M test/functions/bradc/resolution/smallIntBoolTests/paramNon-withenum.good
M test/functions/bradc/resolution/smallIntBoolTests/realBool.good
M test/functions/bradc/resolution/smallIntBoolTests/realBool2.good
M test/functions/deitz/named/test_named3-error.good
M test/functions/deitz/test_overload_type_intent.good
M test/functions/deitz/test_write_type_error.good
M test/functions/ferguson/inout-error1.good
M test/functions/ferguson/query/query-generic-star-tuple-error.good
M test/functions/ferguson/query/query-generic-type-leaf-borrowed.good
M test/functions/ferguson/query/query-generic-type-leaf.good
M test/functions/lydia/method-iter-resolution-bug.bad
M test/functions/sungeun/resolutionErrorMsg.bad
M test/functions/varargs/typeToVarArgs.good
M test/library/standard/Reflection/mix-type-val-args.bad
M test/reductions/vass/reduce-of-more-promoted.bad
M test/types/bool/resolution/resolveQueryTypeW.bad
M test/types/enum/bradc/dispatch/dispatchenums.good
M test/types/enum/bradc/dispatch/enumarray.good
M test/types/enum/bradc/dispatch/enumdispatch.good
M test/types/enum/enumsLikeParams2.good
M test/types/partial/match-param.bad
M test/types/string/param-coercions.good
M test/types/tuple/lydia/mix-type-and-var.bad
M test/types/type_variables/deitz/test_query_expression_error.good
M test/types/type_variables/deitz/test_query_field11.good
M test/types/type_variables/deitz/test_query_field12.good
M test/types/type_variables/deitz/test_query_field7.good
M test/visibility/private/functions/privateOperatorError.good
M test/visibility/private/functions/privateOperatorError2.good
M test/visibility/shadow-overload.good
Compare: Comparing 94f20fe4db24...118399930924 · chapel-lang/chapel · GitHub