Skip to content

Replace methods "create" in Op subclasses by constructors #584

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: code-reflection
Choose a base branch
from
+68 −77
Open

Replace methods "create" in Op subclasses by constructors #584

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
682fa55
Replace methods that create Ops from ExternalizedOp, by constructors
mabbay Sep 26, 2025
2c9a026
Remove unused constructors and methods
mabbay Sep 26, 2025
0e58642
Merge branch 'code-reflection' into rm-op-create
mabbay Sep 29, 2025
372cc17
Restore constructor and method that have usage
mabbay Sep 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 34 additions & 30 deletions src/jdk.incubator.code/share/classes/jdk/incubator/code/dialect/core/CoreOp.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public FuncOp body(Consumer<Block.Builder> c) {
final String funcName;
final Body body;

static FuncOp create(ExternalizedOp def) {
FuncOp(ExternalizedOp def) {
if (!def.operands().isEmpty()) {
throw new IllegalStateException("Bad op " + def.name());
}
Expand All @@ -100,17 +100,14 @@ static FuncOp create(ExternalizedOp def) {
case String s -> s;
case null, default -> throw new UnsupportedOperationException("Unsupported func name value:" + v);
});
return new FuncOp(funcName, def.bodyDefinitions().get(0));
}

FuncOp(FuncOp that, CopyContext cc, OpTransformer oa) {
this(that, that.funcName, cc, oa);
this(funcName, def.bodyDefinitions().get(0));
}

FuncOp(FuncOp that, String funcName, CopyContext cc, OpTransformer ot) {
FuncOp(FuncOp that, CopyContext cc, OpTransformer ot) {
super(that, cc);

this.funcName = funcName;
this.funcName = that.funcName;
this.body = that.body.transform(cc, ot).build(this);
}

Expand All @@ -123,6 +120,13 @@ public FuncOp transform(OpTransformer ot) {
return new FuncOp(this, CopyContext.create(), ot);
}

FuncOp(FuncOp that, String funcName, CopyContext cc, OpTransformer ot) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Group that with the other copying constructor, then its easier to see their relationship and whether we need to keep both.

super(that, cc);

this.funcName = funcName;
this.body = that.body.transform(cc, ot).build(this);
}

public FuncOp transform(String funcName, OpTransformer ot) {
return new FuncOp(this, funcName, CopyContext.create(), ot);
}
Expand Down Expand Up @@ -184,14 +188,14 @@ public static final class FuncCallOp extends CoreOp {
final String funcName;
final TypeElement resultType;

static FuncCallOp create(ExternalizedOp def) {
FuncCallOp(ExternalizedOp def) {
String funcName = def.extractAttributeValue(ATTRIBUTE_FUNC_NAME, true,
v -> switch (v) {
case String s -> s;
case null, default -> throw new UnsupportedOperationException("Unsupported func name value:" + v);
});

return new FuncCallOp(funcName, def.resultType(), def.operands());
this(funcName, def.resultType(), def.operands());
}

FuncCallOp(FuncCallOp that, CopyContext cc) {
Expand Down Expand Up @@ -241,12 +245,12 @@ public static final class ModuleOp extends CoreOp
final SequencedMap<String, FuncOp> table;
final Body body;

static ModuleOp create(ExternalizedOp def) {
ModuleOp(ExternalizedOp def) {
if (!def.operands().isEmpty()) {
throw new IllegalStateException("Bad op " + def.name());
}

return new ModuleOp(def.bodyDefinitions().get(0));
this(def.bodyDefinitions().get(0));
}

ModuleOp(ModuleOp that, CopyContext cc, OpTransformer ot) {
Expand Down Expand Up @@ -759,14 +763,15 @@ public static final class ConstantOp extends CoreOp
final Object value;
final TypeElement type;

static ConstantOp create(ExternalizedOp def) {
ConstantOp(ExternalizedOp def) {
if (!def.operands().isEmpty()) {
throw new IllegalArgumentException("Operation must have zero operands");
}

Object value = def.extractAttributeValue(ATTRIBUTE_CONSTANT_VALUE, true,
v -> processConstantValue(def.resultType(), v));
return new ConstantOp(def.resultType(), value);

this(def.resultType(), value);
}

static Object processConstantValue(TypeElement t, Object value) {
Expand Down Expand Up @@ -871,7 +876,7 @@ public static final class VarOp extends CoreOp
final String varName;
final VarType resultType;

static VarOp create(ExternalizedOp def) {
VarOp(ExternalizedOp def) {
if (def.operands().size() > 1) {
throw new IllegalStateException("Operation must have zero or one operand");
}
Expand All @@ -882,14 +887,11 @@ static VarOp create(ExternalizedOp def) {
case null -> "";
default -> throw new UnsupportedOperationException("Unsupported var name value:" + v);
});
// @@@ Cannot use canonical constructor because type is wrapped
return new VarOp(def, name);
}

VarOp(ExternalizedOp def, String varName) {
// @@@ Cannot use canonical constructor because type is wrapped
super(def.operands());

this.varName = varName;
this.varName = name;
this.resultType = (VarType) def.resultType();
}

Expand Down Expand Up @@ -1115,7 +1117,7 @@ public static final class TupleLoadOp extends CoreOp {

final int index;

static TupleLoadOp create(ExternalizedOp def) {
TupleLoadOp(ExternalizedOp def) {
if (def.operands().size() != 1) {
throw new IllegalStateException("Operation must have one operand");
}
Expand All @@ -1125,7 +1127,8 @@ static TupleLoadOp create(ExternalizedOp def) {
case Integer i -> i;
case null, default -> throw new UnsupportedOperationException("Unsupported tuple index value:" + v);
});
return new TupleLoadOp(def.operands().get(0), index);

this(def.operands().get(0), index);
}

TupleLoadOp(TupleLoadOp that, CopyContext cc) {
Expand Down Expand Up @@ -1172,7 +1175,7 @@ public static final class TupleWithOp extends CoreOp {

final int index;

static TupleWithOp create(ExternalizedOp def) {
TupleWithOp(ExternalizedOp def) {
if (def.operands().size() != 2) {
throw new IllegalStateException("Operation must have two operands");
}
Expand All @@ -1182,7 +1185,8 @@ static TupleWithOp create(ExternalizedOp def) {
case Integer i -> i;
case null, default -> throw new UnsupportedOperationException("Unsupported tuple index value:" + v);
});
return new TupleWithOp(def.operands().get(0), index, def.operands().get(1));

this(def.operands().get(0), index, def.operands().get(1));
}

TupleWithOp(TupleWithOp that, CopyContext cc) {
Expand Down Expand Up @@ -1230,17 +1234,17 @@ static Op createOp(ExternalizedOp def) {
case "cbranch" -> new ConditionalBranchOp(def);
case "closure" -> new ClosureOp(def);
case "closure.call" -> new ClosureCallOp(def);
case "constant" -> ConstantOp.create(def);
case "func" -> FuncOp.create(def);
case "func.call" -> FuncCallOp.create(def);
case "module" -> ModuleOp.create(def);
case "constant" -> new ConstantOp(def);
case "func" -> new FuncOp(def);
case "func.call" -> new FuncCallOp(def);
case "module" -> new ModuleOp(def);
case "quoted" -> new QuotedOp(def);
case "return" -> new ReturnOp(def);
case "tuple" -> new TupleOp(def);
case "tuple.load" -> TupleLoadOp.create(def);
case "tuple.with" -> TupleWithOp.create(def);
case "tuple.load" -> new TupleLoadOp(def);
case "tuple.with" -> new TupleWithOp(def);
case "unreachable" -> new UnreachableOp(def);
case "var" -> VarOp.create(def);
case "var" -> new VarOp(def);
case "var.load" -> new VarAccessOp.VarLoadOp(def);
case "var.store" -> new VarAccessOp.VarStoreOp(def);
case "yield" -> new YieldOp(def);
Expand Down
Loading