Just refactoring to remove some line (hopefully) - #528
Open
TApplencourt wants to merge 76 commits into
Open
Conversation
TApplencourt
force-pushed
the
handle-uuid-hoist
branch
from
August 13, 2026 18:30
ffb3d74 to
44e9396
Compare
opencl_tracepoints.rb defined its own class LTTng with duplicate print_tracepoint/print_enum logic, parallel to the shared utils/LTTng.rb module used by every other (AST-driven) backend. Since a bare `class LTTng` cannot coexist with utils/LTTng.rb's `module LTTng` (Ruby raises TypeError: LTTng is not a class), rename opencl's local tuple-parsing helper to `module LTTngFieldTuple` and drop its now-redundant print_tracepoint/print_enum, routing gen_opencl_probes.rb's two call sites through the shared LTTng.print_tracepoint instead. Also drop opencl_model.rb's duplicate MEMBER_SEPARATOR constant now that utils/LTTng.rb (required transitively via opencl_tracepoints.rb) defines the same value, eliminating a "already initialized constant" warning. Verified byte-identical against devel for opencl_tracepoints.tp, opencl_profiling.tp, opencl_model.yaml, btx_cl_model.yaml, and the remaining opencl_*.tp files, via direct diff and via utils/test_compare_generated_file.py (THAPI_FILTER=opencl). gen_babeltrace_cl_model.rb's parse_field was left unmerged with utils/gen_babeltrace_model_helper.rb's gen_bt_field_model: the latter depends on $types_by_name/$all_enum_names/$all_bitfield_names/ $all_struct_names/$integer_sizes/$integer_signed, all populated only by each AST backend's gen_*_library_base.rb from a parsed C AST. Opencl has no such AST pass (it parses cl.xml into OPENCL_MODEL instead), so building an equivalent global-population pipeline just to reuse gen_bt_field_model would be a large, unverifiable-by-byte-diff architectural addition. Left as documented partial unification (the existing shared gen_yaml call is retained). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
utils/test_compare_generated_file.py compared opencl_profiling.tp but not opencl_tracepoints.tp, the main opencl tracepoint output. Add it to the opencl file list so CI's check-same-generated-files job actually covers the file the opencl unification refactor touches. Verified: rerunning pytest with THAPI_FILTER=opencl against a clean devel baseline build and this branch's build still passes (2 passed, empty DeepDiff). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
OpenCL was the outlier with only opencl_tracepoints.tp/opencl_profiling.tp listed. Mirror the other backends (which guard their btx_*_model.yaml plus every *.tp) by adding the remaining opencl generated artifacts: tracer_opencl.c, btx_cl_model.yaml, opencl_model.yaml, and the arguments/build/devices/dump/source tracepoint files. All are byte-identical between devel and the opencl-unify refactor, so this only widens coverage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extract the six type-classification facts gen_bt_field_model reads from loose globals ($types_by_name, $all_enum_names, $all_bitfield_names, $all_struct_names, $integer_sizes, $integer_signed) plus to_scoped_class_name into an immutable TypeRegistry value object with integer_size/integer_signed? methods. A single global $type_registry is populated exactly as before; the top-level integer_size/integer_signed? helpers now delegate to it. No behaviour change: generated files remain byte-identical. New file utils/type_registry.rb wired into utils/Makefile.am EXTRA_DIST (verified via make distcheck). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Change gen_bt_field_model's signature to take a TypeRegistry as its first argument and read types_by_name / enum_names / bitfield_names / struct_names / integer_size / integer_signed? / class_namer from it instead of the loose globals and the top-level integer_size/integer_signed? helpers. The two internal call sites (gen_event_fields_bt_model, gen_extra_event_fields_bt_model) pass the global $type_registry, still the sole instance. No behaviour change: generated files remain byte-identical. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…als (AST backends) Add TypeRegistry.from_ast, which derives the integer-size/-sign lookups and the by-name type index from a backend's parsed AST (the enum/bitfield/struct name lists are still classified per-backend, since their rules differ). Each AST backend's gen_babeltrace_<x>_model.rb now constructs its own registry and passes it as the first argument through gen_event_bt_model / gen_extra_event_bt_model / gen_event_fields_bt_model / gen_bt_field_model. This removes ALL load-time side effects from gen_babeltrace_model_helper.rb: the $integer_sizes/$integer_signed/$types_by_name/$type_registry globals and the free integer_size/integer_signed? helpers are gone; requiring the helper now only defines functions. State is produced only by the explicit from_ast call that returns it. Generated files remain byte-identical for all backends. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…-time code gen_babeltrace_cl_model.rb defined empty INT_SIZE_MAP/INT_SIGN_MAP/$all_enums/ $all_types placeholders solely to satisfy gen_babeltrace_model_helper.rb's top-level load-time code (which read those globals to build $integer_sizes / $types_by_name). That load-time code was deleted when the helper moved to an explicitly-threaded TypeRegistry, so the placeholders are now dead. Removing them leaves opencl's generated btx_cl_model.yaml byte-identical. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The babeltrace-model de-globalization unified the six AST backends onto the shared gen_bt_field_model, but opencl's parse_field is deliberately left separate. Its de-globalization goal is already met (side-effect-free, reads OPENCL_MODEL explicitly), and merging it could not stay byte-identical: ctf_enum emits real CTF enumerations (AST backends have no such case), the input shape is a flat hash vs AST + LTTng objects, and pointer/signedness is encoded differently (bare-type signedness + explicit pointer flag vs a `*` in the type string whose rule forces unsigned). Comment records this as a reasoned decision so the next reader does not re-attempt it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ng reader Each AST model generator now asserts, right after building its TypeRegistry, whether the backend has bitfield types. Empirically (loading each gen_<x>_library_base and inspecting the populated name list) the runtime invariant is: cuda, hip, mpi and itt have zero bitfield types; ze (148) and omp (12) have them. Note itt's .push line exists but sits in a conditional that never fires for its actual types, so itt asserts empty -- the initial non-empty assertion failed the itt build, confirming the real invariant. Also documents the remaining $all_bitfield_names read in gen_babeltrace_lib_helper.rb: it is in the library/bindings path (not the de-globalized model path), runs after the caller has required its gen_<x>_library_base, and cannot be dropped while gen_library_base.rb still reads $all_struct_names. All 53 oracle files remain byte-identical. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…d scaffolding) Cherry-pick of devel 2614b7a onto opencl-unify-refactor, resolving the opencl_model.rb conflict: this branch keeps MEMBER_SEPARATOR in utils/LTTng.rb rather than inline, so only the dead GENERATE_ENUMS_TRACEPOINTS constant, the enum-tracepoint block it gated, and the dead early-return are removed. Removed content is byte-identical to 2614b7a on all 8 files (164 deletions total). Byte-identical-safe: GENERATE_ENUMS_TRACEPOINTS was permanently false, so the gated blocks never executed and no generated output changes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…aders After 2614b7a deleted the GENERATE_ENUMS_TRACEPOINTS block, ENUMS, ENUM_PARAM_NAME_MAP and ENUM_TYPES have no populator and stay empty forever. Remove the three declarations, the always-false `ENUM_TYPES.include?` guard in lttng_in_type, and unwrap both `if ENUM_PARAM_NAME_MAP[name]` branches in In/OutScalar (the enum branch could never be taken; keep the else body). Byte-identical-safe: every removed reader was dead (empty-collection lookups), so no generated output changes. Verified: 53/53 oracle cases identical to the devel baseline. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Every AST backend (cuda/hip/mpi/omp/itt/ze) reimplemented the same loop to
split typedef'd types into enum/bitfield/struct/union name lists, then derived
the `_flags_t` bitfield aliases. Replace all six with one call to a shared
classify_ast_types in utils/gen_library_base.rb.
Unify the bitfield predicate on name.end_with?('flag_t'). ze previously scanned
enum members for a ZE_BIT value expression; this is provably equivalent on the
built APIs (ze: 74 bitfields either way, 0 diff in both directions; cuda/hip/mpi/
itt: 0; omp: 6 -> 12 after _flags_t derivation), so no per-backend lambda is
needed and the ZE_BIT scan is removed. The now-unused empty-array initializers
for the four name-list globals are dropped (classify_ast_types returns them).
Also removes the dead `$all_enums.find { ... }` whose result cuda/hip/mpi
discarded.
Byte-identical: 53/53 oracle cases identical to the devel baseline.
Net -71 lines (-115 across backends, +32 shared fn + call sites).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Every AST backend repeated the same 6-line block per meta-parameter YAML: load the file, iterate meta_parameters, const_get the type, register. Fold that into a single load_meta_parameters(filename) helper in utils/command.rb and replace all 12 call sites (hip, mpi, omp, itt, cuda x2, cudart, ze x5). Byte-identical: oracle 53/53. opencl left untouched (its own Command class, does not require utils/command.rb). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The identical 3-line upper_snake_case helper was copy-pasted into 7 backend model files (itt, cuda, cudart, hip, ze, ompt, opencl). Define it once at the top of utils/LTTng.rb, which every backend already requires, and drop the duplicates. mpi has its own distinct underscore() and is untouched. Byte-identical: oracle 53/53. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
All 6 AST gen_babeltrace_<x>_model.rb files repeated the identical 5-line TypeRegistry.from_ast(...) call reading the same globals, followed by a per-backend bitfield-presence assertion. Fold both into build_ast_registry( backend, expect_bitfields:) in gen_babeltrace_model_helper.rb; the invariant (ze/omp have bitfields, cuda/hip/mpi/itt do not) is preserved via the flag. Byte-identical: oracle 53/53. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
All 6 AST gen_babeltrace_<x>_model.rb files repeated the same two loops: one building [start, stop] event pairs per command (or a single event for the itt/omp phased:false case), and one building the extra events declared in <x>_events.yaml. Extract gen_command_events_bt_model(registry, provider_commands, phased:) and gen_extra_events_bt_model(registry, filename) into gen_babeltrace_model_helper.rb and collapse the call sites. Byte-identical: oracle 53/53. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
All 6 AST gen_<x>_library_base.rb files derived $objects (pointer-to-struct typedefs plus CustomType aliases of OBJECT_TYPES) and $int_scalars (typedefs aliasing integer types) with the same two loops. Extract find_objects(all_types, extra:) and find_int_scalars(all_types) into utils/gen_library_base.rb; hip's one seeded name is threaded via extra:. Unlike load_meta_parameters these helpers RETURN their result (assigned explicitly at the call site) rather than mutating a global. Byte-identical: oracle 53/53. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
cuda, hip and mpi each carried a byte-identical 35-line copy of the Handle/UUID to_s modules inside their library heredoc. Extract the one copy into print_handle_uuid_modules and call it from the three backends. ze keeps its own copy: it uses :data/:id field names and prints the UUID bytes back to front, so it cannot share this helper as-is. Generated output is byte-identical (oracle: 53 passed). Net -55 LOC.
cuda, hip, mpi, itt and ze each defined print_<x>_object(object) whose whole body was print_object(object). Call the shared helper directly. Generated output is byte-identical (oracle: 53 passed).
The per-backend print_union (all five backends) and mpi's print_struct only re-bound the namespace argument. Pass the namespace at the call site instead; the indirection hid which namespace was in play. Backends that add real behaviour on top of the shared helper (cuda/hip/ze UUID prepends, itt's function-pointer rewriting, ze's version enums) keep their wrappers. Generated output is byte-identical (oracle: 53 passed).
Same pass-through shape as print_union: the wrapper only supplied the namespace. itt, omp and ze keep theirs -- those pick between enum and bitfield printing, or handle version enums. Generated output is byte-identical (oracle: 53 passed).
cuda and hip both prepended the UUID module for structs whose class name mentions UUID, differing only in namespace. Hoist to print_struct_prepending_uuid in gen_library_base. ze keeps its own: it also selects the KUUID module for kernel UUIDs. Generated output is byte-identical (oracle: 53 passed).
CI lints every changed .rb with `rubocop --display-only-safe-correctable`, which fails on any autocorrectable offence. Five came from earlier commits on this branch (argument alignment, a long line, a trailing comma, a while-modifier); the indentation one in gen_babeltrace_lib_helper.rb pre-dates the branch but CI lints that file because we touch it. Applied via targeted `rubocop -a`, not a blanket run: the wider codebase has ~1000 pre-existing offences (Style/GlobalVars, heredoc naming) that are out of scope here. Generated output is byte-identical (oracle: 53 passed on a clean build).
Running the generated-file comparison oracle (pytest) leaves utils/__pycache__ behind, which is easy to sweep into a commit by accident.
The comparison oracle gained backends/opencl/opencl_model.yaml and btx_cl_model.yaml, but the CI build step only asked for libOpenCL.la and opencl_profiling.tp. Neither pulls those two in -- btx_cl_model.yaml is a plain intermediate -- so the oracle hit FileNotFoundError on the base branch. Name them on the make line, as every other backend already does for its model yaml. Verified by reproducing the CI build locally: both files are absent before this change and present after, and identical between base and PR.
load_meta_parameters read content['meta_parameters'] and silently registered nothing when the key was absent, which is indistinguishable from a backend that genuinely has none. A typo in the key -- or the string-vs-symbol confusion -- therefore dropped every entry in the file without a word. Raise instead. A backend with no meta-parameters of its own already has a way to say so explicitly: `meta_parameters: []`, as cudart does. This check is what caught mpi_meta_parameters.yaml using the symbol key `:meta_parameters:`, which had left all of its entries dead since 9ae9297. That file was reconciled with the current code in f2ae743, so every backend builds.
load_meta_parameters populated a global META_PARAMETERS hash as a side
effect, and Command.new read it back by function name. What a backend's
`require` actually loaded, and who consumed it, was invisible at both ends.
It now returns the spec, and Command.new takes it explicitly:
meta_parameters = load_meta_parameters('mpi_meta_parameters.yaml')
Command.new(func, meta_parameters: meta_parameters[func.name])
Backends that split their rows over several files (ze per namespace, cuda
across its two APIs) pass all the filenames to one call. A function
declared in two of them used to silently concatenate both sets of rows;
that now raises.
The key must be a mapping, so a missing or misspelled `meta_parameters:`
is an error rather than an empty spec indistinguishable from a backend
that has none. cudart is the only backend with no rows of its own: its
empty YAML is deleted and it calls Command.new without a spec.
Also drops register_meta_parameter, register_meta_struct (no call sites)
and the Member class it was the sole constructor for.
Generated files are byte-identical across all seven backends.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every AST backend's model does `require_relative '../../utils/command'`, but only CUDA_MODEL listed the file. Editing utils/command.rb therefore regenerated cuda's tracers and models and silently left ze, hip, mpi, itt, omp and cudart stale, so a rebuild could mix generated files from two different versions of the generator. utils/type_registry.rb had the same problem one level down: it is required by gen_babeltrace_model_helper.rb, which every *_LIB_GEN does list, but the transitive dependency was never followed. opencl is unaffected -- it has its own Command class and loads neither file. Verified by touching both files and confirming all seven tracers and all six babeltrace models regenerate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`register_prologue 'clCreateBuffer', code` pushed into a PROLOGUES hash keyed by name, which Command#initialize then read back in its constructor. That read was also a write: the hash defaulted with `h[k] = []`, so looking up a command with no prologues created an entry for it, and the array Command kept was the very one the hash held. Later registrations landed only because of that aliasing -- the same trick applied to meta-parameters would have silently dropped them, since those were `+=`'d into a fresh array. Prologues and epilogues now live on the command object and nowhere else. Backends attach them through a CommandIndex built over their command lists, or call c.add_prologue directly where the loop already has the object in hand (itt's return-type loop, ze's ProcAddrTable loop). An unknown name raises rather than accumulating code onto a command that does not exist. opencl hand-rolled that check three times over; the index gives it to every backend for free, so those wrappers are gone. CommandIndex sits in its own file because opencl builds its commands from the Khronos XML with a Command class of its own and cannot require utils/command.rb. Verified byte-identical against f2ae743: 53 passed.
cuda_model.rb pushed 'CUdeviceptr' onto the shared HEX_INT_TYPES at require time -- the last in-place mutation of a classification constant among the AST backends. Anything requiring cuda_model got a different HEX_INT_TYPES than anything that did not, and nothing said so. find_all_types now takes extra_hex_ints and returns the merged list as TypeClasses#hex_ints, so cuda declares its one type as an input where the rest of its classification already comes from. HEX_INT_TYPES is frozen: a backend that still tries to patch it crashes instead of quietly changing what every other reader sees. Byte-identical: 53/53. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
RESULT_NAME, INIT_FUNCTIONS, STRUCT_MAP and TYPE_CLASSES were read by shared code in utils/ from whichever backend happened to be required. Nothing in those files said which backend they belonged to, and STRUCT_MAP was additionally a module-level hash grown by a require-time side effect of gen_struct_map. Every reader already holds a Command, and a Command belongs to exactly one backend, so the four facts travel with it now. Each model file builds one CONTEXT and passes it at Command.new; `context:` has no default, so a Command built without one raises instead of silently resolving whatever constant is in scope. gen_struct_map is renamed find_struct_map and returns its hash rather than filling a global. ze reads it through CONTEXT in both ze_model.rb and gen_ze_library.rb -- deliberately the map built from the full typedef list (incl. zex) rather than ApiModel's narrower one, which is missing zex_device_module_register_file_exp_t. cuda and cudart pass init_functions: nil: their generators call _init_tracer() from every wrapper, so no function is singled out and Command#init? is never asked. The old /.*/ suggested otherwise. opencl is untouched -- it has its own Command < CLXML and never requires utils/command.rb. Byte-identical: 53/53. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lttng_type took no arguments, so the three definitions that need to know how a name classifies -- Declaration, CustomType, Array -- read the TYPE_CLASSES constant of whichever backend was required. That was the last ambient read left in utils/. It now takes the classification as its first argument. Ten of the thirteen definitions ignore it (named _type_classes) but must accept it for polymorphism: any of them can be reached through Declaration. Every caller already holds a Command, which exposes it as #type_classes. Array's two `super` calls are spelled explicitly: Array#lttng_type also takes keywords, and a bare `super` would forward them to Type#lttng_type, which takes none, masking the intended raise with an ArgumentError. gen_probe_base.rb's `collect(&:lttng_type)` becomes a block -- a symbol-to-proc cannot pass an argument. Byte-identical: 53/53. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
gen_ze_library.rb was reaching into the Command layer's BackendContext for a struct layout while walking types it got from ApiModel -- two sources of the same fact, and the wrong one for the question being asked. ApiModel already derives objects, int_scalars and the name classifications from its own type list; the struct map is the same kind of derivation, so it belongs there too. The generator now asks the model it is walking. The two maps differ by exactly one key, zex_device_module_register_file_exp_t, which ApiModel's type list excludes. Measured across every ze generator: no zex key is ever read through the struct map, and the require-time ProcAddrTable scan in ze_model.rb selects an identical set of children for all 74 matching commands under either map. The zex struct tracepoints come from gen_zex_structs_tracepoints.rb, a Command-driven path that keeps reading the full map through CONTEXT. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CustomType#lttng_type and Array#lttng_type each ran the same five-branch ladder over the same five lists in the same order -- the order being load bearing, since an object typedef is also a pointer and a hex int is also an integer, but nothing said so. TypeClasses#category_of answers it once, and the two lttng_type methods switch on the category. Declaration#lttng_type asked only the last rung, so it gets aggregate? and loses its inner case entirely. The Array form no longer special-cases uint8_t's byte sizing: sizeof(uint8_t) is what the aggregate branch already computes for that name, so uint8_t just routes to :aggregate. Verified disjoint across all seven backends: no name in structs or unions appears in any earlier category, so no branch was relying on being shadowed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`YAMLCAst.load_file` handed back a hash of five lists that every backend then indexed with strings and passed, piecemeal, into a hand-built ApiModel. The model is what callers wanted, so it now owns the constructor: `ApiModel.load_file` parses and defaults in one place, and the six `gen_<x>_library_base.rb` blocks collapse from six lines to one. Backends spanning several namespaces add their models with `+` rather than concatenating five list pairs by hand. That merge is why the derivations are lazy -- summing three models would otherwise compute two answers nobody asked for -- and it is behaviour-neutral here: the lists the merge adds that the old hand-picked ones omitted are empty in every case (measured). The dependency runs one way: api_model.rb requires yaml_ast.rb, nothing requires back. Byte-identical: 53 passed.
The eleven $<x>_api globals existed so that gen_<x>_library_base.rb could reassign them to API. But the model file is what parses the api.yaml, so it is what should name the result: every backend now publishes API directly and the five `API = $<x>_api` lines are gone. ze keeps a second constant because it generates per-namespace artifacts -- one tracepoint provider and struct printer per namespace -- so six generators need one namespace's model specifically. APIS is that map, keyed by namespace, with zer an empty model rather than a missing key so the loops stay uniform. Having it collapses the six-way repetition in gen_ze.rb and gen_babeltrace_ze_model.rb into a loop over the map. The two ze merges stay distinct on purpose: the tracer intercepts zex, the Ruby bindings do not expose it. Unifying them would add six zex entry points to the public bindings -- a behaviour change, not a refactor. Byte-identical: 53 passed.
cudart_model.rb is a peer of cuda_model.rb, not a namespace inside it: separate context (cudaResult vs cuResult), separate tracer, and no generator loads both. It was already pure -- a local, not a global -- but it was the one model file not publishing its API under the shared name. Byte-identical: 53 passed.
Every backend built its commands into a global array, and then each generator that used them re-wrapped those arrays in a CommandIndex or in a hand-written [[provider, commands]] literal -- rebuilding at each use site a grouping the model already knew. The model now publishes one COMMANDS index, built from groups keyed by the LTTng provider that will carry them, and CommandIndex hands those groups back for the generators that emit one file per provider. So a tracepoint generator asks for COMMANDS.groups[provider] and the babeltrace model generator passes COMMANDS.groups straight through. opencl is the one backend whose two groups share a provider, so it keys on what actually separates them: an extension is reached through clGetExtensionFunctionAddress rather than dlsym. In ze this collapses the six copy-pasted command-list assignments into one loop over APIS, the five-way normal_wrapper fan-out into another, and the five hidden-alias blocks into a table of predicates that puts zel's opt-in rule next to the other four namespaces' opt-outs. Removes the last 15 $<x>_commands globals; only $event_lambdas, which is runtime library code rather than codegen, still remains. Generated output is byte-identical. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every other backend checks that each function its meta-parameter spec names is one the API actually defines; ze was the only one that did not, so a key that matched nothing there applied its rows to no command at all, silently. ze's five loaded specs declare 299 functions and all 299 resolve, so this passes today. It is a guard against future drift rather than a fix: the specs are written by hand against headers that keep moving. Generated output is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The five filenames were spelled out by hand next to an APIS map that already names exactly those namespaces, so adding a namespace meant remembering to add it in two places, and zer's exclusion lived in a comment rather than in code. Deriving the list keeps the per-namespace files -- each one sits beside the header it describes -- while making zer's exclusion a real subtraction that switching zer on will have to undo. Generated output is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
gen_zer_structs_tracepoints.rb selected zer's struct typedefs but resolved each one against zel's struct list -- a copy-paste slip from the file it was cloned from. The other five namespaces all match a typedef against their own structs. Inert today: zer is not in ZE_NAMESPACES, so the file is never built, and its api.yaml does not exist yet, so APIS[:zer] is an empty model and the select matches nothing either way. Confirmed by running the generator before and after -- byte-identical, just the two includes. It would have mattered the day zer is switched on: a zer typedef whose struct only exists in zer would have been silently dropped, and one whose name collided with a zel struct would have been classified against the wrong members. Generated output is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three unrelated-looking inconsistencies between backends turned out to be the same thing: how a generator learns which namespace a type name belongs to, decided independently six times. to_name_space failed two different ways. cuda/hip/mpi returned nil for an unrecognized name; ze/omp/itt wrote `.match(...)[1]` and raised NoMethodError on nil. That split was not a style choice -- it is a property of each API. ze/omp/itt/mpi headers declare only their own types, so nil is unreachable. cuda and hip vendor foreign ones (GLuint, dim3, VdpDevice, the OpenCL and VDPAU interop typedefs; 53 in hip, 12 in cuda) that belong to no namespace, so nil is a real answer their callers already handle. Both remain, but as two opt-in paths through one helper: match_name_space with strict: true for the four APIs that own every name they declare, plain for the two that do not. A future unprefixed ze type now raises by name here instead of surfacing as NoMethodError deep inside a generator. hip and mpi also had byte-identical to_class_name bodies -- same code, one substituted namespace -- now prefixed_class_name in gen_library_base. Verified equivalent on all 252 hip+mpi type names before switching. Note String#capitalize would downcase the rest and break HIP_ARRAY_DESCRIPTOR, so only the lowercase spelling is title-cased. Also drops mpi's vendored Rails `underscore`: every step of it was a no-op on all 1324 MPI names (the guard never taken, the camelCase splitter never fired, no '-' or '::' present, and its /(?=a)b/ gsub cannot match any string at all), leaving c.pointer_name.upcase. And nine lines of commented-out scaffolding in cuda_model.rb referencing $cuda_commands, a global this branch deleted. Byte-identical: 53/53. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The de-globalization commits added a lot of prose. Some of it was already
wrong, and a lot of it was the commit messages pasted into the source.
Wrong, and actively misleading the next reader:
* cuda_model.rb said "gen_cuda.rb calls _init_tracer() from every wrapper".
It does not -- normal_wrapper emits no init call; the only call site is
the _uninit trampoline each pointer starts at. The identical sentence in
cudart_model.rb IS true (gen_cudart.rb puts it in normal_wrapper), so the
two must not be "fixed" the same way.
* type_registry.rb said the enum/bitfield/struct rules "differ" per backend.
They did until c8d3510 collapsed all six loops into ApiModel#classified.
* yaml_ast.rb referenced POINTER_TYPES, deleted on this branch; the comment
was its last mention in the tree.
* meta_parameter_spec.rb said an empty meta_parameters mapping raises. It
does not: {} is a Hash and passes the guard.
* gen_babeltrace_model_helper.rb credited yaml_ast.rb with
ScalarMetaParameter (it is in meta_parameters.rb) and pointed at a
"bitfield note" that exists in a commit message, not the tree.
* yaml_ast.rb claimed category disjointness was "asserted"; it was verified
by hand once. Nothing will catch a regression, so the word is now honest.
* gen_library_base.rb's Handle/UUID note described ze, above the printer ze
does not call.
gen_ze.rb's commented-out zer line read struct_types[:zel] -- the same
copy-paste slip 0d64341 had just fixed in gen_zer_structs_tracepoints.rb,
lying in wait for whoever re-enables zer. Corrected in place rather than
deleted: zer scaffolding is deliberate.
The rest is trimming. Four-paragraph headers over fifteen-line functions, a
nineteen-line essay above opencl's parse_field ending "This separation is a
reasoned decision, not an unfinished TODO", and comments narrating what a
method named typedef? does. What survives is the why the code cannot state:
the opaque-forward-declaration distinction, why opencl's field parser cannot
merge with the AST one, why CommandIndex lives apart from command.rb.
Comments only, plus META_PARAMETER_NAMESPACES inlined into its one use.
Byte-identical: 53/53.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three abstractions this branch introduced were bigger than the job. TypeRegistry had two constructors: a 7-keyword `initialize` that only assigns, and a `from_ast` twelve lines above it that does the real derivation then forwards. One caller exists. Merged into one `initialize`, and dropped the `integer_sizes`/`integer_signed` attr_readers -- nothing outside the class ever read the raw hashes, only the two query methods that wrap them. ApiModel exposed `type_classes` and `objects` publicly; nobody outside called either. They now back `object?` and `int_scalars` from private. Its ClassifiedNames Struct was a named type whose only purpose was to be built once and immediately unwrapped by three delegating readers -- replaced by memoizing the three lists directly, which is what the readers returned. `ApiModel#+` stays. It has three call sites, two of them `inject(:+)` over a namespace list, which reads better than a `merge` would. Worth knowing it is concatenation, not set union: duplicate names across summed APIs would be double-counted, and nothing dedups. Verified none exist -- zero typedef names are shared between the ze namespaces. Byte-identical: 53/53. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The two anonymous struct typedefs, MPI_Status and MPI_F08_status, reached devel in "mpi: emit the anonymous struct typedefs instead of dropping them" (#531), so both sides of the oracle emit them and there is nothing left to warn about. This branch keeps the behaviour by a different route: the guard lives in API.struct, where every backend gets it, rather than in each generator. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TApplencourt
force-pushed
the
handle-uuid-hoist
branch
from
August 27, 2026 22:03
44e9396 to
2d3f5fb
Compare
Anonymous block forwarding, `def each(&)`, is Ruby 3.1 syntax, but configure.ac requires only 2.7 and rubocop parses as 2.7. The file raised a SyntaxError on a supported runtime, which CI reported as three Lint/Syntax offenses. Naming the parameter parses everywhere and reads no worse. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every backend opened by copying three readers into locals: typedefs = API.types structs = API.structs funcs = API.functions They are plain attr_readers, so the locals bought nothing. They are left over from the globals era, when a model had to alias $all_types to name it locally. What kept them alive was `find_struct_map(typedefs, structs)`, which seven backends called even though ApiModel#struct_map already memoizes exactly that call. Asking the model for the map leaves the locals with no reader at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every backend ran find_all_types on its own types, and ApiModel ran it again privately to answer object? and int_scalars -- the same classification of the same list, computed twice per backend. The model now publishes type_classes and the backends read it. cuda's hex integer moves with it: CUdeviceptr is a fact about the cuda API, so it is declared where that API is loaded rather than passed to a derivation call, and `+` unions the lists so a merged model keeps what either side named. That makes it available to any backend that later needs one, at no cost to those that do not. find_all_types' keyword loses its `extra_` prefix, which only distinguished it from the shared HEX_INT_TYPES it is added to. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Eight scripts each open-coded the same select: a struct typedef whose definition starts with an `stype` member. Two of them also dropped the `<ns>_base_` types, and wrote that filter two different ways. ze_model now answers both questions. stype_structs is every tagged struct, and concrete_stype_structs is the ones an API call can actually hand you -- the distinction the tracepoint scripts and the printer were already making silently, now named. Naming the namespace once also removes the shape that let a script select one namespace's typedefs and resolve them against another's structs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
In cuda, hip, mpi, itt, omp and cudart, API is the whole API the backend traces. ze had two aggregates and used the name for the smaller one: ze_model.rb all_api = all six namespaces (the tracer) gen_ze_library_base.rb API = ze, zet, zes, zel (the bindings) So API meant one thing in six backends and another in the seventh, and inside ze which one you got depended on the file you required. The union takes the name it has everywhere else. The bindings subset becomes BOUND_API, which says what it is: the namespaces that get a generated Ruby class. Both still exist -- excluding zex from the bindings is deliberate, and folding it in would add six functions and a type to the generated output. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
@thapi-local-reviewer review |
![thapi-local-reviewer[bot]](https://avatars.githubusercontent.com/u/6524907?s=60&v=4){kind=small}
This comment was marked as low quality.
This comment was marked as low quality.
Sorry, something went wrong.
`stype_structs` answered a general question under a Level Zero name. The question is "which structs carry a tag naming their own type", because that tag is what lets the tracer decode a `void *` at runtime -- and that is what earns a struct its own tracepoint. Level Zero spells the tag `stype`; the pattern is not Level Zero's. So the function becomes `tagged_structs(api, tag: 'stype')`. The default keeps every existing caller unchanged, and the parameter states which part is the vendor's and which part is ours. A backend whose API tags its extensible structs the same way asks the same question with its own spelling. The lookup also stops rescanning: `api.structs.find` walked all 381 ze structs once per typedef. `ApiModel#struct_named` indexes them by name, where the other derived facts already live. Struct names are unique in every api.yaml (checked across all backends), so the index and the scan agree by construction. Generated output is byte-identical: oracle 53/53.
The shared library generator called five things it never defined -- to_class_name, to_scoped_class_name, to_name_space, FFI_STRUCT, FFI_UNION -- and read a bare `API` constant, all resolved by whichever backend happened to be required first. Nothing declared that contract. Adding a backend meant discovering it one NameError at a time, and a divergence between two backends was invisible unless you diffed six files by hand. NamingContext makes it one object each backend constructs and passes in. It owns its api.yaml files and parses them once on first use, so the model is a read-only member rather than a global reached across files. Every field set away from a default is now a real divergence, stated in one line where the next reader will find it: hip, mpi nothing but a name and a pattern -- the defaults are their rule omp ffi_prefix: OMPT, because its FFI bases are OMPT* under module OMP cuda hex_ints, two hand-spelled class names, foreign vendored types itt a leading-underscore prefix the shared rule does not strip ze camelCase headers, four initialisms restored after the word split Four backends still pass a class_namer, which is exactly the list of things a general namer would have to absorb. That list was the goal: it is now readable without a diff. Two sites keep reading NAMING as a constant -- both inside the YAMLCAst monkey patch, which has no context in scope. Untangling that is a separate change. Generated output is byte-identical: oracle 53/53.
The shared generators in utils/ read two constants the backends happened to define: API and NAMING. That is what made them shared code that only works when required from the right place -- the reason gen_babeltrace_lib_helper.rb had to carry a comment explaining that API would be in scope. Both now arrive as arguments. NamingContext holds the API model the backend already built, so one object answers both questions and there is a single place that says which API a generator is talking about. `to_ffi` takes the context and hands it down, which is what lets the YAMLCAst patch stop reaching outward. NamingContext no longer parses api_files itself: it is handed the model the backend built, so nothing is parsed twice and `hex_ints` stops being stated in two places. ze is why this matters -- its NAMING listed four namespaces while API is six, so a re-parse would have quietly answered a different question than the constant it replaced. Also reattaches the prefixed_class_name comment, which the previous commit left stranded above the class definition inserted beneath it.
A cleanup pass over the generators, driven by flay and debride rather than by reading: every change below leaves the generated files byte-identical, checked per edit against the previous revision's own output and by the 53-file oracle. The tracers: six backends open a wrapper, fire an entry tracepoint, call the real function and fire an exit tracepoint, and each spelled all four out. utils/gen_tracer_base.rb states them once, so a backend is left saying only what differs -- where its function pointers live, and where its prologues and epilogues sit relative to the call. Two edge cases fold into that main path rather than hiding behind a flag, because measuring the models showed them vacuous elsewhere: no backend declares a function-pointer parameter, and only mpi declares an array one, so handling both unconditionally gives every backend the list it had. ze's pNext walk kept a pointer exemption for input structs that no input struct ever hit. The tracepoint providers: 19 generators were copies of the same 16 lines, differing in a model, a provider symbol and an include. print_tracepoint_provider takes those three, and its `directions:` argument now names the divergence the copies buried -- itt and ompt are callback APIs, one event per call, no return to observe. Dead code, all confirmed by grep before removal: two statements in gen_ompt.rb whose results were discarded, and gen_omp_library.rb's print_bitfield, which had no callers. Duplication elsewhere: struct, union and enum in yaml_ast.rb are one node shape parameterised four ways; tracepoint_gen.rb emitted four near-identical macro families per arity; gen_library_base.rb had two byte-identical layout printers and a struct/union branch that differed only in a class. api_model.rb and type_registry.rb freeze what they derive. Nothing there changes after construction, so a later write is a bug, and the freeze reports it where it happens instead of as output that depends on generation order. utils/Makefile.am was missing gen_probe_base.rb and gen_custom_probes.rb from EXTRA_DIST -- both are build dependencies, so a dist tarball could not regenerate. Added, with gen_tracer_base.rb, and verified by unpacking one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CI runs rubocop twice; the second pass has no --fail-level, so the nine convention offenses this branch introduced failed the build. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A tracepoint provider's events come from two places: the ones a backend writes out by hand in its <backend>_events.yaml, and the ones derived from the prototypes of the functions it traces. Nothing says a provider must pick one -- cuda_exports uses both, one declared event and 40 command pairs -- but the code was split as if it had to, so cuda_exports needed a block hook to reach across the divide. print_tracepoint_provider now takes events_path and emits the declared events itself, so gen_cuda_exports_tracepoints.rb is the same five lines as every other tracepoint generator and the yield hook is gone. gen_custom_probes.rb, which is the declared-events half on its own, reads them through the same helper instead of restating the loop. Every probe rule now names gen_probe_base.rb as a dependency. omp and itt had been listing it inside their _MODEL variables, which also feed five rules that never read it; the rest of the backends had not listed it at all. Byte-identical: all 19 tracepoint generators and six gen_custom_probes namespaces, plus the 53-file oracle. make dist still ships all three shared generators. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
No description provided.