# HG changeset patch
# User Michael Van De Vanter  <michael.van.de.vanter@oracle.com>
# Date 1422560654 28800
# Node ID c7e57dffc5adb848bd21a566773e740662af1a54
# Parent  cc2b817de0b598cca69a7041db482a238642d14f
Truffle/Instrumentation:  comments and minor code cleanup post-review

diff -r cc2b817de0b5 -r c7e57dffc5ad graal/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/TruffleTool.java
--- a/graal/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/TruffleTool.java	Thu Jan 29 11:27:19 2015 +0100
+++ b/graal/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/TruffleTool.java	Thu Jan 29 11:44:14 2015 -0800
@@ -66,16 +66,23 @@
 public abstract class TruffleTool {
 
     private enum ToolState {
+
+        /** Not yet installed, inert. */
         UNINSTALLED,
-        ENABLED_INSTALLED,
-        DISABLED_INSTALLED,
+
+        /** Installed, collecting data. */
+        ENABLED,
+
+        /** Installed, not collecting data. */
+        DISABLED,
+
+        /** Was installed, but now removed, inactive, and no longer usable. */
         DISPOSED;
     }
 
     private ToolState toolState = ToolState.UNINSTALLED;
 
     protected TruffleTool() {
-
     }
 
     /**
@@ -85,11 +92,9 @@
      * @throws IllegalStateException if the tool has previously been installed.
      */
     public final void install() {
-        if (toolState != ToolState.UNINSTALLED) {
-            throw new IllegalStateException("Tool " + getClass().getSimpleName() + " has already been installed");
-        }
+        checkUninstalled();
         if (internalInstall()) {
-            toolState = ToolState.ENABLED_INSTALLED;
+            toolState = ToolState.ENABLED;
         }
     }
 
@@ -97,7 +102,7 @@
      * @return whether the tool is currently collecting data.
      */
     public final boolean isEnabled() {
-        return toolState == ToolState.ENABLED_INSTALLED;
+        return toolState == ToolState.ENABLED;
     }
 
     /**
@@ -107,14 +112,9 @@
      * @throws IllegalStateException if not yet installed or disposed.
      */
     public final void setEnabled(boolean isEnabled) {
-        if (toolState == ToolState.UNINSTALLED) {
-            throw new IllegalStateException("Tool " + getClass().getSimpleName() + " not yet installed");
-        }
-        if (toolState == ToolState.DISPOSED) {
-            throw new IllegalStateException("Tool " + getClass().getSimpleName() + " has been disposed");
-        }
+        checkInstalled();
         internalSetEnabled(isEnabled);
-        toolState = isEnabled ? ToolState.ENABLED_INSTALLED : ToolState.DISABLED_INSTALLED;
+        toolState = isEnabled ? ToolState.ENABLED : ToolState.DISABLED;
     }
 
     /**
@@ -123,12 +123,7 @@
      * @throws IllegalStateException if not yet installed or disposed.
      */
     public final void reset() {
-        if (toolState == ToolState.UNINSTALLED) {
-            throw new IllegalStateException("Tool " + getClass().getSimpleName() + " not yet installed");
-        }
-        if (toolState == ToolState.DISPOSED) {
-            throw new IllegalStateException("Tool " + getClass().getSimpleName() + " has been disposed");
-        }
+        checkInstalled();
         internalReset();
     }
 
@@ -139,12 +134,7 @@
      * @throws IllegalStateException if not yet installed or disposed.
      */
     public final void dispose() {
-        if (toolState == ToolState.UNINSTALLED) {
-            throw new IllegalStateException("Tool " + getClass().getSimpleName() + " not yet installed");
-        }
-        if (toolState == ToolState.DISPOSED) {
-            throw new IllegalStateException("Tool " + getClass().getSimpleName() + " has been disposed");
-        }
+        checkInstalled();
         internalDispose();
         toolState = ToolState.DISPOSED;
     }
@@ -156,7 +146,7 @@
 
     /**
      * No subclass action required.
-     * 
+     *
      * @param isEnabled
      */
     protected void internalSetEnabled(boolean isEnabled) {
@@ -166,4 +156,29 @@
 
     protected abstract void internalDispose();
 
+    /**
+     * Ensure that the tool is currently installed.
+     *
+     * @throws IllegalStateException
+     */
+    private void checkInstalled() throws IllegalStateException {
+        if (toolState == ToolState.UNINSTALLED) {
+            throw new IllegalStateException("Tool " + getClass().getSimpleName() + " not yet installed");
+        }
+        if (toolState == ToolState.DISPOSED) {
+            throw new IllegalStateException("Tool " + getClass().getSimpleName() + " has been disposed");
+        }
+    }
+
+    /**
+     * Ensure that the tool has not yet been installed.
+     *
+     * @throws IllegalStateException
+     */
+    private void checkUninstalled() {
+        if (toolState != ToolState.UNINSTALLED) {
+            throw new IllegalStateException("Tool " + getClass().getSimpleName() + " has already been installed");
+        }
+    }
+
 }
diff -r cc2b817de0b5 -r c7e57dffc5ad graal/com.oracle.truffle.api/src/com/oracle/truffle/api/nodes/Node.java
--- a/graal/com.oracle.truffle.api/src/com/oracle/truffle/api/nodes/Node.java	Thu Jan 29 11:27:19 2015 +0100
+++ b/graal/com.oracle.truffle.api/src/com/oracle/truffle/api/nodes/Node.java	Thu Jan 29 11:44:14 2015 -0800
@@ -437,6 +437,9 @@
      * that intercepts execution events at the node and routes them to any {@link Instrument}s that
      * have been attached to the {@link Probe}. Only one {@link Probe} may be installed at each
      * node; subsequent calls return the one already installed.
+     * <p>
+     * <b>Note:</b> instrumentation requires a appropriate {@link WrapperNode}, which must be
+     * provided by {@link #createWrapperNode()}.
      *
      * @see Instrument
      */
@@ -449,7 +452,7 @@
      * <ol>
      * <li>implements {@link WrapperNode}</li>
      * <li>has {@code this} as it's child, and</li>
-     * <li>whose type is suitable for (unsafe) replacement of {@code this} in the parent.</li>
+     * <li>whose type is safe for replacement of {@code this} in the parent.</li>
      * </ol>
      *
      * @return an appropriately typed {@link WrapperNode} if {@link #isInstrumentable()}.
diff -r cc2b817de0b5 -r c7e57dffc5ad graal/com.oracle.truffle.api/src/com/oracle/truffle/api/tools/NodeExecCounter.java
--- a/graal/com.oracle.truffle.api/src/com/oracle/truffle/api/tools/NodeExecCounter.java	Thu Jan 29 11:27:19 2015 +0100
+++ b/graal/com.oracle.truffle.api/src/com/oracle/truffle/api/tools/NodeExecCounter.java	Thu Jan 29 11:44:14 2015 -0800
@@ -96,8 +96,12 @@
      */
     private final TruffleEventReceiver eventReceiver = new DefaultEventReceiver() {
         @Override
+        public void enter(Node node, VirtualFrame frame) {
+            internalReceive(node);
+        }
+
         @TruffleBoundary
-        public void enter(Node node, VirtualFrame frame) {
+        private void internalReceive(Node node) {
             if (isEnabled()) {
                 final Class<?> nodeClass = node.getClass();
                 AtomicLong nodeCounter = counters.get(nodeClass);