changeset 55897:5eeee2cc94f5

8200701: jdk/jshell/ExceptionsTest.java fails on Windows, after JDK-8198801 8159740: JShell: corralled declarations do not have correct source to wrapper mapping 8212167: JShell : Stack trace of exception has wrong line number Summary: Build corralled (recoverable undeclared definitions) declarations from position translating wraps.... Reviewed-by: jlahoda
author rfield
date Fri, 14 Jun 2019 20:30:10 -0700
parents 49ed5e31fe1e
children d8779634bc67
files src/jdk.jshell/share/classes/jdk/jshell/Corraller.java src/jdk.jshell/share/classes/jdk/jshell/Eval.java src/jdk.jshell/share/classes/jdk/jshell/GeneralWrap.java src/jdk.jshell/share/classes/jdk/jshell/SourceCodeAnalysisImpl.java src/jdk.jshell/share/classes/jdk/jshell/Wrap.java test/langtools/ProblemList.txt test/langtools/jdk/jshell/ClassesTest.java test/langtools/jdk/jshell/ExceptionsTest.java test/langtools/jdk/jshell/KullaTesting.java test/langtools/jdk/jshell/WrapperTest.java
diffstat 10 files changed, 445 insertions(+), 148 deletions(-) [+]
line wrap: on
line diff
--- a/src/jdk.jshell/share/classes/jdk/jshell/Corraller.java	Fri Jun 14 12:08:55 2019 -0400
+++ b/src/jdk.jshell/share/classes/jdk/jshell/Corraller.java	Fri Jun 14 20:30:10 2019 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -25,132 +25,226 @@
 
 package jdk.jshell;
 
-import java.io.IOException;
-import java.io.StringWriter;
 import com.sun.source.tree.ClassTree;
 import com.sun.source.tree.MethodTree;
 import com.sun.source.tree.Tree;
-import com.sun.tools.javac.code.Flags;
+import com.sun.source.tree.Tree.Kind;
 import com.sun.tools.javac.tree.JCTree;
 import com.sun.tools.javac.tree.JCTree.JCBlock;
 import com.sun.tools.javac.tree.JCTree.JCClassDecl;
-import com.sun.tools.javac.tree.JCTree.JCExpression;
 import com.sun.tools.javac.tree.JCTree.JCMethodDecl;
-import com.sun.tools.javac.tree.JCTree.JCNewClass;
-import com.sun.tools.javac.tree.JCTree.JCStatement;
 import com.sun.tools.javac.tree.JCTree.JCVariableDecl;
-import com.sun.tools.javac.tree.Pretty;
-import com.sun.tools.javac.tree.TreeMaker;
-import com.sun.tools.javac.util.Context;
+import com.sun.tools.javac.tree.JCTree.Visitor;
 import com.sun.tools.javac.util.List;
 import com.sun.tools.javac.util.ListBuffer;
-import com.sun.tools.javac.util.Names;
+import static com.sun.tools.javac.code.Flags.FINAL;
+import static com.sun.tools.javac.code.Flags.PUBLIC;
 import static com.sun.tools.javac.code.Flags.STATIC;
 import static com.sun.tools.javac.code.Flags.INTERFACE;
 import static com.sun.tools.javac.code.Flags.ENUM;
-import static com.sun.tools.javac.code.Flags.PUBLIC;
-import com.sun.tools.javac.util.Name;
-import jdk.jshell.spi.SPIResolutionException;
+import jdk.jshell.Wrap.CompoundWrap;
+import jdk.jshell.Wrap.Range;
+import jdk.jshell.Wrap.RangeWrap;
 
 /**
  * Produce a corralled version of the Wrap for a snippet.
- * Incoming tree is mutated.
- *
- * @author Robert Field
  */
