Skip to content

Conversation

@sshekhar-04
Copy link
Contributor

This PR fixes Issue #615, where certain generated operation overloads (secondary factory methods) were incorrectly using array types (Options[]) instead of variadic arguments (Options...).

The Problem
In the ClassGenerator, the buildSecondaryFactory method was responsible for creating overloads of operations (e.g., topK without the indexType attribute). While the main factory method correctly used JavaPoet's .varargs() for the options parameter, the secondary factory was simply adding the parameter without setting the variadic flag. This led to an inconsistent API where some methods required an explicit array creation to pass optional attributes.

The Fix
Modified ClassGenerator.java within the buildSecondaryFactory method to check for the options parameter. When the options parameter is encountered, we now explicitly call "factoryBuilder.varargs()" .

Changes in ClassGenerator.java:
if (attr != null && resolver.typeOf(attr).shouldWrapInClass() && defaultTypes.containsKey(attr)) { body.add("$T.class", defaultTypes.get(attr)); } else { factoryBuilder.addParameter(param); // NEW: Ensure variadic property is preserved for Options if (param.name.equals("options")) { factoryBuilder.varargs(); } factoryBuilder.addJavadoc("\n@param $L $L", param.name, paramTags.get(param.name)); typeVars.addAll(new ResolvedType(param.type).findGenerics()); body.add("$L", param.name); }

Verification Results
Due to local environment constraints with the native build system, verification was focused on the logic within the tensorflow-core-generator. The fix ensures that the secondary factory now mirrors the variadic behavior of the primary create method, resulting in:

  • Before: topK(Operand input, Operand k, Options[] options)
  • After: topK(Operand input, Operand k, Options... options)

Mentors
Directing this to @karllessard and @Craigacp for review, as discussed in the issue tracker regarding the ClassGenerator logic and the desired consistency for the Java API overloads.

@karllessard
Copy link
Collaborator

karllessard commented Jan 22, 2026

Thanks a lot for the fix @sshekhar-04! I suppose that should works. But there are some things you need to do first to validate that the fix works:

  • Run mvn clean install -Pgenerating and verify if the ops are correctly generated.

    • You need to install Bazel prior to run this command, instructions can be found here. Let us know if you are encountering issues
  • Confirm that resulting changes reflect what (and only what) you wanted to modify.

    • If so, include all files (including the modified generated ones) in your commit
  • Finally run mvn spotless:apply, to make sure that the code you are submitting is formatted correctly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants