From fd4aed3b7822cde5b779f9e9e9ab23b210067048 Mon Sep 17 00:00:00 2001
From: Florian Angerer <florian.angerer@oracle.com>
Date: Wed, 31 May 2017 11:31:31 +0200
Subject: [PATCH] Added abstraction for attribute to serialize. Implemented
 more and refined handling of srcrefs.

---
 .../oracle/truffle/r/runtime/RSerialize.java  | 185 ++++++++----------
 .../com/oracle/truffle/r/runtime/RSource.java |  24 ++-
 .../com/oracle/truffle/r/runtime/RSrcref.java |  73 +++++--
 3 files changed, 168 insertions(+), 114 deletions(-)

diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/RSerialize.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/RSerialize.java
index 494ff5bb7b..63106513ce 100644
--- a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/RSerialize.java
+++ b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/RSerialize.java
@@ -53,7 +53,6 @@ import com.oracle.truffle.r.runtime.data.REmpty;
 import com.oracle.truffle.r.runtime.data.RExpression;
 import com.oracle.truffle.r.runtime.data.RExternalPtr;
 import com.oracle.truffle.r.runtime.data.RFunction;
-import com.oracle.truffle.r.runtime.data.RIntVector;
 import com.oracle.truffle.r.runtime.data.RLanguage;
 import com.oracle.truffle.r.runtime.data.RList;
 import com.oracle.truffle.r.runtime.data.RMissing;
@@ -84,7 +83,6 @@ import com.oracle.truffle.r.runtime.env.frame.FrameSlotChangeMonitor;
 import com.oracle.truffle.r.runtime.ffi.DLL;
 import com.oracle.truffle.r.runtime.gnur.SEXPTYPE;
 import com.oracle.truffle.r.runtime.nodes.RCodeBuilder;
-import com.oracle.truffle.r.runtime.nodes.RSourceSectionNode;
 import com.oracle.truffle.r.runtime.nodes.RSyntaxCall;
 import com.oracle.truffle.r.runtime.nodes.RSyntaxConstant;
 import com.oracle.truffle.r.runtime.nodes.RSyntaxElement;