-class Corraller extends Pretty {
+class Corraller extends Visitor {
 
-    private final StringWriter out;
-    private final int keyIndex;
-    private final TreeMaker make;
-    private final Names names;
-    private JCBlock resolutionExceptionBlock;
+    /** Visitor result field: a Wrap
+     */
+    protected Wrap result;
 
-    public Corraller(int keyIndex, Context context) {
-        this(new StringWriter(), keyIndex, context);
+    private final TreeDissector dis;
+    private final String resolutionExceptionBlock;
+    private final String source;
+
+    public Corraller(TreeDissector dis, int keyIndex, String source) {
+        this.dis = dis;
+        this.resolutionExceptionBlock = "\n      { throw new jdk.jshell.spi.SPIResolutionException(" + keyIndex + "); }";
+        this.source = source;
     }
 
-    private Corraller(StringWriter out, int keyIndex, Context context) {
-        super(out, false);
-        this.out = out;
-        this.keyIndex = keyIndex;
-        this.make = TreeMaker.instance(context);
-        this.names = Names.instance(context);
+    public Wrap corralType(ClassTree tree) {
+        return corralToWrap(tree);
     }
 
-    public Wrap corralType(ClassTree ct) {
-        ((JCClassDecl) ct).mods.flags |= Flags.STATIC | Flags.PUBLIC;
-        return corral(ct);
+    public Wrap corralMethod(MethodTree tree) {
+        return corralToWrap(tree);
     }
 
-    public Wrap corralMethod(MethodTree mt) {
-        ((JCMethodDecl) mt).mods.flags |= Flags.STATIC | Flags.PUBLIC;
-        return corral(mt);
+    private Wrap corralToWrap(Tree tree) {
+        try {
+            JCTree jct = (JCTree) tree;
+            Wrap w = new CompoundWrap(
+                    "    public static\n    ",
+                    corral(jct));
+            debugWrap("corralToWrap SUCCESS source: %s -- wrap:\n %s\n", tree, w.wrapped());
+            return w;
+        } catch (Exception ex) {
+            debugWrap("corralToWrap FAIL: %s - %s\n", tree, ex);
+            //ex.printStackTrace(System.err);
+            return null;
+        }
     }
 
-    private Wrap corral(Tree tree) {
-        try {
-            printStat((JCTree) tree);
-        } catch (IOException e) {
-            throw new AssertionError(e);
+    // Corral a single node.
+//    @SuppressWarnings("unchecked")
+    private <T extends JCTree> Wrap corral(T tree) {
+        if (tree == null) {
+            return null;
+        } else {
+            tree.accept(this);
+            Wrap tmpResult = this.result;
+            this.result = null;
+            return tmpResult;
         }
-        return Wrap.simpleWrap(out.toString());
+    }
+
+    private String defaultConstructor(JCClassDecl tree) {
+        return "  public " + tree.name.toString() + "() " +
+                resolutionExceptionBlock;
+    }
+
+    /* ***************************************************************************
+     * Visitor methods
+     ****************************************************************************/
+
+    @Override
+    public void visitClassDef(JCClassDecl tree) {
+        boolean isEnum = (tree.mods.flags & ENUM) != 0;
+        boolean isInterface = (tree.mods.flags & INTERFACE ) != 0;
+        int classBegin = dis.getStartPosition(tree);
+        int classEnd = dis.getEndPosition(tree);
+        //debugWrap("visitClassDef: %d-%d = %s\n", classBegin, classEnd, source.substring(classBegin, classEnd));
+        ListBuffer<Object> wrappedDefs = new ListBuffer<>();
+        int bodyBegin = -1;
+        if (tree.defs != null && !tree.defs.isEmpty()) {
+            if (isEnum) {
+                // copy the enum constants verbatim
+                int enumBegin = dis.getStartPosition(tree.defs.head);
+                JCTree t = null; // null to shut-up compiler, always set because non-empty
+                List<? extends JCTree> l = tree.defs;
+                for (; l.nonEmpty(); l = l.tail) {
+                    t = l.head;
+                    if (t.getKind() == Kind.VARIABLE) {
+                        if ((((JCVariableDecl)t).mods.flags & (PUBLIC | STATIC | FINAL)) != (PUBLIC | STATIC | FINAL)) {
+                            // non-enum constant, process normally
+                            break;
+                        }
+                    } else {
+                        // non-variable, process normally
+                        break;
+                    }
+                }
+                int constEnd = l.nonEmpty()                  // end of constants
+                        ? dis.getStartPosition(l.head) - 1   // is one before next defs, if there is one
+                        : dis.getEndPosition(t);             // and otherwise end of the last constant
+                wrappedDefs.append(new RangeWrap(source, new Range(enumBegin, constEnd)));
+                // handle any other defs
+                for (; l.nonEmpty(); l = l.tail) {
+                    wrappedDefs.append("\n");
+                    t = l.head;
+                    wrappedDefs.append(corral(t));
+                }
+            } else {
+                // non-enum
+                boolean constructorSeen = false;
+                for (List<? extends JCTree> l = tree.defs; l.nonEmpty(); l = l.tail) {
+                    wrappedDefs.append("\n   ");
+                    JCTree t = l.head;
+                    switch (t.getKind()) {
+                        case METHOD:
+                            constructorSeen = constructorSeen || ((MethodTree)t).getName() == tree.name.table.names.init;
+                            break;
+                        case BLOCK:
+                            // throw exception in instance initializer too -- inline because String not Wrap
+                            wrappedDefs.append((((JCBlock)t).flags & STATIC) != 0
+                                    ? new RangeWrap(source, dis.treeToRange(t))
+                                    : resolutionExceptionBlock);
+                            continue; // already appended, skip append below
+                    }
+                    wrappedDefs.append(corral(t));
+                }
+                if (!constructorSeen && !isInterface && !isEnum) {
+                    // Generate a default constructor, since
+                    // this is a regular class and there are no constructors
+                    if (wrappedDefs.length() > 0) {
+                        wrappedDefs.append("\n ");
+                    }
+                    wrappedDefs.append(defaultConstructor(tree));
+                }
+            }
+            bodyBegin = dis.getStartPosition(tree.defs.head);
+        }
+        Object defs = wrappedDefs.length() == 1
+            ? wrappedDefs.first()
+            : new CompoundWrap(wrappedDefs.toArray());
+        if (bodyBegin < 0) {
+            int brace = source.indexOf('{', classBegin);
+            if (brace < 0 || brace >= classEnd) {
+                throw new IllegalArgumentException("No brace found: " + source.substring(classBegin, classEnd));
+            }
+            bodyBegin = brace + 1;
+        }
+        // body includes openning brace
+        result = new CompoundWrap(
+                new RangeWrap(source, new Range(classBegin, bodyBegin)),
+                defs,
+                "\n}"
+        );
+    }
+
+    // Corral the body
+    @Override
+    public void visitMethodDef(JCMethodDecl tree) {
+        int methodBegin = dis.getStartPosition(tree);
+        int methodEnd = dis.getEndPosition(tree);
+        //debugWrap("+visitMethodDef: %d-%d = %s\n", methodBegin, methodEnd,
+        //        source.substring(methodBegin, methodEnd));
+        int bodyBegin = dis.getStartPosition(tree.getBody());
+        if (bodyBegin < 0) {
+            bodyBegin = source.indexOf('{', methodBegin);
+            if (bodyBegin > methodEnd) {
+                bodyBegin = -1;
+            }
+        }
+        if (bodyBegin > 0) {
+            //debugWrap("-visitMethodDef BEGIN: %d = '%s'\n", bodyBegin,
+            //        source.substring(methodBegin, bodyBegin));
+            Range noBodyRange = new Range(methodBegin, bodyBegin);
+            result = new CompoundWrap(
+                    new RangeWrap(source, noBodyRange),
+                    resolutionExceptionBlock);
+        } else {
+            Range range = new Range(methodBegin, methodEnd);
+            result = new RangeWrap(source, range);
+        }
+    }
+
+    // Remove initializer, if present
+    @Override
+    public void visitVarDef(JCVariableDecl tree) {
+        int begin = dis.getStartPosition(tree);
+        int end = dis.getEndPosition(tree);
+        if (tree.init == null) {
+            result = new RangeWrap(source, new Range(begin, end));
+        } else {
+            int sinit = dis.getStartPosition(tree.init);
+            int eq = source.lastIndexOf('=', sinit);
+            if (eq < begin) {
+                throw new IllegalArgumentException("Equals not found before init: " + source + " @" + sinit);
+            }
+            result = new CompoundWrap(new RangeWrap(source, new Range(begin, eq - 1)), ";");
+        }
     }
 
     @Override
-    public void visitBlock(JCBlock tree) {
-        // Top-level executable blocks (usually method bodies) are corralled
-        super.visitBlock((tree.flags & STATIC) != 0
-                ? tree
-                : resolutionExceptionBlock());
+    public void visitTree(JCTree tree) {
+        throw new IllegalArgumentException("Unexpected tree: " + tree);
     }
 
-    @Override
-    public void visitVarDef(JCVariableDecl tree) {
-        // No field inits in corralled classes
-        tree.init = null;
-        super.visitVarDef(tree);
-    }
-
-    @Override
-    public void visitClassDef(JCClassDecl tree) {
-        if ((tree.mods.flags & (INTERFACE | ENUM)) == 0 &&
-                !tree.getMembers().stream()
-                .anyMatch(t -> t.getKind() == Tree.Kind.METHOD &&
-                ((MethodTree) t).getName() == tree.name.table.names.init)) {
-            // Generate a default constructor, since
-            // this is a regular class and there are no constructors
-            ListBuffer<JCTree> ndefs = new ListBuffer<>();
-            ndefs.addAll(tree.defs);
-            ndefs.add(make.MethodDef(make.Modifiers(PUBLIC),
-                    tree.name.table.names.init,
-                    null, List.nil(), List.nil(), List.nil(),
-                    resolutionExceptionBlock(), null));
-            tree.defs = ndefs.toList();
-        }
-        super.visitClassDef(tree);
-    }
-
-    // Build a compiler tree for an exception throwing block, e.g.:
-    // {
-    //     throw new jdk.jshell.spi.SPIResolutionException(9);
-    // }
-    private JCBlock resolutionExceptionBlock() {
-        if (resolutionExceptionBlock == null) {
-            JCExpression expClass = null;
-            // Split the exception class name at dots
-            for (String id : SPIResolutionException.class.getName().split("\\.")) {
-                Name nm = names.fromString(id);
-                if (expClass == null) {
-                    expClass = make.Ident(nm);
-                } else {
-                    expClass = make.Select(expClass, nm);
-                }
-            }
-            JCNewClass exp = make.NewClass(null,
-                    null, expClass, List.of(make.Literal(keyIndex)), null);
-            resolutionExceptionBlock = make.Block(0L, List.of(make.Throw(exp)));
-        }
-        return resolutionExceptionBlock;
+    void debugWrap(String format, Object... args) {
+        //state.debug(this, InternalDebugControl.DBG_WRAP, format, args);
     }
 }
--- a/src/jdk.jshell/share/classes/jdk/jshell/Eval.java	Fri Jun 14 12:08:55 2019 -0400
+++ b/src/jdk.jshell/share/classes/jdk/jshell/Eval.java	Fri Jun 14 20:30:10 2019 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -44,7 +44,6 @@
 import com.sun.source.tree.NewClassTree;
 import com.sun.source.tree.Tree;
 import com.sun.source.tree.VariableTree;
-import com.sun.source.util.TreeScanner;
 import com.sun.tools.javac.tree.JCTree;
 import com.sun.tools.javac.tree.Pretty;
 import java.io.IOException;
@@ -622,7 +621,6 @@
                         name = "$" + ++varNumber;
                     }
                 }
-                TreeDissector dis = TreeDissector.createByFirstClass(pt);
                 ExpressionInfo varEI =
                         ExpressionToTypeInfo.localVariableTypeForInitializer(compileSource, state, true);
                 String declareTypeName;
@@ -634,6 +632,7 @@
                     fullTypeName = varEI.fullTypeName;
                     displayTypeName = varEI.displayTypeName;
 
+                    TreeDissector dis = TreeDissector.createByFirstClass(pt);
                     Pair<Wrap, Wrap> anonymous2Member =
                             anonymous2Member(varEI, compileSource, new Range(0, compileSource.length()), dis, expr.getExpression());
                     guts = Wrap.tempVarWrap(anonymous2Member.second.wrapped(), declareTypeName, name, anonymous2Member.first);
@@ -680,8 +679,8 @@
         String name = klassTree.getSimpleName().toString();
         DiagList modDiag = modifierDiagnostics(klassTree.getModifiers(), dis, false);
         TypeDeclKey key = state.keyMap.keyForClass(name);
-        // Corralling mutates.  Must be last use of pt, unitTree, klassTree
-        Wrap corralled = new Corraller(key.index(), pt.getContext()).corralType(klassTree);
+        // Corralling
+        Wrap corralled = new Corraller(dis, key.index(), compileSource).corralType(klassTree);
 
         Wrap guts = Wrap.classMemberWrap(compileSource);
         Snippet snip = new TypeDeclSnippet(key, userSource, guts,
@@ -752,8 +751,8 @@
         Tree returnType = mt.getReturnType();
         DiagList modDiag = modifierDiagnostics(mt.getModifiers(), dis, true);
         MethodKey key = state.keyMap.keyForMethod(name, parameterTypes);
-        // Corralling mutates.  Must be last use of pt, unitTree, mt
-        Wrap corralled = new Corraller(key.index(), pt.getContext()).corralMethod(mt);
+        // Corralling
+        Wrap corralled = new Corraller(dis, key.index(), compileSource).corralMethod(mt);
 
         if (modDiag.hasErrors()) {
             return compileFailResult(modDiag, userSource, Kind.METHOD);
--- a/src/jdk.jshell/share/classes/jdk/jshell/GeneralWrap.java	Fri Jun 14 12:08:55 2019 -0400
+++ b/src/jdk.jshell/share/classes/jdk/jshell/GeneralWrap.java	Fri Jun 14 20:30:10 2019 -0700
@@ -28,7 +28,10 @@
 /**
  * Common interface for all wrappings of snippet source to Java source.
  *
- * @author Robert Field
+ * Snippet index is index into the source of the snippet.  Note: If the snippet is a sub-range of
+ * the source, the index is not the index in the snippet.
+ *
+ * Wrap index is index into the wrapped snippet.
  */
 interface GeneralWrap {
 
--- a/src/jdk.jshell/share/classes/jdk/jshell/SourceCodeAnalysisImpl.java	Fri Jun 14 12:08:55 2019 -0400
+++ b/src/jdk.jshell/share/classes/jdk/jshell/SourceCodeAnalysisImpl.java	Fri Jun 14 20:30:10 2019 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -301,7 +301,7 @@
             SourcePositions sp = at.trees().getSourcePositions();
             CompilationUnitTree topLevel = at.firstCuTree();
             List<Suggestion> result = new ArrayList<>();
-            TreePath tp = pathFor(topLevel, sp, code.snippetIndexToWrapIndex(cursor));
+            TreePath tp = pathFor(topLevel, sp, code, cursor);
             if (tp != null) {
                 Scope scope = at.trees().getScope(tp);
                 Predicate<Element> accessibility = createAccessibilityFilter(at, tp);
@@ -563,7 +563,10 @@
         }
     }
 
-    private TreePath pathFor(CompilationUnitTree topLevel, SourcePositions sp, int pos) {
+    private TreePath pathFor(CompilationUnitTree topLevel, SourcePositions sp, GeneralWrap wrap, int snippetEndPos) {
+        int wrapEndPos = snippetEndPos == 0
+                ? wrap.snippetIndexToWrapIndex(snippetEndPos)
+                : wrap.snippetIndexToWrapIndex(snippetEndPos - 1) + 1;
         TreePath[] deepest = new TreePath[1];
 
         new TreePathScanner<Void, Void>() {
@@ -576,7 +579,7 @@
                 long end = sp.getEndPosition(topLevel, tree);
                 long prevEnd = deepest[0] != null ? sp.getEndPosition(topLevel, deepest[0].getLeaf()) : -1;
 
-                if (start <= pos && pos <= end &&
+                if (start <= wrapEndPos && wrapEndPos <= end &&
                     (start != end || prevEnd != end || deepest[0] == null ||
                      deepest[0].getParentPath().getLeaf() != getCurrentPath().getLeaf())) {
                     deepest[0] = new TreePath(getCurrentPath(), tree);
@@ -1176,7 +1179,7 @@
         return proc.taskFactory.analyze(codeWrap, List.of(keepParameterNames), at -> {
             SourcePositions sp = at.trees().getSourcePositions();
             CompilationUnitTree topLevel = at.firstCuTree();
-            TreePath tp = pathFor(topLevel, sp, codeWrap.snippetIndexToWrapIndex(cursor));
+            TreePath tp = pathFor(topLevel, sp, codeWrap, cursor);
 
             if (tp == null)
                 return Collections.emptyList();
@@ -1526,7 +1529,7 @@
         return proc.taskFactory.analyze(codeWrap, at -> {
             SourcePositions sp = at.trees().getSourcePositions();
             CompilationUnitTree topLevel = at.firstCuTree();
-            TreePath tp = pathFor(topLevel, sp, codeWrap.snippetIndexToWrapIndex(codeFin.length()));
+            TreePath tp = pathFor(topLevel, sp, codeWrap, codeFin.length());
             if (tp.getLeaf().getKind() != Kind.IDENTIFIER) {
                 return new QualifiedNames(Collections.emptyList(), -1, true, false);
             }
--- a/src/jdk.jshell/share/classes/jdk/jshell/Wrap.java	Fri Jun 14 12:08:55 2019 -0400
+++ b/src/jdk.jshell/share/classes/jdk/jshell/Wrap.java	Fri Jun 14 20:30:10 2019 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -34,8 +34,6 @@
 /**
  * Wrapping of source into Java methods, fields, etc.  All but outer layer
  * wrapping with imports and class.
- *
- * @author Robert Field
  */
 abstract class Wrap implements GeneralWrap {
 
@@ -184,7 +182,7 @@
 
     public static final class Range {
         final int begin;
-        final int end;
+        final int end; // exclusive
 
         Range(int begin, int end) {
             this.begin = begin;
@@ -216,7 +214,7 @@
 
         @Override
         public String toString() {
-            return "Range[" + begin + "," + end + "]";
+            return "Range[" + begin + "," + end + ")";
         }
     }
 
@@ -280,8 +278,12 @@
                     before += s.length();
                 } else if (o instanceof Wrap) {
                     Wrap w = (Wrap) o;
-                    if (sni >= w.firstSnippetIndex() && sni <= w.lastSnippetIndex()) {
-                        return w.snippetIndexToWrapIndex(sni) + before;
+                    if (sni >= w.firstSnippetIndex() && sni < w.lastSnippetIndex()) {
+                        int wwi = w.snippetIndexToWrapIndex(sni);
+                        debugWrap("\nCommoundWrap.snippetIndexToWrapIndex: SnippetIndex(%d) -> WrapIndex(%d + %d = %d)"
+                                        + "\n   === %s",
+                                sni, wwi, before, wwi + before, wrapped());
+                        return wwi + before;
                     }
                     before += w.wrapped().length();
                 }
@@ -300,8 +302,8 @@
                     w = (Wrap) o;
                     int len = w.wrapped().length();
                     if ((wi - before) <= len) {
-                        //System.err.printf("Defer to wrap %s - wi: %d. before; %d   -- %s  >>> %s\n",
-                        //        w, wi, before, w.debugPos(wi - before), w.wrapped());
+                        debugWrap("CommoundWrap.wrapIndexToWrap: Defer to wrap %s - wi: %d. before; %d   >>> %s\n",
+                                w, wi, before, w.wrapped());
                         return w;
                     }
                     before += len;
@@ -321,9 +323,10 @@
                     Wrap w = (Wrap) o;
                     int len = w.wrapped().length();
                     if ((wi - before) <= len) {
-                        //System.err.printf("Defer to wrap %s - wi: %d. before; %d   -- %s  >>> %s\n",
-                        //        w, wi, before, w.debugPos(wi - before), w.wrapped());
-                        return w.wrapIndexToSnippetIndex(wi - before);
+                        int si = w.wrapIndexToSnippetIndex(wi - before);
+                        debugWrap("\nCommoundWrap.wrapIndexToSnippetIndex: WrapIndex(%d) -> SnippetIndex(%d)\n",
+                                wi, si);
+                        return si;
                     }
                     before += len;
                 }
@@ -369,7 +372,7 @@
                 } else if (o instanceof Wrap) {
                     w = (Wrap) o;
                     int lns = countLines(w.wrapped());
-                    if ((wline - before) < lns) {
+                    if ((wline - before) <= lns) {
                         return w;
                     }
                     before += lns;
@@ -388,7 +391,7 @@
                 } else if (o instanceof Wrap) {
                     Wrap w = (Wrap) o;
                     int lns = countLines(w.wrapped());
-                    if ((wline - before) < lns) {
+                    if ((wline - before) <= lns) {
                         return w.wrapLineToSnippetLine(wline - before);
                     }
                     before += lns;
@@ -409,16 +412,21 @@
 
         @Override
         public String toString() {
-            return "CompoundWrap(" + Arrays.stream(os).map(Object::toString).collect(joining(",")) + ")";
+            return "CompoundWrap(" + Arrays.stream(os)
+                    .map(o -> (o instanceof String)
+                            ? "\"" + o + "\""
+                            : o.toString())
+                    .collect(joining(","))
+                    + ")";
         }
     }
 
-    private static class RangeWrap extends Wrap {
+    static class RangeWrap extends Wrap {
 
         final Range range;
-        final String wrapped;
-        final int firstSnline;
-        final int lastSnline;
+        final String wrapped;   // The snippet portion of the source
+        final int firstSnline;  // Line count to start of snippet portion
+        final int lastSnline;   // Line count to end of snippet portion
 
         RangeWrap(String snippetSource, Range usedWithinSnippet) {
             this.range = usedWithinSnippet;
@@ -436,24 +444,39 @@
         @Override
         public int snippetIndexToWrapIndex(int sni) {
             if (sni < range.begin) {
+                debugWrap("\nRangeWrap.snippetIndexToWrapIndex: ERR before SnippetIndex(%d) -> WrapIndex(%d + %d = %d)\n",
+                        sni, 0);
                 return 0;
             }
             if (sni > range.end) {
+                debugWrap("\nRangeWrap.snippetIndexToWrapIndex: ERR after SnippetIndex(%d) -> WrapIndex(%d + %d = %d)\n",
+                        sni, range.length());
                 return range.length();
             }
-            return sni - range.begin;
+            int wi = sni - range.begin;
+            debugWrap("\nRangeWrap.snippetIndexToWrapIndex: SnippetIndex(%d) -> WrapIndex(%d + %d = %d)"
+                            + "\n   === %s",
+                    sni, sni, range.begin, sni - range.begin, wrapped());
+            return wi;
         }
 
         @Override
         public int wrapIndexToSnippetIndex(int wi) {
             if (wi < 0) {
+                debugWrap("\nRangeWrap.wrapIndexToSnippetIndex: ERR before WrapIndex(%d) -> SnippetIndex(%d)\n",
+                        wi, 0);
                 return 0; // bad index
             }
             int max = range.length();
             if (wi > max) {
-                wi = max;
+                debugWrap("\nRangeWrap.wrapIndexToSnippetIndex: ERR after WrapIndex(%d) -> SnippetIndex(%d)\n",
+                        wi, max + range.begin);
+                return max + range.begin;
             }
-            return wi + range.begin;
+            int sni = wi + range.begin;
+            debugWrap("\nRangeWrap.wrapIndexToSnippetIndex: WrapIndex(%d) -> SnippetIndex(%d)\n",
+                    wi, sni);
+            return sni;
         }
 
         @Override
@@ -535,4 +558,9 @@
             super("    public static ", wtype, brackets + " ", wname, semi(wname));
         }
     }
+
+    void debugWrap(String format, Object... args) {
+        //System.err.printf(format, args);
+        //state.debug(this, InternalDebugControl.DBG_WRAP, format, args);
+    }
 }
--- a/test/langtools/ProblemList.txt	Fri Jun 14 12:08:55 2019 -0400
+++ b/test/langtools/ProblemList.txt	Fri Jun 14 20:30:10 2019 -0700
@@ -37,7 +37,6 @@
 
 jdk/jshell/UserJdiUserRemoteTest.java                                           8173079    linux-all
 jdk/jshell/UserInputTest.java                                                   8169536    generic-all
-jdk/jshell/ExceptionsTest.java                                                  8200701    windows-all
 
 ###########################################################################
 #
--- a/test/langtools/jdk/jshell/ClassesTest.java	Fri Jun 14 12:08:55 2019 -0400
+++ b/test/langtools/jdk/jshell/ClassesTest.java	Fri Jun 14 20:30:10 2019 -0700
@@ -344,7 +344,7 @@
                    "  public T get() {return null;}\n" +
                    "}",
                    added(VALID),
-                   ste(aClass, Status.RECOVERABLE_DEFINED, Status.VALID, true, null));
+                   ste(aClass, Status.RECOVERABLE_DEFINED, Status.VALID, false, null));
         assertEval("new A()");
     }
 
--- a/test/langtools/jdk/jshell/ExceptionsTest.java	Fri Jun 14 12:08:55 2019 -0400
+++ b/test/langtools/jdk/jshell/ExceptionsTest.java	Fri Jun 14 20:30:10 2019 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -24,7 +24,7 @@
 /*
  * @test
  * @summary Tests for exceptions
- * @bug 8198801
+ * @bug 8198801 8212167
  * @build KullaTesting TestingInputStream
  * @run testng ExceptionsTest
  */
@@ -207,6 +207,49 @@
                         newStackTraceElement("", "", cr2.snippet(), 1)));
     }
 
+    // test 8212167
+    public void throwLineFormat1() {
+        SnippetEvent se = assertEvalException(
+                "if (true) { \n" +
+                        "   int x = 10; \n" +
+                        "   int y = 10 / 0;}"
+        );
+        assertExceptionMatch(se,
+                new ExceptionInfo(ArithmeticException.class, "/ by zero",
+                        newStackTraceElement("", "", se.snippet(), 3)));
+    }
+
+    public void throwLineFormat3() {
+        Snippet sp = methodKey(assertEval(
+                "int p() \n" +
+                        "  { return 4/0; }"));
+        Snippet sm = methodKey(assertEval(
+                "int m(int x)\n" +
+                        "       \n" +
+                        "       {\n" +
+                        "          return p() + x; \n" +
+                        "       }"));
+        Snippet sn = methodKey(assertEval(
+                "int n(int x) {\n" +
+                        "         try {\n" +
+                        "           return m(x);\n" +
+                        "         }\n" +
+                        "         catch (Throwable ex) {\n" +
+                        "           throw new IllegalArgumentException( \"GOT:\", ex);\n" +
+                        "         }\n" +
+                        "       }"));
+        SnippetEvent se = assertEvalException("n(33);");
+        assertExceptionMatch(se,
+                new ExceptionInfo(IllegalArgumentException.class, null,
+                        new ExceptionInfo(ArithmeticException.class, "/ by zero",
+                                newStackTraceElement("", "p", sp, 2),
+                                newStackTraceElement("", "m", sm, 4),
+                                newStackTraceElement("", "n", sn, 3),
+                                newStackTraceElement("", "", se.snippet(), 1)),
+                        newStackTraceElement("", "n", sn, 6),
+                        newStackTraceElement("", "", se.snippet(), 1)));
+    }
+
     @Test(enabled = false) // TODO 8129427
     public void outOfMemory() {
         assertEval("import java.util.*;");
@@ -333,7 +376,8 @@
                     }
                     assertEquals(actualElement.getFileName(), expectedElement.getFileName(), message + " : file names");
                     assertEquals(actualElement.getLineNumber(), expectedElement.getLineNumber(), message + " : line numbers"
-                        + " -- actual: " + actual + ", expected: " + expected);
+                        + " -- actual: " + actualElement.getLineNumber() + ", expected: " + expectedElement.getLineNumber() +
+                            " -- in: " + actualElement.getClassName());
                 }
             }
         }
--- a/test/langtools/jdk/jshell/KullaTesting.java	Fri Jun 14 12:08:55 2019 -0400
+++ b/test/langtools/jdk/jshell/KullaTesting.java	Fri Jun 14 20:30:10 2019 -0700
@@ -911,7 +911,10 @@
 
     public void assertCompletionIncludesExcludes(String code, Boolean isSmart, Set<String> expected, Set<String> notExpected) {
         List<String> completions = computeCompletions(code, isSmart);
-        assertTrue(completions.containsAll(expected), String.valueOf(completions));
+        assertTrue(completions.containsAll(expected), "Expected completions: "
+                + String.valueOf(expected)
+                + ", got: "
+                + String.valueOf(completions));
         assertTrue(Collections.disjoint(completions, notExpected), String.valueOf(completions));
     }
 
--- a/test/langtools/jdk/jshell/WrapperTest.java	Fri Jun 14 12:08:55 2019 -0400
+++ b/test/langtools/jdk/jshell/WrapperTest.java	Fri Jun 14 20:30:10 2019 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -23,7 +23,7 @@
 
 /*
  * @test
- * @bug 8159111
+ * @bug 8159111 8159740
  * @summary test wrappers and dependencies
  * @modules jdk.jshell/jdk.jshell
  * @build KullaTesting
@@ -62,15 +62,126 @@
         assertPosition(swg, src, 15, 6);
     }
 
-    @Test(enabled = false) // TODO 8159740
+    // test 8159740
     public void testMethodCorralled() {
         String src = "void glib() { f(); }";
+        //            _123456789_123456789
         Snippet g = methodKey(assertEval(src, added(RECOVERABLE_DEFINED)));
         SnippetWrapper swg = getState().sourceCodeAnalysis().wrapper(g);
-        assertWrapperHas(swg, src, Kind.METHOD, "void", "glib");
+        assertWrapperHas(swg, src, Kind.METHOD, "SPIResolutionException",
+                "void", "glib");
+        assertPosition(swg, src, 0, 4);
         assertPosition(swg, src, 5, 4);
     }
 
+    // test 8159740
+    public void testClassCorralled0() {
+        String src = "class AAA { float mmm(double d1234) { return (float) (f0 * d1234); } }";
+        //            _123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789
+        Snippet a = classKey(assertEval(src, added(RECOVERABLE_DEFINED)));
+        SnippetWrapper swa = getState().sourceCodeAnalysis().wrapper(a);
+        assertWrapperHas(swa, src, Kind.TYPE_DECL, "SPIResolutionException",
+                "class", "AAA", "float", "mmm", "double", "d1234");
+        assertPosition(swa, src, 0, 5);
+        assertPosition(swa, src, 6, 3);
+        assertPosition(swa, src, 12, 5);
+        assertPosition(swa, src, 18, 3);
+        assertPosition(swa, src, 22, 6);
+        assertPosition(swa, src, 29, 5);
+    }
+
+    // test 8159740
+    public void testClassCorralled() {
+        String src = "class AAA { int xxx = x0 + 4; float mmm(float ffff) { return f0 * ffff; } }";
+        //            _123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789
+        Snippet a = classKey(assertEval(src, added(RECOVERABLE_DEFINED)));
+        SnippetWrapper swa = getState().sourceCodeAnalysis().wrapper(a);
+        assertWrapperHas(swa, src, Kind.TYPE_DECL, "SPIResolutionException",
+                "class", "AAA", "int", "xxx", "float", "mmm", "ffff");
+        assertPosition(swa, src, 0, 5);
+        assertPosition(swa, src, 6, 3);
+        assertPosition(swa, src, 12, 3);
+        assertPosition(swa, src, 16, 3);
+        assertPosition(swa, src, 30, 5);
+        assertPosition(swa, src, 36, 3);
+        assertPosition(swa, src, 40, 5);
+        assertPosition(swa, src, 46, 4);
+    }
+
+    // test 8159740
+    public void testClassWithConstructorCorralled() {
+        String src = "public class AAA { AAA(String b) {} int xxx = x0 + 4; float mmm(float ffff) { return f0 * ffff; } }";
+        //            _123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789
+        Snippet a = classKey(assertEval(src, added(RECOVERABLE_DEFINED)));
+        SnippetWrapper swa = getState().sourceCodeAnalysis().wrapper(a);
+        assertWrapperHas(swa, src, Kind.TYPE_DECL, "SPIResolutionException",
+                "class", "AAA", "String", "int", "xxx", "float", "mmm", "ffff");
+        assertPosition(swa, src, 7, 5);
+        assertPosition(swa, src, 13, 3);
+        assertPosition(swa, src, 19, 3);
+        assertPosition(swa, src, 23, 5);
+        assertPosition(swa, src, 30, 1);
+        assertPosition(swa, src, 36, 3);
+        assertPosition(swa, src, 40, 3);
+        assertPosition(swa, src, 54, 5);
+        assertPosition(swa, src, 60, 3);
+        assertPosition(swa, src, 64, 5);
+        assertPosition(swa, src, 70, 4);
+    }
+
+    // test 8159740
+    public void testInterfaceCorralled() {
+        String src = "interface AAA { default float mmm(double d1234) { return (float) (f0 * d1234); } }";
+        //            _123456789_123456789_123456789_123456789_123456789_123456789_123456789_123456789
+        Snippet a = classKey(assertEval(src, added(RECOVERABLE_DEFINED)));
+        SnippetWrapper swa = getState().sourceCodeAnalysis().wrapper(a);
+        assertWrapperHas(swa, src, Kind.TYPE_DECL, "SPIResolutionException",
+                "interface", "AAA", "float", "mmm", "double", "d1234");
+        assertPosition(swa, src, 0, 9);
+        assertPosition(swa, src, 10, 3);
+        assertPosition(swa, src, 16, 7);
+        assertPosition(swa, src, 24, 5);
+        assertPosition(swa, src, 30, 3);
+        assertPosition(swa, src, 34, 6);
+        assertPosition(swa, src, 41, 5);
+    }
+
+    // test 8159740
+    public void testEnumCorralled() {
+        String src =
+                "public enum Planet {\n" +
+                "    MERCURY (3.303e+23, 2.4397e6),\n" +
+                "    VENUS   (4.869e+24, 6.0518e6),\n" +
+                "    EARTH   (5.976e+24, 6.37814e6),\n" +
+                "    MARS    (6.421e+23, 3.3972e6),\n" +
+                "    JUPITER (1.9e+27,   7.1492e7),\n" +
+                "    SATURN  (5.688e+26, 6.0268e7),\n" +
+                "    URANUS  (8.686e+25, 2.5559e7),\n" +
+                "    NEPTUNE (1.024e+26, 2.4746e7);\n" +
+                "\n" +
+                "    private final double mass;   // in kilograms\n" +
+                "    private final double radius; // in meters\n" +
+                "    Planet(double mass, double radius) {\n" +
+                "        this.mass = mass;\n" +
+                "        this.radius = radius;\n" +
+                "    }\n" +
+                "    private double mass() { return mass; }\n" +
+                "    private double radius() { return radius; }\n" +
+                "\n" +
+                "    double surfaceGravity() {\n" +
+                "        return GRAVITATIONAL_CONSTANT * mass / (radius * radius);\n" +
+                "    }\n" +
+                "    double surfaceWeight(double otherMass) {\n" +
+                "        return otherMass * surfaceGravity();\n" +
+                "    }\n" +
+                "}\n";
+        Snippet a = classKey(assertEval(src, added(RECOVERABLE_DEFINED)));
+        SnippetWrapper swa = getState().sourceCodeAnalysis().wrapper(a);
+        assertWrapperHas(swa, src, Kind.TYPE_DECL, "SPIResolutionException",
+                "enum", "Planet", "double", "mass", "EARTH", "NEPTUNE", "MERCURY",
+                "radius", "surfaceGravity", "surfaceWeight");
+    }
+
     public void testMethodBad() {
         String src = "void flob() { ?????; }";
         List<SnippetWrapper> swl = getState().sourceCodeAnalysis().wrappers(src);
@@ -182,23 +293,36 @@
     private void assertWrapperHas(SnippetWrapper sw, String source, Kind kind, String... has) {
         assertEquals(sw.source(), source);
         assertEquals(sw.kind(), kind);
+        String s = sw.wrapped();
         if (kind == Kind.IMPORT) {
-            assertTrue(sw.wrapped().contains("import"));
+            assertHas(s, "import");
         } else {
             String cn = sw.fullClassName();
             int idx = cn.lastIndexOf(".");
-            assertTrue(sw.wrapped().contains(cn.substring(idx+1)));
-            assertTrue(sw.wrapped().contains("class"));
+            assertHas(s, cn.substring(idx+1));
+            assertHas(s, "class");
         }
-        for (String s : has) {
-            assertTrue(sw.wrapped().contains(s));
+        for (String hx : has) {
+            assertHas(s, hx);
         }
     }
 
+    private void assertHas(String s, String has) {
+        assertTrue(s.contains(has), "Expected to find '" + has + "' in: '" + s + "'");
+    }
+
     private void assertPosition(SnippetWrapper sw, String source, int start, int length) {
+        //System.err.printf("\n#assertPosition:\n#  debug-source: %s\n#  SnippetWrapper --\n#    source: %s\n#    wrapped: %s\n",
+        //        source, sw.source(), sw.wrapped());
+        //System.err.printf("#  start: %d    length: %d\n", start, length);
         int wpg = sw.sourceToWrappedPosition(start);
-        assertEquals(sw.wrapped().substring(wpg, wpg+length),
-                source.substring(start, start+length),
+        //System.err.printf("#  wrappedPos: %d\n", wpg);
+        String wrappedPart = sw.wrapped().substring(wpg, wpg+length);
+        String sourcePart = source.substring(start, start+length);
+        //System.err.printf("#  wrapped @ wrappedPos: %s\n", wrappedPart);
+        //System.err.printf("#  source @ start: %s\n", sourcePart);
+
+        assertEquals(wrappedPart, sourcePart,
                 "position " + wpg + " in " + sw.wrapped());
         assertEquals(sw.wrappedToSourcePosition(wpg), start);
     }