checker: report .wait() on non-thread arrays, unknown map methods and unknown enum values - #28794
MARCROCK22 wants to merge 3 commits into
Conversation
medvednikov
left a comment
There was a problem hiding this comment.
Reviewed at 8a687041d8d0013937c1020d636248959fc5a373.
No actionable correctness issue found in the three-file diff. The non-thread-array check avoids unresolved generic receivers and preserves an explicitly declared alias-chain wait method. The map change narrows the builtin fallback instead of changing the method implementations. For enum selectors, the new callable-member exemption covers declared static functions and receiver methods used as values, while escaped keyword fields are normalized separately from member lookup.
The diagnostic tests assert exact errors/counts and include positive map operations, alias-chain methods, enum function values, and escaped fields. One non-blocking coverage addition would be a positive builtin .wait() case for []thread and []thread int (including an alias of a thread-handle type), checking returned values where applicable. The new tests currently prove the non-thread rejection and the user-method exemption, but do not directly protect the intended builtin thread-array acceptance branch.
Validation: complete patch plus surrounding known-call classification and enum-name/member resolution code were inspected. I did not execute V tests or reproduce the reported baseline results because no V compiler is available locally. CI status and results were not considered.
50f3038 to
1193b62
Compare
…nd unknown enum values V1 reports these three mistakes; V3 accepted them, so they only failed later (in the C compiler or at run time) or not at all, and editors showed nothing. - `.wait()` on an array that is not an array of threads reports V1's error. A `wait()` declared on an alias of an array (or an alias of that alias) is still allowed. - A method maps do not have (`m.foo()`) reports "unknown function". - `Color.nope` and a `.nope` branch in a `match` on a `Color` report "unknown enum field". Static functions declared for the enum, methods passed as values and escaped keywords (`Mode.@none`) are still accepted.
Review follow-up: the diagnostic tests covered `.wait()` on arrays that are not thread handles and user `wait` methods, but not the builtin call they must keep accepting. The new test calls it on `[]thread int` and `[]thread` and uses the results as values of the element type. An alias of a thread handle type is left out: V1 rejects it (`aliases of `thread` types are not allowed`) and reports the same `.wait()` error this check does.
master now words unknown function errors as `unknown function: m.close` (2dcb74c), so the exact-message assertion of this test failed once the branch was merged with it. Assert the error kind and the named method instead: that is what this check adds, and the wording belongs to the existing unknown-call path.
1193b62 to
7296162
Compare
What
V3 does not report three mistakes that V1 reports. Programs with them get past the checker and then fail in the C compiler, crash at run time, or run with the mistake unnoticed. Editors show nothing either, since VLS takes its diagnostics from the default compiler. This PR adds the three checks to V3, with V1's behavior as the reference.
1.
.wait()on an array that is not an array of threadsThe message is the one V1 gives. A
wait()method declared for an alias of an array is still allowed, also through a chain of aliases (type B = A, whereAdeclares it): V3 compiles and runs that.2. A method that maps do not have
is_known_callreturnedtruefor any method called on a map. It now accepts the builtin map methods (clone,move,delete,clear,free,keys,values,reserve,str). Methods declared for maps are resolved beforeis_known_callis reached, so they are not affected.3. An enum value that does not exist
Still accepted:
fn Color.first():Color.first(),f := Color.first);apply(Color.label),colors.map(Color.label)), which V3 compiles and runs;from,from_stringandzero(for@[flag]enums);@(Mode.@none).Behavior change to note
Color.fromorPerm.zeroused as a value, without calling them, is now reported as an unknown enum field. Before this PR, V3 accepted it and the program then crashed at run time (invalid memory access) or failed in the C compiler. V1 does not compile it either.Tests
vlib/v/compiler_tests/checker_diagnostic_files_test.v, each seen failing before its fix../v vlib/v/compiler_tests/checker_diagnostic_files_test.v: 16 of 17 pass. The failing one,test_local_shadowing_in_child_scope_and_fn_literal_is_allowed, fails the same way on master.vlib/v/checker/tests(throughvlib/v/compiler_errors_test.v): 465 of 1360 fail, the same 465 as on master, so no new failures../v test vlib/v/types/: 23 of its 26 test files pass.storage_source_test.vandtype_cache_test.vfail the same way on master.checker_ownership_alias_test.v(built with-d ownership) did not finish in 40 minutes on this machine, and on master it did not finish in 30 minutes either.vlib/v/slow_tests/inout/compiler_test.vcannot run on master: its first fixture,dump_sumtype_of_fntype.vv, importsv.ast, which was removed in 2a7447b, and the runner panics.v test-allwas not run.Related
This PR adds the diagnostics; vlang/vls#520 adds the editor side of the same work: completion of enum values and static functions after
Color., and member completion for any expression. The two go together: the new errors only show up in the editor through VLS once this PR is merged. In code, neither PR needs the other to build or to pass its tests, so they can be merged in any order.