@@ -1435,7 +1433,6 @@ public class RSerialize {
                 } else if (type == SEXPTYPE.SYMSXP) {
                     writeSymbol((RSymbol) obj);
                 } else {
-                    SourceSection sourceSection = getSourceSection(obj);
                     if (type == SEXPTYPE.ENVSXP) {
                         REnvironment env = (REnvironment) obj;
                         addReadRef(obj);
@@ -1466,27 +1463,20 @@ public class RSerialize {
                             }
                             terminatePairList();
                             writeItem(RNull.instance); // hashtab
-                            DynamicObject attributes = env.getAttributes();
-                            writeAttributes(attributes, sourceSection);
-                            if (attributes == null && sourceSection == null) {
+                            OutAttributes attributes = new OutAttributes(env, type);
+                            if (attributes.hasAttributes()) {
+                                writeAttributes(attributes);
+                            } else {
                                 writeItem(RNull.instance);
                             }
                         }
                     } else {
                         // flags
-                        DynamicObject attributes = null;
-                        SourceSection ss = sourceSection;
-                        if (obj instanceof RAttributable) {
-                            RAttributable rattr = (RAttributable) obj;
-                            attributes = rattr.getAttributes();
-                            if (attributes != null && attributes.isEmpty()) {
-                                attributes = null;
-                            }
-                        }
+                        OutAttributes attributes = new OutAttributes(obj, type);
                         boolean hasTag = gnuRType == SEXPTYPE.CLOSXP || gnuRType == SEXPTYPE.DOTSXP || (gnuRType == SEXPTYPE.PROMSXP && !((RPromise) obj).isEvaluated()) ||
                                         (type == SEXPTYPE.LISTSXP && !((RPairList) obj).isNullTag());
                         int gpbits = getGPBits(obj);
-                        int flags = Flags.packFlags(gnuRType, gpbits, isObject(obj), attributes != null, hasTag);
+                        int flags = Flags.packFlags(gnuRType, gpbits, isObject(obj), attributes.hasAttributes(), hasTag);
                         stream.writeInt(flags);
                         switch (type) {
                             case STRSXP: {
@@ -1613,21 +1603,13 @@ public class RSerialize {
                                     stream.writeString(name);
                                     break;
                                 }
-                                if (type == SEXPTYPE.FUNSXP) {
-                                    RFunction fun = (RFunction) obj;
-                                    RSyntaxFunction body = (RSyntaxFunction) fun.getRootNode();
-                                    ss = body.getLazySourceSection();
-                                }
                                 tailCall = true;
 
                                 // attributes written first to avoid recursion on cdr
-                                writeAttributes(attributes, ss);
+                                writeAttributes(attributes);
                                 if (attributes != null) {
                                     attributes = null;
                                 }
-                                if (ss != null) {
-                                    ss = null;
-                                }
 
                                 switch (type) {
                                     case FUNSXP: {
@@ -1698,19 +1680,12 @@ public class RSerialize {
                                 throw RInternalError.unimplemented(type.name());
                         }
 
-                        writeAttributes(attributes, ss);
+                        writeAttributes(attributes);
                     }
                 }
             } while (tailCall);
         }
 
-        private static SourceSection getSourceSection(Object obj) {
-            if (obj instanceof RSourceSectionNode) {
-                return ((RSourceSectionNode) obj).getLazySourceSection();
-            }
-            return null;
-        }
-
         private static Object getValueIgnoreActiveBinding(Frame frame, String key) {
             FrameDescriptor fd = frame.getFrameDescriptor();
             FrameSlot slot = fd.findFrameSlot(key);
@@ -1780,27 +1755,31 @@ public class RSerialize {
             }
         }
 
-        private void writeAttributes(DynamicObject attributes, SourceSection ss) throws IOException {
-            if (ss != null && ss != RSyntaxNode.LAZY_DEPARSE) {
-                String path = ss.getSource().getURI().getPath();
-                REnvironment createSrcfile = RSrcref.createSrcfile(path);
-                Object createLloc = RSrcref.createLloc(ss);
-                writePairListEntry(RRuntime.R_SRCREF, createLloc);
-                writePairListEntry(RRuntime.R_SRCFILE, createSrcfile);
-            }
-            if (attributes != null) {
-                // have to convert to GnuR pairlist
-                Iterator<RAttributesLayout.RAttribute> iter = RAttributesLayout.asIterable(attributes).iterator();
-                while (iter.hasNext()) {
-                    RAttributesLayout.RAttribute attr = iter.next();
-                    // name is the tag of the virtual pairlist
-                    // value is the car
-                    // next is the cdr
-                    writePairListEntry(attr.getName(), attr.getValue());
+        private void writeAttributes(OutAttributes outAttrs) throws IOException {
+            if (outAttrs != null) {
+                SourceSection ss = outAttrs.getSourceReferenceAttributes();
+                if (ss != null) {
+                    String path = ss.getSource().getURI().getPath();
+                    REnvironment createSrcfile = RSrcref.createSrcfile(path);
+                    Object createLloc = RSrcref.createLloc(ss, createSrcfile);
+                    writePairListEntry(RRuntime.R_SRCREF, createLloc);
+                    writePairListEntry(RRuntime.R_SRCFILE, createSrcfile);
+                }
+                DynamicObject attributes = outAttrs.getExplicitAttributes();
+                if (attributes != null) {
+                    // have to convert to GnuR pairlist
+                    Iterator<RAttributesLayout.RAttribute> iter = RAttributesLayout.asIterable(attributes).iterator();
+                    while (iter.hasNext()) {
+                        RAttributesLayout.RAttribute attr = iter.next();
+                        // name is the tag of the virtual pairlist
+                        // value is the car
+                        // next is the cdr
+                        writePairListEntry(attr.getName(), attr.getValue());
+                    }
+                }
+                if (outAttrs.hasAttributes()) {
+                    terminatePairList();
                 }
-            }
-            if (attributes != null || ss != null && ss != RSyntaxNode.LAZY_DEPARSE) {
-                terminatePairList();
             }
         }
 
@@ -2108,6 +2087,52 @@ public class RSerialize {
         }
     }
 
+    /**
+     * An abstraction of implicit and explicit attributes to serialize.
+     */
+    private static class OutAttributes {
+        private DynamicObject explicitAttributes;
+        private SourceSection ss;
+
+        OutAttributes(RAttributable obj, SEXPTYPE type) {
+
+            explicitAttributes = obj.getAttributes();
+            initSourceSection(obj, type);
+        }
+
+        OutAttributes(Object obj, SEXPTYPE type) {
+
+            if (obj instanceof RAttributable) {
+                explicitAttributes = ((RAttributable) obj).getAttributes();
+            }
+            initSourceSection(obj, type);
+        }
+
+        private void initSourceSection(Object obj, SEXPTYPE type) {
+            if (type == SEXPTYPE.FUNSXP) {
+                RFunction fun = (RFunction) obj;
+                RSyntaxFunction body = (RSyntaxFunction) fun.getRootNode();
+                SourceSection lazySourceSection = body.getLazySourceSection();
+                if (lazySourceSection != RSyntaxNode.LAZY_DEPARSE) {
+                    ss = lazySourceSection;
+                }
+            }
+        }
+
+        public boolean hasAttributes() {
+            return explicitAttributes != null && !explicitAttributes.isEmpty() || ss != null && ss != RSyntaxNode.LAZY_DEPARSE;
+        }
+
+        public DynamicObject getExplicitAttributes() {
+            return explicitAttributes;
+        }
+
+        public SourceSection getSourceReferenceAttributes() {
+            return ss;
+        }
+
+    }
+
     /**
      * For {@code lazyLoadDBinsertValue}.
      */
@@ -2418,21 +2443,6 @@ public class RSerialize {
         return state.closePairList();
     }
 
-    private static Object extractFromList(Object tag, SEXPTYPE expectedType) {
-        SEXPTYPE type = SEXPTYPE.typeForClass(tag.getClass());
-        if (type == expectedType) {
-            return tag;
-        } else if (type == SEXPTYPE.LISTSXP) {
-            for (RPairList item : (RPairList) tag) {
-                Object data = item.car();
-                if (SEXPTYPE.typeForClass(data.getClass()) == expectedType) {
-                    return data;
-                }
-            }
-        }
-        return null;
-    }
-
     /**
      * A collection of static functions that will transform a pairlist into an AST using the
      * {@link RCodeBuilder}.
@@ -2585,7 +2595,8 @@ public class RSerialize {
         } else {
             Object srcref = func.getAttr(RRuntime.R_SRCREF);
             if (srcref instanceof RAbstractIntVector) {
-                handleSrcrefAttr((RAbstractIntVector) srcref, null, elem);
+                SourceSection ss = RSrcref.createSourceSection((RAbstractIntVector) srcref, null);
+                elem.setSourceSection(ss);
             }
         }
     }
@@ -2597,12 +2608,13 @@ public class RSerialize {
     private static void handleSrcrefAttr(RAttributable func, RSyntaxCall elem) {
         Object srcref = func.getAttr(RRuntime.R_SRCREF);
         if (srcref instanceof RAbstractIntVector) {
-            handleSrcrefAttr((RAbstractIntVector) srcref, null, elem);
+            SourceSection ss = RSrcref.createSourceSection((RAbstractIntVector) srcref, null);
+            elem.setSourceSection(ss);
         } else if (srcref instanceof RList) {
             try {
                 Object srcfile = func.getAttr(RRuntime.R_SRCFILE);
                 assert srcfile instanceof REnvironment;
-                Source source = RSource.fromFile((REnvironment) srcfile);
+                Source source = RSource.fromSrcfile((REnvironment) srcfile);
 
                 RList l = (RList) srcref;
                 RSyntaxElement[] syntaxArguments = elem.getSyntaxArguments();
@@ -2611,10 +2623,11 @@ public class RSerialize {
                 for (int i = 0; i < l.getLength(); i++) {
                     Object dataAt = l.getDataAt(i);
                     assert dataAt instanceof RAbstractIntVector;
+                    SourceSection ss = RSrcref.createSourceSection((RAbstractIntVector) dataAt, source);
                     if (i == 0) {
-                        handleSrcrefAttr((RAbstractIntVector) dataAt, source, elem);
+                        elem.setSourceSection(ss);
                     } else {
-                        handleSrcrefAttr((RAbstractIntVector) dataAt, source, syntaxArguments[i - 1]);
+                        syntaxArguments[i - 1].setSourceSection(ss);
                     }
                 }
             } catch (NoSuchFileException e) {
@@ -2624,30 +2637,4 @@ public class RSerialize {
             }
         }
     }
-
-    private static void handleSrcrefAttr(RAbstractIntVector srcrefVec, Source sharedSource, RSyntaxElement elem) {
-
-        try {
-            Source source;
-            if (sharedSource != null) {
-                source = sharedSource;
-            } else {
-                Object srcfile = srcrefVec.getAttr(RRuntime.R_SRCFILE);
-                assert srcfile instanceof REnvironment;
-                source = RSource.fromFile((REnvironment) srcfile);
-            }
-            int startLine = srcrefVec.getDataAt(0);
-            int startColumn = srcrefVec.getDataAt(1);
-            int startIdx = source.getLineStartOffset(startLine) + startColumn;
-            int length = source.getLineStartOffset(srcrefVec.getDataAt(2)) + srcrefVec.getDataAt(3) - startIdx;
-            SourceSection createSection = source.createSection(startLine, startColumn, length);
-            elem.setSourceSection(createSection);
-        } catch (NoSuchFileException e) {
-            RError.warning(RError.SHOW_CALLER, RError.Message.GENERIC, "Missing source file: " + e.getMessage());
-        } catch (IOException e) {
-            RError.warning(RError.SHOW_CALLER, RError.Message.GENERIC, "Cannot access source file: " + e.getMessage());
-        } catch (IllegalArgumentException e) {
-            RError.warning(RError.SHOW_CALLER, RError.Message.GENERIC, "Invalid source reference: " + e.getMessage());
-        }
-    }
 }
diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/RSource.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/RSource.java
index c345c3e30c..e4bdde9ff4 100644
--- a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/RSource.java
+++ b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/RSource.java
@@ -27,6 +27,8 @@ import java.io.IOException;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URL;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 
 import com.oracle.truffle.api.source.Source;
 import com.oracle.truffle.api.source.SourceSection;
@@ -192,9 +194,13 @@ public class RSource {
     /**
      * Create an (external) source from an R srcfile ({@link RSrcref#createSrcfile(String)}).
      */
-    public static Source fromFile(REnvironment env) throws IOException {
-        String path = (String) RRuntime.r2Java(env.get(SrcrefFields.filename.name()));
-        return fromFileName(path, false);
+    public static Source fromSrcfile(REnvironment env) throws IOException {
+        Path filename = Paths.get((String) RRuntime.r2Java(env.get(SrcrefFields.filename.name())));
+        if (filename.isAbsolute()) {
+            return fromFileName(filename.toString(), false);
+        }
+        Path wd = Paths.get((String) RRuntime.r2Java(env.get(SrcrefFields.wd.name())));
+        return fromFileName(wd.resolve(filename).toString(), false);
     }
 
     /**
@@ -231,6 +237,18 @@ public class RSource {
         return uri.getPath();
     }
 
+    /**
+     * Like {@link #getPath(Source)} but ignoring if {@code source} is "internal".
+     */
+    public static String getPathInternal(Source source) {
+        if (source == null) {
+            return null;
+        }
+        URI uri = source.getURI();
+        assert uri != null;
+        return uri.getPath();
+    }
+
     /**
      * Always returns a non-null string even for internal sources.
      */
diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/RSrcref.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/RSrcref.java
index d2a27b6377..247bf965ad 100644
--- a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/RSrcref.java
+++ b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/RSrcref.java
@@ -14,7 +14,9 @@ package com.oracle.truffle.r.runtime;
 import java.io.IOException;
 import java.nio.file.FileSystems;
 import java.nio.file.Files;
+import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.nio.file.attribute.PosixFileAttributes;
 
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
@@ -25,9 +27,11 @@ import com.oracle.truffle.r.runtime.data.RDataFactory;
 import com.oracle.truffle.r.runtime.data.RIntVector;
 import com.oracle.truffle.r.runtime.data.RNull;
 import com.oracle.truffle.r.runtime.data.RStringVector;
+import com.oracle.truffle.r.runtime.data.model.RAbstractIntVector;
 import com.oracle.truffle.r.runtime.env.REnvironment;
 import com.oracle.truffle.r.runtime.env.REnvironment.PutException;
 import com.oracle.truffle.r.runtime.ffi.BaseRFFI;
+import com.oracle.truffle.r.runtime.nodes.RSourceSectionNode;
 import com.oracle.truffle.r.runtime.nodes.RSyntaxNode;
 
 /**
@@ -50,7 +54,9 @@ public class RSrcref {
         timestamp,
         filename,
         isFile,
-        wd;
+        wd,
+        fixedNewlines,
+        lines;
     }
 
     private static final RStringVector SRCREF_ATTR = RDataFactory.createStringVectorFromScalar(RRuntime.R_SRCREF);
@@ -66,17 +72,10 @@ public class RSrcref {
     @TruffleBoundary
     private static REnvironment createSrcfile(Path path) {
         // A srcref is an environment
-        double mtime;
-        try {
-            PosixFileAttributes pfa = Files.readAttributes(path, PosixFileAttributes.class);
-            mtime = pfa.lastModifiedTime().toMillis();
-        } catch (IOException ex) {
-            mtime = RRuntime.DOUBLE_NA;
-        }
         REnvironment env = RDataFactory.createNewEnv("");
         env.safePut(SrcrefFields.Enc.name(), "unknown");
         env.safePut(SrcrefFields.encoding.name(), "native.enc");
-        env.safePut(SrcrefFields.timestamp.name(), mtime);
+        env.safePut(SrcrefFields.timestamp.name(), getTimestamp(path));
         env.safePut(SrcrefFields.filename.name(), path.toString());
         env.safePut(SrcrefFields.isFile.name(), RRuntime.LOGICAL_TRUE);
         env.safePut(SrcrefFields.wd.name(), BaseRFFI.GetwdRootNode.create().getCallTarget().call());
@@ -84,6 +83,17 @@ public class RSrcref {
         return env;
     }
 
+    private static double getTimestamp(Path path) {
+        double mtime;
+        try {
+            PosixFileAttributes pfa = Files.readAttributes(path, PosixFileAttributes.class);
+            mtime = pfa.lastModifiedTime().toMillis();
+        } catch (IOException ex) {
+            mtime = RRuntime.DOUBLE_NA;
+        }
+        return mtime;
+    }
+
     /**
      * Creates an {@code lloc} structure from a {@link SourceSection} and associated {@code srcfile}
      * . assert: srcfile was created from the {@link Source} associated with {@code ss} or is
@@ -107,13 +117,18 @@ public class RSrcref {
             env = RDataFactory.createNewEnv("src");
             env.setClassAttr(RDataFactory.createStringVector(new String[]{"srcfilecopy", RRuntime.R_SRCFILE}, true));
             try {
-                env.put("filename", source.getPath() == null ? "" : source.getPath());
-                env.put("fixedNewlines", RRuntime.LOGICAL_TRUE);
+                Path path = Paths.get(RSource.getPathInternal(source));
+                env.put(SrcrefFields.filename.name(), path.toString());
+                env.put(SrcrefFields.fixedNewlines.name(), RRuntime.LOGICAL_TRUE);
                 String[] lines = new String[source.getLineCount()];
                 for (int i = 0; i < lines.length; i++) {
                     lines[i] = source.getCode(i + 1);
                 }
-                env.put("lines", RDataFactory.createStringVector(lines, true));
+                env.put(SrcrefFields.lines.name(), RDataFactory.createStringVector(lines, true));
+                env.safePut(SrcrefFields.Enc.name(), "unknown");
+                env.safePut(SrcrefFields.isFile.name(), RRuntime.asLogical(Files.isRegularFile(path)));
+                env.safePut(SrcrefFields.timestamp.name(), getTimestamp(path));
+                env.safePut(SrcrefFields.wd.name(), BaseRFFI.GetwdRootNode.create().getCallTarget().call());
             } catch (PutException e) {
                 throw RInternalError.shouldNotReachHere(e);
             }
@@ -149,4 +164,38 @@ public class RSrcref {
         lloc.setAttr(RRuntime.R_SRCFILE, srcfile);
         return lloc;
     }
+
+    public static SourceSection createSourceSection(RAbstractIntVector srcrefVec, Source sharedSource) {
+
+        try {
+            Source source;
+            if (sharedSource != null) {
+                source = sharedSource;
+            } else {
+                Object srcfile = srcrefVec.getAttr(RRuntime.R_SRCFILE);
+                assert srcfile instanceof REnvironment;
+                source = RSource.fromSrcfile((REnvironment) srcfile);
+            }
+            int startLine = srcrefVec.getDataAt(0);
+            int startColumn = srcrefVec.getDataAt(1);
+            int startIdx = getLineStartOffset(source, startLine) + startColumn;
+            int length = getLineStartOffset(source, srcrefVec.getDataAt(2)) + srcrefVec.getDataAt(3) - startIdx;
+            return source.createSection(startLine, startColumn, length);
+        } catch (NoSuchFileException e) {
+            RError.warning(RError.SHOW_CALLER, RError.Message.GENERIC, "Missing source file: " + e.getMessage());
+        } catch (IOException e) {
+            RError.warning(RError.SHOW_CALLER, RError.Message.GENERIC, "Cannot access source file: " + e.getMessage());
+        } catch (IllegalArgumentException e) {
+            RError.warning(RError.SHOW_CALLER, RError.Message.GENERIC, "Invalid source reference: " + e.getMessage());
+        }
+        return RSourceSectionNode.LAZY_DEPARSE;
+    }
+
+    private static int getLineStartOffset(Source source, int lineNum) {
+        try {
+            return source.getLineStartOffset(lineNum);
+        } catch (IllegalArgumentException e) {
+            throw new IllegalArgumentException(String.format("line %d does not exist in source %s", lineNum, RSource.getPathInternal(source)), e);
+        }
+    }
 }
-- 
GitLab