Skip to content

Commit 5f0c09d

Browse files
davleopodavleopo
committed
[GR-65021] Unify error object decoration.
PullRequest: graal/20888
2 parents 5463e86 + de1482b commit 5f0c09d

File tree

3 files changed

+163
-22
lines changed

3 files changed

+163
-22
lines changed

‎compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/debug/test/GraalErrorTest.java‎

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,17 @@
2727
import org.junit.Assert;
2828
import org.junit.Test;
2929

30+
import jdk.graal.compiler.api.directives.GraalDirectives;
31+
import jdk.graal.compiler.core.test.GraalCompilerTest;
32+
import jdk.graal.compiler.debug.Assertions;
3033
import jdk.graal.compiler.debug.GraalError;
34+
import jdk.graal.compiler.debug.TTY;
35+
import jdk.graal.compiler.nodes.FixedNode;
36+
import jdk.graal.compiler.nodes.LoopBeginNode;
37+
import jdk.graal.compiler.nodes.StructuredGraph;
38+
import jdk.graal.compiler.serviceprovider.GraalServices;
3139

32-
public class GraalErrorTest {
40+
public class GraalErrorTest extends GraalCompilerTest {
3341

3442
@FunctionalInterface
3543
interface Error {
@@ -79,4 +87,48 @@ private static void error(String msg, Error error) {
7987
Assert.assertTrue(ex.getMessage().contains(msg));
8088
}
8189
}
90+
91+
public static final boolean LOG_TTY = Boolean.parseBoolean(GraalServices.getSavedProperty("test.GraalErrorTest.stdout"));
92+
93+
static int foo(int limit) {
94+
int res = 0;
95+
for (int i = 0; i < limit; i++) {
96+
res += GraalDirectives.sideEffect(i);
97+
}
98+
return res;
99+
}
100+
101+
// run with -Dtest.GraalErrorTest.stdout=true to inspect how error messages are formatted
102+
@Test
103+
public void testErrorExampels() {
104+
StructuredGraph g = parseEager("foo", StructuredGraph.AllowAssumptions.NO);
105+
106+
FixedNode loopBeginNode = g.getNodes(LoopBeginNode.TYPE).first();
107+
108+
try {
109+
assert false : Assertions.errorMessage("Message", loopBeginNode);
110+
} catch (Throwable t) {
111+
if (LOG_TTY) {
112+
TTY.printf("Assertions.errorMessage(..) -> %s%n%n", t.getMessage());
113+
}
114+
}
115+
116+
try {
117+
assert false : Assertions.errorMessageContext("Message", loopBeginNode);
118+
} catch (Throwable t) {
119+
if (LOG_TTY) {
120+
TTY.printf("Assertions.errorMessageContext(..) -> %s%n%n", t.getMessage());
121+
}
122+
}
123+
124+
try {
125+
GraalError.guarantee(false, "Message %s", loopBeginNode);
126+
} catch (Throwable t) {
127+
if (LOG_TTY) {
128+
TTY.printf("Guarantee -> %s%n%n", t.getMessage());
129+
t.printStackTrace();
130+
}
131+
}
132+
133+
}
82134
}

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/debug/Assertions.java‎

Lines changed: 71 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,7 @@
2626

2727
import java.util.Arrays;
2828

29-
import jdk.graal.compiler.core.common.type.Stamp;
3029
import jdk.graal.compiler.nodeinfo.Verbosity;
31-
import jdk.graal.compiler.nodes.NodeView;
3230
import jdk.graal.compiler.nodes.ValueNode;
3331
import jdk.graal.compiler.nodes.cfg.HIRBlock;
3432
import jdk.graal.compiler.options.Option;
@@ -77,72 +75,128 @@ private static String formatAssertionContextArgV(Object... args) {
7775
for (int i = 0; i < args.length; i += 2) {
7876
Object val = args[i + 1];
7977
sb.append(args[i]).append("=");
80-
formatObjectContext(sb, val);
78+
sb.append(decorateObjectErrorContext(val));
8179
if (++entriesDone < entries) {
8280
sb.append(";");
8381
}
8482
}
8583
return sb.toString();
8684
}
8785

88-
private static void formatObjectContext(StringBuilder sb, Object val) {
86+
/**
87+
* Returns a string representation of the given object, enhancing it with additional contextual
88+
* information if such data is available.
89+
*/
90+
public static Object decorateObjectErrorContext(Object o) {
8991
try {
90-
if (val instanceof HIRBlock b) {
91-
sb.append(b.toString(Verbosity.All));
92-
} else if (val instanceof ValueNode v) {
93-
sb.append(v.toString(Verbosity.All));
94-
sb.append("/");
95-
Stamp stamp = v.stamp(NodeView.DEFAULT);
96-
sb.append(stamp).append(" ");
97-
sb.append(v.getStackKind());
98-
sb.append("\\");
92+
if (o instanceof HIRBlock b) {
93+
return b.toString(Verbosity.All);
94+
} else if (o instanceof ValueNode v) {
95+
return v.toString(Verbosity.All);
9996
} else {
100-
sb.append(val);
97+
return o;
10198
}
10299
} catch (Throwable e) {
103-
sb.append("Error calling toString on object ").append(e.getMessage());
100+
return "Error calling toString on object " + e.getMessage();
104101
}
105102
}
106103

104+
/**
105+
* Returns an enhanced error message for the supplied string and object. It considers arguments
106+
* one by one. Each argument is represented by its {@code toString} representation enhanced with
107+
* {@code decorateObjectErrorContext(arg[i])}. It uses
108+
* {@link #decorateObjectErrorContext(Object)} for the {@code toString} of the values.
109+
*
110+
* For example the following code
111+
*
112+
* <pre>
113+
* LoopBeginNode loopBegin = getLoopBegin();
114+
* Assertions.errorMessage("Message", loopBeginNode);
115+
* </pre>
116+
*
117+
* would be formatted to (for an arbitrary loop begin node with {@code id=6}, output is cropped
118+
* for brevity):
119+
*
120+
* {@code [0]=Message;[1]=6|LoopBegin { guestLoopEndsSafepointState...}}.
121+
*
122+
*/
107123
public static String errorMessage(Object... args) {
108124
StringBuilder sb = new StringBuilder();
109-
int entries = args.length;
125+
final int entries = args.length;
110126
int entriesDone = 0;
111127
for (int i = 0; i < args.length; i++) {
112128
Object val = args[i];
113129
sb.append("[").append(i).append("]=");
114-
formatObjectContext(sb, val);
130+
sb.append(decorateObjectErrorContext(val));
115131
if (++entriesDone < entries) {
116132
sb.append(";");
117133
}
118134
}
119135
return sb.toString();
120136
}
121137

138+
/**
139+
* Returns an enhanced error message for the supplied string and object. It considers arguments
140+
* pair wise in a key - value fashion. Each pair is then represented in its {@code toString}
141+
* representation of the form {@code key=decorateObjectErrorContext(value)}. It uses
142+
* {@link #decorateObjectErrorContext(Object)} for the {@code toString} of the values.
143+
*
144+
* For example the following code
145+
*
146+
* <pre>
147+
* LoopBeginNode loopBegin = getLoopBegin();
148+
* Assertions.errorMessageContext("Message", loopBeginNode);
149+
* </pre>
150+
*
151+
* would be formatted to (for an arbitrary loop begin node with {@code id=6}, output is cropped
152+
* for brevity):
153+
*
154+
* {@code Message=6|LoopBegin { guestLoopEndsSafepointState=ENABLED, splits=0,
155+
* cloneFromNodeId=-1}...}.
156+
*
157+
*/
122158
public static String errorMessageContext(String s1, Object o1) {
123159
return formatAssertionContextArgV(s1, o1);
124160
}
125161

162+
/**
163+
* See {@link #errorMessageContext(String, Object)}.
164+
*/
126165
public static String errorMessageContext(String s1, Object o1, String s2, Object o2) {
127166
return formatAssertionContextArgV(s1, o1, s2, o2);
128167
}
129168

169+
/**
170+
* See {@link #errorMessageContext(String, Object)}.
171+
*/
130172
public static String errorMessageContext(String s1, Object o1, String s2, Object o2, String s3, Object o3) {
131173
return formatAssertionContextArgV(s1, o1, s2, o2, s3, o3);
132174
}
133175

176+
/**
177+
* See {@link #errorMessageContext(String, Object)}.
178+
*/
134179
public static String errorMessageContext(String s1, Object o1, String s2, Object o2, String s3, Object o3, String s4, Object o4) {
135180
return formatAssertionContextArgV(s1, o1, s2, o2, s3, o3, s4, o4);
136181
}
137182

183+
/**
184+
* See {@link #errorMessageContext(String, Object)}.
185+
*/
138186
public static String errorMessageContext(String s1, Object o1, String s2, Object o2, String s3, Object o3, String s4, Object o4, String s5, Object o5) {
139187
return formatAssertionContextArgV(s1, o1, s2, o2, s3, o3, s4, o4, s5, o5);
140188
}
141189

190+
/**
191+
* See {@link #errorMessageContext(String, Object)}.
192+
*/
142193
public static String errorMessageContext(String s1, Object o1, String s2, Object o2, String s3, Object o3, String s4, Object o4, String s5, Object o5, String s6, Object o6) {
143194
return formatAssertionContextArgV(s1, o1, s2, o2, s3, o3, s4, o4, s5, o5, s6, o6);
144195
}
145196

197+
/**
198+
* See {@link #errorMessageContext(String, Object)}.
199+
*/
146200
public static String errorMessageContext(String s1, Object o1, String s2, Object o2, String s3, Object o3, String s4, Object o4, String s5, Object o5, String s6, Object o6, String s7,
147201
Object o7) {
148202
return formatAssertionContextArgV(s1, o1, s2, o2, s3, o3, s4, o4, s5, o5, s6, o6, s7, o7);

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/debug/GraalError.java‎

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
package jdk.graal.compiler.debug;
2626

2727
import java.util.ArrayList;
28+
import java.util.Arrays;
29+
import java.util.List;
2830
import java.util.Locale;
2931

3032
import jdk.vm.ci.code.Architecture;
@@ -255,8 +257,10 @@ public GraalError(String msg) {
255257
* @param args parameters to String.format - parameters that implement {@link Iterable} will be
256258
* expanded into a [x, x, ...] representation.
257259
*/
260+
@SuppressWarnings("this-escape")
258261
public GraalError(String msg, Object... args) {
259262
super(format(msg, args));
263+
potentiallyAddContext(args);
260264
}
261265

262266
/**
@@ -272,8 +276,42 @@ public GraalError(Throwable cause) {
272276
* This constructor creates a {@link GraalError} for a given causing Throwable instance with
273277
* detailed error message.
274278
*/
279+
@SuppressWarnings("this-escape")
275280
public GraalError(Throwable cause, String msg, Object... args) {
276281
super(format(msg, args), cause);
282+
potentiallyAddContext(args);
283+
}
284+
285+
private void potentiallyAddContext(Object[] args) {
286+
if (args != null) {
287+
List<String> betterContext = new ArrayList<>();
288+
for (int i = 0; i < args.length; i++) {
289+
if (args[i] instanceof Iterable<?>) {
290+
for (Object o : (Iterable<?>) args[i]) {
291+
String potentialBetterString = potentialBetterString(o);
292+
if (potentialBetterString != null) {
293+
betterContext.add(potentialBetterString);
294+
}
295+
}
296+
} else {
297+
String potentialBetterString = potentialBetterString(args[i]);
298+
if (potentialBetterString != null) {
299+
betterContext.add(potentialBetterString);
300+
}
301+
}
302+
}
303+
if (!betterContext.isEmpty()) {
304+
addContext(Arrays.toString(betterContext.toArray()));
305+
}
306+
}
307+
}
308+
309+
private static String potentialBetterString(Object o) {
310+
Object potentialBetterString = Assertions.decorateObjectErrorContext(o);
311+
if (!potentialBetterString.toString().equals(o.toString())) {
312+
return potentialBetterString.toString();
313+
}
314+
return null;
277315
}
278316

279317
/**
@@ -296,10 +334,7 @@ public static GraalError asGraalError(Throwable t) {
296334

297335
@Override
298336
public String toString() {
299-
StringBuilder str = new StringBuilder();
300-
str.append(super.toString());
301-
str.append(context());
302-
return str.toString();
337+
return super.toString() + context();
303338
}
304339

305340
public String context() {

0 commit comments

Comments
 (0)