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

refactor!: Remove tracking of "culprit" from Require methods #5739

Merged
merged 3 commits into from
Jul 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
10 changes: 5 additions & 5 deletions Base/src/main/java/io/deephaven/base/stats/Composite.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,13 @@ public Composite(long now, Value... values) {

// ----------------------------------------------------------------
private static Value[] checkValues(Value[] values) {
Require.neqNull(values, "values", 1);
Require.gtZero(values.length, "values.length", 1);
Require.neqNull(values[0], "values[0]", 1);
Require.neqNull(values, "values");
Require.gtZero(values.length, "values.length");
Require.neqNull(values[0], "values[0]");
char typeTag = values[0].getTypeTag();
for (int nIndex = 1; nIndex < values.length; nIndex++) {
Require.neqNull(values[nIndex], "values[nIndex]", 1);
Require.eq(values[nIndex].getTypeTag(), "values[nIndex].getTypeTag()", typeTag, "typeTag", 1);
Require.neqNull(values[nIndex], "values[nIndex]");
Require.eq(values[nIndex].getTypeTag(), "values[nIndex].getTypeTag()", typeTag, "typeTag");
}
return values;
}
Expand Down
2,118 changes: 513 additions & 1,605 deletions Base/src/main/java/io/deephaven/base/verify/Require.java

Large diffs are not rendered by default.

109 changes: 2 additions & 107 deletions Base/src/main/java/io/deephaven/base/verify/RequirementFailure.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,122 +3,17 @@
//
package io.deephaven.base.verify;

import java.io.PrintStream;
import java.io.PrintWriter;
import java.io.StringWriter;

// ----------------------------------------------------------------
/**
* {@link RuntimeException} to be thrown on requirement failures.
*/
public class RequirementFailure extends RuntimeException {

/**
* The number of stack frames that should be removed from the stack to find the method whose requirements did not
* hold.
*/
private int m_nCallsBelowRequirer;

// ----------------------------------------------------------------
public RequirementFailure(String message, int nCallsBelowRequirer) {
public RequirementFailure(String message) {
super(message);
m_nCallsBelowRequirer = nCallsBelowRequirer;
}

// ----------------------------------------------------------------
public RequirementFailure(String message, Exception caughtException, int nCallsBelowRequirer) {
public RequirementFailure(String message, Exception caughtException) {
super(message, caughtException);
m_nCallsBelowRequirer = nCallsBelowRequirer;
}

// ----------------------------------------------------------------
/**
* Gets the number of stack frames that should be removed from the stack to find the caller which failed to meet
* requirements.
*/
public int getNumCallsBelowRequirer() {
return m_nCallsBelowRequirer;
}

// ----------------------------------------------------------------
@Override
public void printStackTrace() {
printStackTrace(System.err);
}

// ----------------------------------------------------------------
@Override
public void printStackTrace(PrintStream s) {
s.print(getFixedStackTrace());
}

// ----------------------------------------------------------------
@Override
public void printStackTrace(PrintWriter s) {
s.print(getFixedStackTrace());
}

// ----------------------------------------------------------------
/**
* Gets a stack trace with a line added identifying the offending stack frame.
*/
private StringBuffer getFixedStackTrace() {
StringBuffer sb = getOriginalStackTrace();

String sStackTrace = sb.toString();
int nInsertPoint = sStackTrace.indexOf("\n\tat ");
for (int nCount = m_nCallsBelowRequirer; nCount >= 0; nCount--) {
nInsertPoint = sStackTrace.indexOf("\n\tat ", nInsertPoint + 1);
}
if (-1 != nInsertPoint) {
sb.insert(nInsertPoint, "\n (culprit:)");
}
return sb;
}

// ----------------------------------------------------------------
/**
* Gets the unmodified stack trace, instead of the one with the culprit identified.
*/
public StringBuffer getOriginalStackTrace() {
StringWriter stringWriter = new StringWriter();
PrintWriter printWriter = new PrintWriter(stringWriter);
super.printStackTrace(printWriter);
printWriter.close();
return stringWriter.getBuffer();
}

// ----------------------------------------------------------------
/**
* If this stack frame caused the exception, adjust the culprit to be the caller. Used when a delegating method
* can't verify all requirements itself but shouldn't receive the blame.
*/
public RequirementFailure adjustForDelegatingMethod() {
if (isThisStackFrameCulprit(1)) {
m_nCallsBelowRequirer++;
}
return this;
}

// ----------------------------------------------------------------
/**
* If this stack frame caused the exception, adjust the culprit to be the caller. Used when a delegating method
* can't verify all requirements itself but shouldn't receive the blame.
*/
public RequirementFailure adjustForDelegatingMethodAndSyntheticAccessor() {
if (isThisStackFrameCulprit(0)) {
m_nCallsBelowRequirer += 2;
}
return this;
}

// ----------------------------------------------------------------
/**
* Returns true if this stack frame is blamed for causing the exception.
*/
public boolean isThisStackFrameCulprit(int nFramesBelowTargetFrame) {
StackTraceElement[] stackTrace = new Throwable().getStackTrace();
StackTraceElement[] failureStackTrace = getStackTrace();
return failureStackTrace.length - m_nCallsBelowRequirer == stackTrace.length - nFramesBelowTargetFrame;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,7 @@ public void testLowGarbageArrayIntegerMap() {
try {
integerToStringMap.put(-1, "negative one");
fail("expected bad index to fail");
} catch (RequirementFailure requirementFailure) {
assertTrue(requirementFailure.isThisStackFrameCulprit(-1));
} catch (RequirementFailure expected) {
}

try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,8 @@ public void testThreadSafeLenientFixedSizePoolNoFactory() {
// too small
try {
new ThreadSafeLenientFixedSizePool<Object>(6, m_mockObjectFactory, m_mockClearingProcedure);
} catch (RequirementFailure requirementFailure) {
assertTrue(requirementFailure.isThisStackFrameCulprit(0));
fail("expected to throw");
} catch (RequirementFailure expected) {
}

// minimum size
Expand All @@ -156,15 +156,15 @@ public void testThreadSafeLenientFixedSizePoolNoFactory() {
// no factory
try {
ThreadSafeLenientFixedSizePool.FACTORY.create(OBJECTS.length, null, m_mockClearingProcedure);
} catch (RequirementFailure requirementFailure) {
assertTrue(requirementFailure.isThisStackFrameCulprit(0));
fail("expected to throw");
} catch (RequirementFailure expected) {
}

// too small
try {
ThreadSafeLenientFixedSizePool.FACTORY.create(6, m_mockObjectFactory, m_mockClearingProcedure);
} catch (RequirementFailure requirementFailure) {
assertTrue(requirementFailure.isThisStackFrameCulprit(0));
fail("expected to throw");
} catch (RequirementFailure expected) {
}

// minimum size
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,29 +30,17 @@ public class CollectionUtil {

// ----------------------------------------------------------------
public static <K, V> Map<K, V> unmodifiableMapFromArray(Class<K> typeK, Class<V> typeV, Object... data) {
try {
return Collections.unmodifiableMap(mapFromArray(typeK, typeV, data));
} catch (RequirementFailure e) {
throw e.adjustForDelegatingMethod();
}
return Collections.unmodifiableMap(mapFromArray(typeK, typeV, data));
}

// ----------------------------------------------------------------
public static <K, V> Map<K, V> unmodifiableInvertMap(Map<V, K> sourceMap) {
try {
return Collections.unmodifiableMap(invertMap(sourceMap));
} catch (RequirementFailure e) {
throw e.adjustForDelegatingMethod();
}
return Collections.unmodifiableMap(invertMap(sourceMap));
}

// ----------------------------------------------------------------
public static <E> Set<E> unmodifiableSetFromArray(E... data) {
try {
return Collections.unmodifiableSet(setFromArray(data));
} catch (RequirementFailure e) {
throw e.adjustForDelegatingMethod();
}
return Collections.unmodifiableSet(setFromArray(data));
}

// ----------------------------------------------------------------
Expand Down
11 changes: 2 additions & 9 deletions Util/src/main/java/io/deephaven/util/Utils.java
Original file line number Diff line number Diff line change
Expand Up @@ -375,19 +375,12 @@ public static String getSimpleNameFor(@NotNull Class<?> objectClass) {
/**
* require (o instanceof type)
*/
public static <T> T castTo(Object o, String name, Class<T> type, int numCallsBelowRequirer) {
io.deephaven.base.verify.Require.instanceOf(o, name, type, numCallsBelowRequirer);
public static <T> T castTo(Object o, String name, Class<T> type) {
io.deephaven.base.verify.Require.instanceOf(o, name, type);
// noinspection unchecked
return (T) o;
}

/**
* require (o instanceof type)
*/
public static <T> T castTo(Object o, String name, Class<T> type) {
return castTo(o, name, type, 1);
}

/**
* Describe the object in a standardized format without accessing its fields or otherwise risking interacting with
* partially-initialized state.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,8 @@ public void testThreadSafeLenientFixedSizePoolNoFactory() {
try {
new io.deephaven.base.pool.ThreadSafeLenientFixedSizePool<Object>(6, m_mockObjectFactory,
m_mockClearingProcedure);
} catch (RequirementFailure requirementFailure) {
assertTrue(requirementFailure.isThisStackFrameCulprit(0));
fail("expected to throw");
} catch (RequirementFailure expected) {
}

// minimum size
Expand All @@ -162,16 +162,16 @@ public void testThreadSafeLenientFixedSizePoolNoFactory() {
try {
io.deephaven.base.pool.ThreadSafeLenientFixedSizePool.FACTORY.create(OBJECTS.length, null,
m_mockClearingProcedure);
} catch (RequirementFailure requirementFailure) {
assertTrue(requirementFailure.isThisStackFrameCulprit(0));
fail("expected to throw");
} catch (RequirementFailure expected) {
}

// too small
try {
io.deephaven.base.pool.ThreadSafeLenientFixedSizePool.FACTORY.create(6, m_mockObjectFactory,
m_mockClearingProcedure);
} catch (RequirementFailure requirementFailure) {
assertTrue(requirementFailure.isThisStackFrameCulprit(0));
fail("expected to throw");
} catch (RequirementFailure expected) {
}

// minimum size
Expand Down
Loading