julia_gc: include julia.h globally for better CHANGED_BAG

.github/workflows/CI.yml

@@ -123,10 +123,11 @@ jobs:
             test-suites: "testbuildsys testmockpkg testinstall"
             extra: "NO_COVERAGE=1 ABI=32 BUILDDIR=out-of-tree CONFIGFLAGS=\"\""
 
-          # test Julia integration
+          # test Julia integration; this also runs testmockpkg, to verify
+          # packages can still be compiled
           - os: ubuntu-22.04
             shell: bash
-            test-suites: "testinstall"
+            test-suites: "testmockpkg testinstall"
             extra: "JULIA=yes CONFIGFLAGS=\"--enable-debug\""
 
           - os: windows-2022

Makefile.rules

@@ -231,6 +231,17 @@ GAP_CPPFLAGS += $(CPPFLAGS)
 SYSINFO_CPPFLAGS += $(CPPFLAGS)
 
 
+########################################################################
+# C and C++ compiler flags
+########################################################################
+
+# The Julia headers are external to GAP, so we pass their include directory via
+# `-isystem` to avoid warnings from julia.h being reported in GAP builds (and
+# in package builds, which may use `-Werror`). These flags are needed by
+# packages, too, as src/gasman.h includes julia.h.
+JULIA_ISYSTEM_CFLAGS = $(patsubst -I%,-isystem %,$(JULIA_CFLAGS))
+
+
 ########################################################################
 # C compiler flags
 ########################################################################
@@ -240,7 +251,8 @@ SYSINFO_CFLAGS = $(ABI_CFLAGS)
 GAP_CFLAGS += $(PTHREAD_CFLAGS)
 SYSINFO_CFLAGS += $(PTHREAD_CFLAGS)
 
-GAP_CFLAGS += $(JULIA_CFLAGS) # not added to SYSINFO_CFLAGS
+GAP_CFLAGS += $(JULIA_ISYSTEM_CFLAGS)
+SYSINFO_CFLAGS += $(JULIA_ISYSTEM_CFLAGS)
 
 # Finally add user provided CFLAGS
 GAP_CFLAGS += $(CFLAGS)
@@ -256,6 +268,9 @@ SYSINFO_CXXFLAGS = $(ABI_CFLAGS)
 GAP_CXXFLAGS += $(PTHREAD_CFLAGS)
 SYSINFO_CXXFLAGS += $(PTHREAD_CFLAGS)
 
+GAP_CXXFLAGS += $(JULIA_ISYSTEM_CFLAGS)
+SYSINFO_CXXFLAGS += $(JULIA_ISYSTEM_CFLAGS)
+
 # Finally add user provided CXXFLAGS
 GAP_CXXFLAGS += $(CXXFLAGS)
 SYSINFO_CXXFLAGS += $(CXXFLAGS)

configure.ac

