Merge pull request #482 from savi-lang/add/ffi-globals

Add `:ffi global` variable bindings.

Author: jemc

core/declarators/declarators.savi

@@ -389,7 +389,7 @@
   :intrinsic
   :term filenames NameList
 
-:: Declare a binding to an unsafe foreign functions (FFI), such as a C function.
+:: Declare a binding to an unsafe foreign function (FFI), such as a C function.
 ::
 :: It is common to define such bindings on a dedicated private module, which is
 :: usually, by convention, named `_FFI` to make it clear at call sites.
@@ -430,12 +430,77 @@
   :term ret Type
     :optional
 
+:: Declare a binding to an unsafe foreign function (FFI) global variable,
+:: such as a C global variable.
+::
+:: It is common to define such bindings on a dedicated private module, which is
+:: usually, by convention, named `_FFI` to make it clear at call sites.
+::
+:: Because FFI bindings are inherently unsafe, it becomes the job of FFI-using
+:: library authors to guarantee safety of the packages they publish.
+:: FFI functions should never be exposed directly as a public feature - they
+:: should be carefully wrapped in a library-specific way that can guarantee
+:: memory safety, concurrency safety, and capability security safety,
+:: up to the same high standards as the Savi standard library.
+::
+:: This can be easier said than done. When in doubt, avoid using FFI bindings
+:: and prefer implementing features in pure Savi code where possible, or
+:: ask for an FFI library safety review from experienced community members.
+::
+:: An `:ffi` global declaration uses `var` or `let` and is similar to a
+:: `:var` or `:let` declaration in that it binds a getter and optional setter
+:: (only for `var`) to a global variable in the foreign library.
+:: Use `var` if the global variable is expected to change (either on the Savi
+:: side or on the C side), or `let` if it's expected to be set just once.
+::
+:: WARNING: Savi programs involve actors and are implicitly multi-threaded,
+:: so global variables whose values are expected to change during the life
+:: of the program are very likely to cause memory safety issues.
+:: You should strongly consider using thread-local FFI variables instead
+:: (not yet implemented).
+:declarator ffi
+  :intrinsic
+  :context type
+  :begins ffi
+
+  :keyword global
+  :term var_or_let enum (var, let)
+  :term name Name
+  :term type Type
+
+// TODO: Implement and document this.
+:declarator ffi
+  :intrinsic
+  :context type
+  :begins ffi
+
+  :keyword thread_local
+  :term var_or_let enum (var, let)
+  :term name Name
+  :term type Type
+
+:: Override the foreign name used for linking this FFI declaration.
+::
+:: Normally, an FFI declaration uses the same name for C linking that it uses
+:: in Savi. But it's possible to use a different link name by declaring it here.
+::
+:: This can be useful if you want to use a less verbose name in Savi,
+:: or if you want to bind multiple Savi `:ffi` declarations to the same C name.
 :declarator foreign_name
   :intrinsic
   :context ffi
 
   :term name Name
 
+:: Specify a C shared library that fulfills this particular FFI declaration.
+::
+:: If this declaration is used in the program, that shared library will be
+:: linked to the final program binary. If it's not used, the library won't be.
+::
+:: This is useful for situations where the library may only be appropriate
+:: to link on some platforms but not others. In such a situation, you can
+:: specify the link library here and make sure every invocation of the
+:: platform-specific function is guarded by a platform check conditional block.
 :declarator link_lib
   :intrinsic
   :context ffi

spec/integration/run-ffi-link-c-files/savi.run.output.txt

@@ -1,2 +1,4 @@
 2 + 2 == 4!
 4 - 2 == 2!
+the magic number is 3
+the other magic number is 4.2

spec/integration/run-ffi-link-c-files/src/Main.savi

@@ -2,13 +2,19 @@
 :ffi_link_c_files (
   "../vendor/mylib_add.c"
   "../vendor/mylib_sub.c"
+  "../vendor/mylib_magic_number.c"
 )
 
 :module _FFI
   :ffi mylib_add(a I32, b I32) I32
   :ffi mylib_sub(a I32, b I32) I32
+  :ffi global let mylib_magic_number I32
+  :ffi global let mylib_other_magic_number F64
+    :foreign_name mylib_magic_number_2
 
 :actor Main
   :new (env)
     env.out.print("2 + 2 == \(_FFI.mylib_add(2, 2))!")
     env.out.print("4 - 2 == \(_FFI.mylib_sub(4, 2))!")
+    env.out.print("the magic number is \(_FFI.mylib_magic_number)")
+    env.out.print("the other magic number is \(_FFI.mylib_other_magic_number)")

spec/integration/run-ffi-link-c-files/vendor/mylib_magic_number.c

@@ -0,0 +1,2 @@
+int mylib_magic_number = 3;
+double mylib_magic_number_2 = 4.2;

src/savi/compiler/code_gen.cr

@@ -817,7 +817,16 @@ class Savi::Compiler::CodeGen
 
   def gen_func_impl(gtype, gfunc, llvm_func)
     return gen_intrinsic(gtype, gfunc, llvm_func) if gfunc.func.has_tag?(:compiler_intrinsic)
