fix(core): report unhashable string-enum arguments as ValueError, not TypeError - #2540
Open
LeSingh1 wants to merge 1 commit into
Open
fix(core): report unhashable string-enum arguments as ValueError, not TypeError#2540LeSingh1 wants to merge 1 commit into
LeSingh1 wants to merge 1 commit into
Conversation
`check_str_enum` exists to turn a bad option value into a ValueError that
names the accepted values. It tests membership against a set:
if value not in {m.value for m in enum_class}:
`x in <set>` hashes `x`, so an unhashable argument dies inside the check
before the ValueError is ever constructed:
>>> WorkqueueResourceOptions(sharing_scope=["green_ctx_balanced"])
TypeError: unhashable type: 'list'
>>> WorkqueueResourceOptions(sharing_scope="bogus")
ValueError: 'bogus' is not a valid WorkqueueSharingScopeType.
Must be 'device' or 'green_ctx_balanced'
The result is inconsistent by argument type rather than by validity: `5`,
`None`, `object()`, and even a `set` all get the intended ValueError (CPython
special-cases `set.__contains__` to retry unhashable arguments as a
frozenset), while a list, dict, or bytearray gets a TypeError from a helper
whose entire job is to raise ValueError.
Test membership against a tuple instead. Tuple membership compares with
`==` and never hashes, so unhashable values simply compare unequal and fall
into the existing error path. Every currently-accepted value keeps being
accepted, and every currently-rejected value keeps its exact message.
The affected validator reaches user input from
`WorkqueueResourceOptions.__post_init__` (_device_resources.pyx),
`ManagedMemoryResourceOptions` handling (_managed_memory_resource.pyx),
`make_program_cache_key(code_type=...)` (utils/_program_cache/_keys.py,
which passes non-str values straight through), and graph alloc-node memory
types.
`validators.py` had no tests; this adds the first ones for both
`check_str_enum` and `format_or_list`.
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
cuda/core/_utils/validators.py::check_str_enumexists to turn a bad option value into aValueErrorthat names the accepted values. It tests membership against a set:x in <set>hashesx, so an unhashable argument dies inside the membership test, before theValueErroris ever built:The outcome depends on the type of the argument rather than on its validity. Measured against
check_str_enum(value, SourceCodeType)onmain:main"fortran"ValueError(intended)5,None,object()ValueError(intended){"c++"}(aset)ValueError— CPython special-casesset.__contains__to retry an unhashable argument as a frozenset["c++"]TypeError: unhashable type: 'list'{"code_type": "c++"}TypeError: unhashable type: 'dict'bytearray(b"c++")TypeError: unhashable type: 'bytearray'A helper whose entire job is to raise
ValueErrorleaks aTypeErrorfrom its own implementation detail, and the message says nothing about which values are accepted.WorkqueueResourceOptions.__post_init__is a plain dataclass hook — the failure happens at construction, with no device or context involved.Fix
Test membership against a tuple. Tuple membership compares with
==and never hashes, so an unhashable value simply compares unequal and falls into the existing error path.SourceCodeType.CXX) and theallow_none=Truepath.TypeErrorrows above change, and they change to theValueErrorthe function documents.The enums involved have at most a handful of members, so tuple membership is not a meaningful cost.
Where this is reachable
check_str_enumvalidates user input from:WorkqueueResourceOptions.__post_init__(_device_resources.pyx:152)ManagedMemoryResourceOptionspreferred-location handling (_memory/_managed_memory_resource.pyx:178)make_program_cache_key(code_type=...)(utils/_program_cache/_keys.py:756; the precedingcode_type.lower() if isinstance(code_type, str) else code_typepasses non-str values straight through)graph/_graph_node.pyx:851)Tests
validators.pyhad no tests at all —grep -rn "check_str_enum" cuda_core/tests/returns nothing today. This addscuda_core/tests/test_utils_validators.pycoveringformat_or_list(0/1/2/3 values and theNone-first shape used byallow_none),check_str_enum's accept path (raw values and enum members), theallow_nonebehavior, the invalid-value path parametrized over hashable and unhashable arguments, and the publicWorkqueueResourceOptionsentry point. None of it needs a GPU.Verification I could and could not do
validators.pystandalone (it has no imports) and ran the full parametrization against both theupstream/mainimplementation and the fixed one. Onmainthelist,dict, andbytearraycases raiseTypeErrorand fail; with the fix all cases raise the expectedValueError. Every accept case and everyformat_or_listoutput is byte-identical before and after.ruff check/ruff format --checkclean on both changed Python files;toolshed/check_spdx.pyclean on the new file;python -m py_compileclean.pytest cuda_core/tests/test_utils_validators.pyas written. I have no environment wherecuda.coreis importable (no CUDA driver, no built extension modules), so the file's imports could not be exercised locally. The two names it pulls fromcuda.core—WorkqueueResourceOptionsandcuda.core.typing.SourceCodeType— follow existing usage intest_green_context.pyandtest_program_cache.py. Please treat CI as the first real run.