From 918a176bb78b59deb2f6a0bf956ba744d253353a Mon Sep 17 00:00:00 2001
From: stepan <stepan.sindelar@oracle.com>
Date: Mon, 29 Jan 2018 19:30:04 +0100
Subject: [PATCH] Intern all RSymbol instances to enable equality checks in C

---
 .../r/ffi/impl/nodes/ListAccessNodes.java     |  3 ++-
 .../oracle/truffle/r/runtime/RRuntime.java    |  6 +++---
 .../truffle/r/runtime/data/RDataFactory.java  |  6 +++---
 .../truffle/r/runtime/data/RSymbol.java       | 20 +++++++++++++++++--
 .../testrffi/testrffi/tests/simpleTests.R     |  2 +-
 5 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/nodes/ListAccessNodes.java b/com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/nodes/ListAccessNodes.java
index b18eaf5c49..379adb5684 100644
--- a/com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/nodes/ListAccessNodes.java
+++ b/com.oracle.truffle.r.ffi.impl/src/com/oracle/truffle/r/ffi/impl/nodes/ListAccessNodes.java
@@ -32,6 +32,7 @@ import com.oracle.truffle.r.ffi.impl.nodes.ListAccessNodesFactory.SETCARNodeGen;
 import com.oracle.truffle.r.nodes.attributes.SpecialAttributesFunctions.GetNamesAttributeNode;
 import com.oracle.truffle.r.nodes.attributes.SpecialAttributesFunctions.SetNamesAttributeNode;
 import com.oracle.truffle.r.runtime.RInternalError;
+import com.oracle.truffle.r.runtime.data.CharSXPWrapper;
 import com.oracle.truffle.r.runtime.data.RArgsValuesAndNames;
 import com.oracle.truffle.r.runtime.data.RDataFactory;
 import com.oracle.truffle.r.runtime.data.RLanguage;
@@ -67,7 +68,7 @@ public final class ListAccessNodes {
 
         @Specialization
         protected Object car(RSymbol sym) {
-            return sym;
+            return CharSXPWrapper.create(sym.getName());
         }
 
         @Specialization
diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/RRuntime.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/RRuntime.java
index 589957931b..41a88e9b7e 100644
--- a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/RRuntime.java
+++ b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/RRuntime.java
@@ -5,7 +5,7 @@
  *
  * Copyright (c) 1995-2012, The R Core Team
  * Copyright (c) 2003, The R Foundation
- * Copyright (c) 2013, 2017, Oracle and/or its affiliates
+ * Copyright (c) 2013, 2018, Oracle and/or its affiliates
  *
  * All rights reserved.
  */
@@ -167,7 +167,7 @@ public class RRuntime {
 
     public static final String OP_NAMESPACE_SCOPE_EXPORTED = "::";
 
-    public static final RSymbol DEFERRED_DEFAULT_MARKER = new RSymbol("__Deferred_Default_Marker__");
+    public static final RSymbol DEFERRED_DEFAULT_MARKER = RSymbol.install("__Deferred_Default_Marker__");
 
     public static final String R_TARGET = "target";
     public static final String R_DOT_TARGET = ".target";
@@ -184,7 +184,7 @@ public class RRuntime {
     public static final String R_SRCFILE = "srcfile";
 
     public static final String NULL = "NULL";
-    public static final RSymbol PSEUDO_NULL = new RSymbol("\u0001NULL\u0001");
+    public static final RSymbol PSEUDO_NULL = RSymbol.install("\u0001NULL\u0001");
     public static final String UNBOUND = "UNBOUND";
 
     @CompilationFinal(dimensions = 1) private static final String[] rawStringCache = new String[256];
diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RDataFactory.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RDataFactory.java
index 4d2214df1c..131261bdbc 100644
--- a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RDataFactory.java
+++ b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RDataFactory.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2013, 2018, 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
@@ -618,7 +618,7 @@ public final class RDataFactory {
 
         public final RSymbol createSymbol(String name) {
             assert Utils.isInterned(name);
-            return traceDataCreated(new RSymbol(name));
+            return traceDataCreated(RSymbol.install(name));
         }
 
         /*
@@ -1197,7 +1197,7 @@ public final class RDataFactory {
 
     public static RSymbol createSymbol(String name) {
         assert Utils.isInterned(name);
-        return traceDataCreated(new RSymbol(name));
+        return traceDataCreated(RSymbol.install(name));
     }
 
     /*
diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RSymbol.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RSymbol.java
index ad8f31449e..903fd638fd 100644
--- a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RSymbol.java
+++ b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/data/RSymbol.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2018, 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
@@ -22,6 +22,12 @@
  */
 package com.oracle.truffle.r.runtime.data;
 
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Function;
+
+import com.oracle.truffle.api.CompilerAsserts;
+import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.CompilerDirectives.ValueType;
 import com.oracle.truffle.r.runtime.RType;
 
@@ -32,14 +38,24 @@ import com.oracle.truffle.r.runtime.RType;
 @ValueType
 public final class RSymbol extends RAttributeStorage {
 
+    /**
+     * Note: GnuR caches all symbols and some packages rely on their identity.
+     */
+    private static final ConcurrentHashMap<String, RSymbol> symbolTable = new ConcurrentHashMap<>();
+
     public static final RSymbol MISSING = RDataFactory.createSymbol("");
 
     private final String name;
 
-    public RSymbol(String name) {
+    private RSymbol(String name) {
         this.name = name;
     }
 
+    public static RSymbol install(String name) {
+        CompilerAsserts.neverPartOfCompilation();
+        return symbolTable.computeIfAbsent(name, RSymbol::new);
+    }
+
     @Override
     public RType getRType() {
         return RType.Symbol;
diff --git a/com.oracle.truffle.r.test.native/packages/testrffi/testrffi/tests/simpleTests.R b/com.oracle.truffle.r.test.native/packages/testrffi/testrffi/tests/simpleTests.R
index be9770bc6c..37a36ada65 100644
--- a/com.oracle.truffle.r.test.native/packages/testrffi/testrffi/tests/simpleTests.R
+++ b/com.oracle.truffle.r.test.native/packages/testrffi/testrffi/tests/simpleTests.R
@@ -118,4 +118,4 @@ rffi.RfEvalWithPromiseInPairList()
 # CAR/CDR tests
 rffi.CAR(NULL)
 rffi.CDR(NULL)
-rffi.CAR(as.symbol('a'))
+invisible(rffi.CAR(as.symbol('a'))) # TODO: printing CHARSEXP not implemented in FastR
-- 
GitLab