-    return gen_ffi_impl(gtype, gfunc, llvm_func) if gfunc.func.has_tag?(:ffi)
+
+    if gfunc.func.has_tag?(:ffi)
+      if gfunc.func.has_tag?(:ffi_global_getter)
+        return gen_ffi_global_getter_impl(gtype, gfunc, llvm_func)
+      elsif gfunc.func.has_tag?(:ffi_global_setter)
+        return gen_ffi_global_setter_impl(gtype, gfunc, llvm_func)
+      else
+        return gen_ffi_impl(gtype, gfunc, llvm_func)
+      end
+    end
 
     # Fields with no initializer body can be skipped.
     return if gfunc.func.has_tag?(:field) && gfunc.func.body.nil?
@@ -902,8 +911,6 @@ class Savi::Compiler::CodeGen
   end
 
   def gen_ffi_impl(gtype, gfunc, llvm_func)
-    llvm_ffi_func = gen_ffi_decl(gfunc)
-
     gen_func_start(llvm_func, gtype, gfunc)
 
     param_count = llvm_func.params.size
@@ -957,12 +964,42 @@ class Savi::Compiler::CodeGen
     else
       raise NotImplementedError.new(gfunc.calling_convention)
     end
+  end
 
-    # And possibly cast the return type, for the same reasons
-    # that we possibly cast the argument types earlier above.
-    ret_type = llvm_type_of(gfunc.func.ret.not_nil!, gfunc)
+  def gen_ffi_global_decl(gfunc)
+    llvm_type = llvm_type_of(gfunc.func.ret.not_nil!, gfunc)
 
-    value
+    global = @mod.globals.add(llvm_type, gfunc.func.metadata[:ffi_link_name].as(String))
+    global.linkage = LLVM::Linkage::External
+    global.global_constant = gfunc.func.has_tag?(:ffi_global_constant)
+    global.externally_initialized = true # TODO: false and set an initializer if this ffi var has an initializer
+    global
+  end
+
+  def gen_ffi_global_getter_impl(gtype, gfunc, llvm_func)
+    gen_func_start(llvm_func, gtype, gfunc)
+
+    global = gen_ffi_global_decl(gfunc)
+    llvm_type = llvm_type_of(gfunc.func.ret.not_nil!, gfunc)
+
+    value = @builder.load(llvm_type, global, "#{global.name}.LOAD")
+
+    @builder.ret(value)
+
+    gen_func_end(gfunc)
+  end
+
+  def gen_ffi_global_setter_impl(gtype, gfunc, llvm_func)
+    gen_func_start(llvm_func, gtype, gfunc)
+
+    global = @mod.globals[gfunc.func.metadata[:ffi_link_name]]
+
+    value = llvm_func.params[0]
+    @builder.store(value, global)
+
+    @builder.ret(value)
+
+    gen_func_end(gfunc)
   end
 
   def gen_intrinsic_cpointer(gtype, gfunc, llvm_func)
@@ -1851,14 +1888,14 @@ class Savi::Compiler::CodeGen
       args,
       arg_exprs,
       arg_frames,
-      gfunc.func.has_tag?(:ffi) ? gfunc : nil,
+      gfunc.func.has_tag?(:ffi_call) ? gfunc : nil,
       needs_virtual_call,
       use_receiver,
       !cont.nil?,
     )
 
     # If this was an FFI call, skip calling-convention-specific handling.
-    return result if gfunc.func.has_tag?(:ffi)
+    return result if gfunc.func.has_tag?(:ffi_call)
 
     case gfunc.calling_convention
     when GenFunc::Simple
@@ -3201,7 +3238,6 @@ class Savi::Compiler::CodeGen
     global.linkage = LLVM::Linkage::Private
     global.initializer = const
     global.global_constant = true
-    global.unnamed_addr = true
     global
   end
 

src/savi/ext/llvm/lib_llvm.cr

@@ -12,6 +12,8 @@ lib LibLLVM
   fun const_bit_cast = LLVMConstBitCast(value : ValueRef, to_type : TypeRef) : ValueRef
   fun set_unnamed_addr = LLVMSetUnnamedAddr(global : ValueRef, is_unnamed_addr : Int32)
   fun is_unnamed_addr = LLVMIsUnnamedAddr(global : ValueRef) : Int32
+  fun set_externally_initialized = LLVMSetExternallyInitialized(global : ValueRef, is_externally_initialized : Int32)
+  fun is_externally_initialized = LLVMIsExternallyInitialized(global : ValueRef) : Int32
   fun parse_bitcode_in_context = LLVMParseBitcodeInContext(context : ContextRef, mem_buf : MemoryBufferRef, out_m : ModuleRef*, out_message : UInt8**) : Int32
   fun link_modules = LLVMLinkModules2(dest : ModuleRef, src : ModuleRef) : Int32
   fun strip_module_debug_info = LLVMStripModuleDebugInfo(mod : ModuleRef) : Bool

src/savi/ext/llvm/value_methods.cr

@@ -7,6 +7,14 @@ module LLVM::ValueMethods
     LibLLVM.is_unnamed_addr(self) != 0
   end
 
+  def externally_initialized=(externally_initialized)
+    LibLLVM.set_externally_initialized(self, externally_initialized ? 1 : 0)
+  end
+
+  def externally_initialized?
+    LibLLVM.is_externally_initialized(self) != 0
+  end
+
   def dll_storage_class : LLVM::DLLStorageClass
     LibLLVM.get_dll_storage_class(self)
   end

src/savi/program.cr

@@ -389,6 +389,10 @@ class Savi::Program
       :constructor,
       :copies,
       :ffi,
+      :ffi_call,
+      :ffi_global_constant,
+      :ffi_global_getter,
+      :ffi_global_setter,
       :field,
       :hygienic,
       :inline,

src/savi/program/declarator/intrinsic.cr

@@ -304,25 +304,63 @@ module Savi::Program::Intrinsic
 
         scope.on_body { |body| function.body = body }
       when "ffi"
-        name, params =
-          AST::Extract.name_and_params(terms["name_and_params"].not_nil!)
+        if terms["thread_local"]?
+          raise NotImplementedError.new(":ffi thread_local")
+        elsif terms["global"]?
+          name = terms["name"].as(AST::Identifier)
+          t = terms["type"].as(AST::Term)
 
-        scope.current_function = function = Program::Function.new(
-          AST::Identifier.new("non").from(declare.terms.first),
-          name,
-          params,
-          terms["ret"]?.as(AST::Term?),
-        )
-
-        function.add_tag(:ffi)
-        function.add_tag(:inline)
-        function.add_tag(:variadic) if terms["variadic"]?
-
-        ffi_link_name = function.ident.value
-        ffi_link_name = ffi_link_name[0...-1] if ffi_link_name.ends_with?("!")
-        function.metadata[:ffi_link_name] = ffi_link_name
+          scope.current_function = function = Program::Function.new(
+            AST::Identifier.new("non").from(declare.terms.first),
+            name,
+            nil,
+            t,
+          )
+
+          ffi_link_name = function.ident.value
+          ffi_link_name = ffi_link_name[0...-1] if ffi_link_name.ends_with?("!")
+          function.metadata[:ffi_link_name] = ffi_link_name
+          function.add_tag(:ffi)
+          function.add_tag(:ffi_global_getter)
+          function.add_tag(:inline)
+          function.body = nil
+
+          if terms["var_or_let"].as(AST::Identifier).value == "let"
+            function.add_tag(:ffi_global_constant)
+          else
+            setter_function = Program::Function.new(
+              AST::Identifier.new("non").from(declare.terms.first),
+              AST::Identifier.new("#{name.value}=").from(name),
+              AST::Group.new("(", [t]).from(t),
+              t,
+            )
+
+            setter_function.metadata[:ffi_link_name] = ffi_link_name
+            setter_function.add_tag(:ffi)
+            setter_function.add_tag(:ffi_global_setter)
+            setter_function.add_tag(:inline)
+            setter_function.body = nil
+          end
+        else
+          name, params =
+            AST::Extract.name_and_params(terms["name_and_params"].not_nil!)
 
-        function.body = nil
+          scope.current_function = function = Program::Function.new(
+            AST::Identifier.new("non").from(declare.terms.first),
+            name,
+            params,
+            terms["ret"]?.as(AST::Term?),
+          )
+
+          ffi_link_name = function.ident.value
+          ffi_link_name = ffi_link_name[0...-1] if ffi_link_name.ends_with?("!")
+          function.metadata[:ffi_link_name] = ffi_link_name
+          function.add_tag(:ffi)
+          function.add_tag(:ffi_call)
+          function.add_tag(:inline)
+          function.add_tag(:variadic) if terms["variadic"]?
+          function.body = nil
+        end
       when "let", "var"
         type = scope.current_type
 
@@ -583,9 +621,23 @@ module Savi::Program::Intrinsic
       when "foreign_name"
         name = terms["name"].as(AST::Identifier)
         scope.current_function.metadata[:ffi_link_name] = name.value
+
+        if scope.current_function.has_tag?(:ffi_global_getter)
+          setter_function = scope.current_type.functions.last
+          if setter_function.has_tag?(:ffi_global_setter)
+            setter_function.metadata[:ffi_link_name] = name.value
+          end
+        end
       when "link_lib"
         name = terms["name"].as(AST::Identifier)
         scope.current_function.metadata[:ffi_link_lib] = name.value
+
+        if scope.current_function.has_tag?(:ffi_global_getter)
+          setter_function = scope.current_type.functions.last
+          if setter_function.has_tag?(:ffi_global_setter)
+            setter_function.metadata[:ffi_link_lib] = name.value
+          end
+        end
       else
         raise NotImplementedError.new(declarator.pretty_inspect)
       end

src/savi/program/declarator/term_acceptor.cr

@@ -21,7 +21,7 @@ abstract class Savi::Program::Declarator::TermAcceptor
     end
 
     def name : String
-      "_" # we don't care about where we save the term
+      @keyword
     end
 
     def try_accept(term : AST::Term) : AST::Term?