From 5a67f9f330332f8957976841819707d09124dc94 Mon Sep 17 00:00:00 2001
From: Blaine Bublitz <blaine.bublitz@gmail.com>
Date: Tue, 14 Sep 2021 17:36:33 -0700
Subject: [PATCH] feat!: Remove MacOS-specific library flags (#111)

This now requires consumers to specify `-cc clang++` on their
executables when compiling on MacOS. This is the only way to get
Binaryen to link into OCaml code on Mac's Arm64 architecture.
---
 README.md                     | 18 ++++++++++++++++++
 src/config/dune               |  8 --------
 src/config/library_flags.ml   | 16 ----------------
 src/dune                      |  2 --
 test/config/dune              |  8 ++++++++
 test/config/ocamlopt_flags.ml | 16 ++++++++++++++++
 test/dune                     |  4 ++++
 7 files changed, 46 insertions(+), 26 deletions(-)
 delete mode 100644 src/config/dune
 delete mode 100644 src/config/library_flags.ml
 create mode 100644 test/config/dune
 create mode 100644 test/config/ocamlopt_flags.ml

diff --git a/README.md b/README.md
index 1ff7ba3a..de5d607e 100644
--- a/README.md
+++ b/README.md
@@ -42,6 +42,24 @@ This project aims to provide full feature parity with the [Binaryen C API](https
 
 None of these are particularly challenging to create bindings for—they just haven't been written yet. If you need anything that's missing, feel free to open a PR.
 
+## MacOS C++ Compiler
+
+When including this library in your `dune` MacOS executables, you'll need to specify `-cc clang++` in your `(ocamlopt_flags)` stanza. This is required because Binaryen will throw errors for itself to catch and using `clang++` is the only way to handle them correctly. You can find more info on this [ocaml issue](https://github.com/ocaml/ocaml/issues/10423).
+
+Your stanza could look something like this:
+
+```diff
+ (executable
+  (name example)
+  (public_name example)
+  (package example)
++ (ocamlopt_flags -cc clang++)
+  (modules example)
+  (libraries binaryen))
+```
+
+These flags likely won't work on other operating systems, so you'll probably need to use `dune-configurator` to vary the flags per platform. You can see an example of this in our [tests/](./tests/dune).
+
 ## Static Linking
 
 If you are planning to create portable binaries for Windows, it will try to find Cygwin/MinGW locations in your `PATH`. To avoid this, you probably want to add this to your `(executable)` stanzas:
diff --git a/src/config/dune b/src/config/dune
deleted file mode 100644
index 7c4e3de6..00000000
--- a/src/config/dune
+++ /dev/null
@@ -1,8 +0,0 @@
-(executable
- (name library_flags)
- (libraries dune.configurator))
-
-(rule
- (targets library_flags.sexp)
- (action
-  (run ./library_flags.exe)))
diff --git a/src/config/library_flags.ml b/src/config/library_flags.ml
deleted file mode 100644
index f7720e87..00000000
--- a/src/config/library_flags.ml
+++ /dev/null
@@ -1,16 +0,0 @@
-module C = Configurator.V1
-
-let () =
-  C.main ~name:"library_flags" (fun c ->
-      let default = [] in
-
-      let library_flags =
-        match C.ocaml_config_var c "system" with
-        | Some "macosx" ->
-            (* These flags preserve the original C error behavior *)
-            [ "-cclib"; "-Wl,-keep_dwarf_unwind" ]
-        | Some _ -> default
-        | None -> default
-      in
-
-      C.Flags.write_sexp "library_flags.sexp" library_flags)
diff --git a/src/dune b/src/dune
index ddd8c19d..294a05ae 100644
--- a/src/dune
+++ b/src/dune
@@ -11,8 +11,6 @@
     binaryen_stubs_modules binaryen_stubs_settings ocaml_helpers)
   (flags -O2 -Wall -Wextra))
  (foreign_archives binaryen)
- (library_flags
-  (:include ./config/library_flags.sexp))
  (c_library_flags -lstdc++ -lpthread))
 
 (rule
diff --git a/test/config/dune b/test/config/dune
new file mode 100644
index 00000000..b5d8ea6a
--- /dev/null
+++ b/test/config/dune
@@ -0,0 +1,8 @@
+(executable
+ (name ocamlopt_flags)
+ (libraries dune.configurator))
+
+(rule
+ (targets ocamlopt_flags.sexp)
+ (action
+  (run ./ocamlopt_flags.exe)))
diff --git a/test/config/ocamlopt_flags.ml b/test/config/ocamlopt_flags.ml
new file mode 100644
index 00000000..df053bd9
--- /dev/null
+++ b/test/config/ocamlopt_flags.ml
@@ -0,0 +1,16 @@
+module C = Configurator.V1
+
+let () =
+  C.main ~name:"ocamlopt_flags" (fun c ->
+      let default = [] in
+
+      let ocamlopt_flags =
+        match C.ocaml_config_var c "system" with
+        | Some "macosx" ->
+            (* These flags preserve the original C++ error behavior. Ref https://github.com/ocaml/ocaml/issues/10423 *)
+            [ "-cc"; "clang++" ]
+        | Some _ -> default
+        | None -> default
+      in
+
+      C.Flags.write_sexp "ocamlopt_flags.sexp" ocamlopt_flags)
diff --git a/test/dune b/test/dune
index d7a8010e..41ae72f3 100644
--- a/test/dune
+++ b/test/dune
@@ -4,6 +4,8 @@
  (name test)
  (modules test)
  (libraries binaryen)
+ (ocamlopt_flags
+  (:include ./config/ocamlopt_flags.sexp))
  (action
   (run %{test})))
 
@@ -20,6 +22,8 @@
  (name test_native)
  (modules test_native)
  (libraries binaryen binaryen.native)
+ (ocamlopt_flags
+  (:include ./config/ocamlopt_flags.sexp))
  (action
   (run %{test})))