Skip to content
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.

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

[GR-53454] Warn when @ReportPolymorphism is used incorrectly #9160

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Next Next commit
[GR-53454] Warn when @ReportPolymorphism is used incorrectly
* Remove duplicate .required() check.
  • Loading branch information
eregon committed Jun 24, 2024
commit 7feb67306426f27175626d1179d6c6c7ba8ed974
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
*/
package jdk.graal.compiler.truffle.test;

import com.oracle.truffle.api.dsl.test.ExpectWarning;
import org.graalvm.polyglot.Context;
import org.junit.Assert;
import org.junit.Before;
Expand Down Expand Up @@ -449,6 +450,7 @@ public void testTwoMegamorpicSpec2() {
testMegamorphicHelper(callTarget, args);
}

@ExpectWarning("This node uses @ReportPolymorphism on the class and @ReportPolymorphism.Megamorphic on some specializations, the latter annotation has no effect. Remove one of the annotations to resolve this.")
@NodeChild
abstract static class PolymorphicAndMegamorpic extends SplittingTestNode {

Expand Down
1 change: 1 addition & 0 deletions truffle/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ This changelog summarizes major changes between Truffle versions relevant to lan
* GR-40931 Virtual threads with a polyglot context are now experimentally supported on HotSpot. Experimental because access to caller frames in write or materialize mode is not yet supported and maximum 65535 threads concurrently accessing the context.
* GR-40931 Using virtual threads in a native-image will now emulate virtual threads using platform threads until Loom support for Truffle languages in native-image is implemented.
* GR-40931 Added [`TruffleThreadBuilder#virtual()`](https://www.graalvm.org/truffle/javadoc/com/oracle/truffle/api/TruffleThreadBuilder.html#virtual(boolean)) for languages to create virtual threads.
* GR-53454 Added warning in the annotation processor when `@ReportPolymorphism` is used incorrectly.

## Version 24.0.0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ private TruffleSuppressedWarnings() {
public static final String NEVERDEFAULT = "truffle-neverdefault";
public static final String INLINING_RECOMMENDATION = "truffle-inlining";
public static final String SHARING_RECOMMENDATION = "truffle-sharing";
public static final String SPLITTING = "truffle-splitting";
public static final String ABSTRACT_LIBRARY_EXPORT = "truffle-abstract-export";
public static final String ASSUMPTION = "truffle-assumption";
public static final String GUARD = "truffle-guard";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3018,9 +3018,7 @@ private CodeExecutableElement createExecuteAndSpecialize(boolean inlined) {

if (reportPolymorphismAction.required()) {
builder.end().startFinallyBlock();
if (reportPolymorphismAction.required()) {
generateCheckNewPolymorphismState(builder, frameState, reportPolymorphismAction);
}
generateCheckNewPolymorphismState(builder, frameState, reportPolymorphismAction);
builder.end();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,7 @@ public void setReachesFallback(boolean reachesFallback) {
this.reachesFallback = reachesFallback;
}

/** == [email protected]. */
public boolean isReportPolymorphism() {
return reportPolymorphism;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,7 @@ public NodeData parseNode(TypeElement originalTemplateType) {
verifyConstructors(node);
verifySpecializationThrows(node);
verifyFrame(node);
verifyReportPolymorphism(node);

if (isGenerateSlowPathOnly(node)) {
removeFastPathSpecializations(node, node.getSharedCaches());
Expand Down Expand Up @@ -4429,6 +4430,26 @@ private static void verifyFrame(NodeData node) {
}
}

private static void verifyReportPolymorphism(NodeData node) {
if (node.isReportPolymorphism()) {
List<SpecializationData> reachableSpecializations = node.getReachableSpecializations();
if (reachableSpecializations.size() == 1 && reachableSpecializations.get(0).getMaximumNumberOfInstances() == 1) {
node.addSuppressableWarning(TruffleSuppressedWarnings.SPLITTING,
"This node uses @ReportPolymorphism but has a single specialization instance, so the annotation has no effect. Remove the annotation or move it to another node to resolve this.");
}

if (reachableSpecializations.stream().noneMatch(SpecializationData::isReportPolymorphism)) {
node.addSuppressableWarning(TruffleSuppressedWarnings.SPLITTING,
"This node uses @ReportPolymorphism but all specializations use @ReportPolymorphism.Exclude. Remove some excludes or do not use ReportPolymorphism at all for this node to resolve this.");
}

if (reachableSpecializations.stream().anyMatch(SpecializationData::isReportMegamorphism)) {
node.addSuppressableWarning(TruffleSuppressedWarnings.SPLITTING,
"This node uses @ReportPolymorphism on the class and @ReportPolymorphism.Megamorphic on some specializations, the latter annotation has no effect. Remove one of the annotations to resolve this.");
}
}
}

private static String createMethodSignature(final ExecutableElement method) {
final StringBuilder result = new StringBuilder();
result.append(getSimpleName(method.getReturnType())).append(' ').append(getSimpleName((TypeElement) method.getEnclosingElement())).append("::").append(
Expand Down