@@ -815,8 +815,12 @@ AS_IF([test "x$with_julia" != xno ],[
   AS_IF([test "x$JULIA_CFLAGS" = x],[
     AS_IF([test -f "${JL_SHARE}/julia-config.jl"], [], [AC_MSG_ERROR([no julia-config.jl found])])
     JULIA_CFLAGS=$(${JULIA} --startup-file=no ${JL_SHARE}/julia-config.jl --cflags 2>/dev/null)
-    JULIA_CFLAGS=${JULIA_CFLAGS/-std=gnu99/}  # need to remove -std=gnu99 for our C11 and C++ code
     AS_IF([ test $? != 0 ], [AC_MSG_ERROR([failed to obtain JULIA_CFLAGS from julia-config.jl])])
+    JULIA_CFLAGS=${JULIA_CFLAGS/-std=gnu99/}  # need to remove -std=gnu99 for our C11 and C++ code
+    JULIA_CFLAGS=${JULIA_CFLAGS/-std=gnu11/}  # need to remove -std=gnu11 for our C++ code
+    # strip quotes: these flags are passed on to packages via sysinfo.gap,
+    # where they are subject to word splitting but not quote removal
+    JULIA_CFLAGS=${JULIA_CFLAGS//\'/}
   ])
   AC_MSG_RESULT([${JULIA_CFLAGS}])
 

src/gasman.h

@@ -366,7 +366,30 @@ EXPORT_INLINE void CHANGED_BAG(Bag bag)
 
 #elif defined(USE_JULIA_GC)
 
-void CHANGED_BAG(Bag bag);
+// Julia's headers include C++ headers when compiled as C++; since our own
+// headers may be included from within an `extern "C"` block, we must ensure
+// julia.h is not affected by that.
+#ifdef __cplusplus
+extern "C++" {
+#endif
+#include <julia.h>
+#ifdef __cplusplus
+}
+#endif
+
+EXPORT_INLINE void CHANGED_BAG(Bag bag)
+{
+    // The following is a copy of Julia's write barrier `jl_gc_wb_back` and
+    // must be kept in sync with it. We cannot just call `jl_gc_wb_back`, as
+    // Julia declares it `static inline`, and C forbids referencing an
+    // identifier with internal linkage from an inline function with external
+    // linkage. Marking `CHANGED_BAG` as `static inline` instead is not an
+    // option either, as it is used by other `EXPORT_INLINE` functions (such
+    // as `PushPlist`), which then would run into the very same problem.
+    void * p = BAG_HEADER(bag);
+    if (__unlikely(jl_astaggedvalue(p)->bits.gc == 3 /* GC_OLD_MARKED */))
+        jl_gc_queue_root((jl_value_t *)p);
+}
 
 BOOL IsGapObj(void *);
 

src/intfuncs.c

@@ -177,16 +177,10 @@ UInt4 nextrandMT_int32(UInt4* mt)
 // compile and run any of them on any platform, but your performance with the
 // non-native version will be less than optimal.
 
-//-----------------------------------------------------------------------------
-// MurmurHash3 was written by Austin Appleby, and is placed in the public
-// domain. The author hereby disclaims copyright to this source code.
-
 /* Minor modifications to get it to compile in C rather than C++ and
 integrate with GAP  SL*/
 
 
-#define FORCE_INLINE static inline
-
 #ifndef SYS_IS_64_BIT
 
 //-----------------------------------------------------------------------------
@@ -202,15 +196,15 @@ static inline uint32_t rotl32 ( uint32_t x, int8_t r )
 // Block read - if your platform needs to do endian-swapping or can only
 // handle aligned reads, do the conversion here
 
-FORCE_INLINE uint32_t getblock4 ( const uint32_t * p, int i )
+static inline uint32_t getblock4 ( const uint32_t * p, int i )
 {
   return p[i];
 }
 
 //-----------------------------------------------------------------------------
 // Finalization mix - force all bits of a hash block to avalanche
 
-FORCE_INLINE uint32_t fmix4 ( uint32_t h )
+static inline uint32_t fmix4 ( uint32_t h )
 {
   h ^= h >> 16;
   h *= 0x85ebca6b;
@@ -304,7 +298,7 @@ static inline uint64_t rotl64 ( uint64_t x, int8_t r )
 // hope that on archs which don't need this, the compiler will optimize it back
 // into a direct copy (verified to happen with GCC and clang on x86_64)
 
-FORCE_INLINE uint64_t getblock8 ( const uint64_t * p, int i )
+static inline uint64_t getblock8 ( const uint64_t * p, int i )
 {
   uint64_t val;
   memcpy(&val, p + i, sizeof(uint64_t));
@@ -314,7 +308,7 @@ FORCE_INLINE uint64_t getblock8 ( const uint64_t * p, int i )
 //-----------------------------------------------------------------------------
 // Finalization mix - force all bits of a hash block to avalanche
 
-FORCE_INLINE uint64_t fmix8 ( uint64_t k )
+static inline uint64_t fmix8 ( uint64_t k )
 {
   k ^= k >> 33;
   k *= BIG_CONSTANT(0xff51afd7ed558ccd);

src/julia_gc.c

@@ -664,7 +664,7 @@ static void JFinalizer(jl_value_t * obj)
     UInt        tnum = hdr->type;
 
     // if a bag needing a finalizer is retyped to a new tnum which no longer
-    // needs one, it may happen that JFinalize is called even though
+    // needs one, it may happen that JFinalizer is called even though
     // TabFreeFuncBags[tnum] is NULL
     if (TabFreeFuncBags[tnum])
         TabFreeFuncBags[tnum]((Bag)&contents);
@@ -828,11 +828,6 @@ BOOL IsGapObj(void * p)
     return jl_typeis(p, DatatypeGapObj);
 }
 
-void CHANGED_BAG(Bag bag)
-{
-    jl_gc_wb_back(BAG_HEADER(bag));
-}
-
 void SwapMasterPoint(Bag bag1, Bag bag2)
 {
     SWAP(UInt *, bag1->body, bag2->body);