tools-island: a seam has two halves, and scan is where they are compared

The generator served one island. A seam has two implementations of one
`extern "C"` boundary -- a device island and a host fallback -- and exactly
one of them is in any link, which is the arrangement every example under
`examples/09-heterogeneous` has. Handed only the device half, the generator
produced an empty boundary in a `--no-accel` build and the C++ side failed
on an unresolved name.

`scan` now merges entries by name, so a build program hands it both files
unconditionally: both exist on disk in either build, and which one is
compiled is the manifest's decision rather than a condition the build
program repeats.

Two definitions of one name that declare it differently are refused there,
naming both files and both signatures. Nothing else catches that. The two
halves are never in one translation unit and never in one link, and C
language linkage does not mangle, so a build with disagreeing halves is
clean and the artifact reads its arguments by whichever signature it was
compiled with. `scan` is the only point at which both texts exist at once.

The fixture gains the host half under `cfg(not(accelerator = "vulkan"))`,
so both legs are built and run, and a CI step perturbs one signature and
asserts the refusal names both files. The README paragraph claiming the two
could not disagree under `scan` was true only while `scan` took one file.

Author: Sunrisepeak

.github/workflows/ci.yml

@@ -243,16 +243,14 @@ jobs:
           # island reads the generated header through the compiler's
           # forced-include flag -- so a project using this generator has no
           # header in its source tree and no line naming one.
-          if grep -rn '#include' src/main.cpp src/kernels/saxpy.c; then
+          if grep -rn '#include' src/main.cpp src/kernels/saxpy.c src/cpu/saxpy.c; then
             echo "FAIL: a source names an include; the generator exists to remove it"
             exit 1
           fi
           # AND THE CHECK THAT INCLUDE USED TO DO IS STILL THERE. The compiler
-          # sees the declarations, so a definition whose signature drifted fails
-          # where it was written. Verified by hand against this fixture with a
-          # hand-written entry list declaring `double n` where the definition
-          # says `unsigned n`: `saxpy.c:22: error: conflicting types for
-          # 'scale_device'`, at the definition rather than at the link.
+          # sees the declarations, so a definition whose signature drifted from
+          # the generated header fails where it was written rather than at the
+          # link.
           grep -q 'include' build.mcpp \
             || { echo "FAIL: the build program no longer forces the header in"; exit 1; }
           # THE MARKER SELECTS, AND THE UNMARKED MUST NOT TRAVEL. The island has
@@ -275,6 +273,77 @@ jobs:
             "$d/island_interface.kernels.h"
           echo "ok: one declaration, two generated artefacts, and only what was marked"
 
+      # THE SEAM'S OTHER HALF, AND THE DISAGREEMENT THAT NOTHING ELSE CATCHES.
+      #
+      # A seam has two implementations of one `extern "C"` boundary -- a device
+      # island and a host fallback -- and exactly one of them is in any link.
+      # `build.mcpp` hands `scan` both files unconditionally, because both exist
+      # on disk in either build and the manifest decides which is compiled. A
+      # build program that asked `mcpp::accel()` instead would carry a second
+      # copy of a decision the manifest already states.
+      #
+      # The CPU leg is not a variant of the step above. It is the build in which
+      # the device half is ABSENT, so a generator that read only the file it was
+      # given first would produce an empty boundary and the C++ side would fail
+      # on an unresolved name.
+      - name: the same boundary, generated from the half the build selected
+        working-directory: tests/island-interface
+        run: |
+          rm -rf target
+          "$MCPP" build --no-accel > cpu.log 2>&1 || {
+            echo "FAIL: the CPU leg does not build"; tail -25 cpu.log; exit 1; }
+          "$MCPP" run --no-accel | tee cpurun.log
+          grep -q '^out\[0\]=6 out\[1\]=12 out\[2\]=18 out\[3\]=24' cpurun.log
+          grep -q '^all ok' cpurun.log
+          # ONE set of declarations, not two. Both files mark the same two entry
+          # points; a merge that appended would declare each twice, which the
+          # module rejects as a redefinition and the header silently accepts.
+          d=target/.build-mcpp/out/island
+          n=$(grep -c '^export using ::' "$d/island_interface.kernels.cppm")
+          [ "$n" = 2 ] || {
+            echo "FAIL: the module re-exports $n names; the two halves declare 2"
+            cat "$d/island_interface.kernels.cppm"; exit 1; }
+          echo "ok: the CPU leg reaches the same generated boundary"
+
+      # AND THE REVERSE LEG, WHICH IS THE POINT OF SCANNING BOTH.
+      #
+      # C language linkage does not mangle and the two halves are never in one
+      # link, so two declarations of one name that disagree produce a clean
+      # build and an artifact that reads its arguments by whichever signature it
+      # happened to be compiled with. `scan` is the only place both texts exist
+      # at once, so it is the only place that can refuse.
+      - name: two halves that disagree are refused where both texts exist
+        working-directory: tests/island-interface
+        run: |
+          cp src/cpu/saxpy.c /tmp/cpu_saxpy.bak
+          sed -i 's/^int scale_device(float a, float\* out, unsigned n) {/int scale_device(float a, float* out, double n) {/' \
+            src/cpu/saxpy.c
+          grep -q 'double n' src/cpu/saxpy.c || {
+            echo "FAIL: the fixture was not perturbed; this step would assert nothing"
+            cp /tmp/cpu_saxpy.bak src/cpu/saxpy.c; exit 1; }
+          rm -rf target
+          set +e
+          "$MCPP" build > neg.log 2>&1
+          rc=$?
+          set -e
+          cp /tmp/cpu_saxpy.bak src/cpu/saxpy.c
+          [ "$rc" != 0 ] || { echo "FAIL: the disagreement was accepted"; tail -20 neg.log; exit 1; }
+          grep -q 'declare it differently' neg.log || {
+            echo "FAIL: refused, but not for this reason"; tail -20 neg.log; exit 1; }
+          # Both file names, because a diagnostic naming one leaves the reader
+          # to find the other.
+          grep -q 'src/kernels/saxpy.c' neg.log
+          grep -q 'src/cpu/saxpy.c' neg.log
+          echo "ok: refused, naming both definitions and both signatures"
+          # And it builds again once they agree, so this step cannot leave a
+          # fixture that refuses everything.
+          rm -rf target
+          "$MCPP" build > restored.log 2>&1 || {
+            echo "FAIL: the fixture no longer builds after the perturbation was undone"
+            tail -20 restored.log; exit 1; }
+          rm -f neg.log restored.log cpu.log cpurun.log
+          echo "ok: and it builds again once the two agree"
+
       # NO BUILD PROGRAM AT ALL, WHICH IS WHAT THE TWO KEYS BUY.
       #
       # `spirv-consumer` and `spirv-module-consumer` both write a `build.mcpp`,

README.md

@@ -274,8 +274,8 @@ The entry points are marked where they are defined, and the signature exists
 once:
 
 ```c
-// src/kernels/saxpy.cu
-#include "myapp.kernels.h"          // generated; defines the marker as nothing
+// src/kernels/saxpy.cu -- no include: the generated header arrives through the
+// compiler's forced-include flag, which is what defines the marker as nothing.
 
 MCPP_EXPORT_C
 int saxpy_device(float a, const float* x, const float* y, float* out, unsigned n) { ... }
@@ -327,17 +327,44 @@ until mcpp writes it.
 
 **The check that include used to do is still there.** The compiler sees the
 declarations, so a definition whose signature drifted from its declaration fails
-where it was written rather than at the link. Verified against the fixture with
-a hand-written entry list declaring `double n` where the definition says
-`unsigned n`:
+where it was written rather than at the link:
 
 ```
 src/kernels/saxpy.c:22:5: error: conflicting types for 'scale_device'
 ```
 
-That check is only reachable on the `emit` path, where the list and the
-definition are separate things. Under `scan` the header is generated *from* the
-definition, so the two cannot disagree at all.
+**A seam has two halves, and `scan` is where they are compared.** A device
+island and a host fallback implement one `extern "C"` boundary, and exactly one
+of them is in any link. A build program hands `scan` both, unconditionally --
+both files exist on disk in either build, and which one is compiled is the
+manifest's decision rather than a condition the build program repeats:
+
+```cpp
+const std::vector<std::string> islands{
+    std::string(mcpp::manifest_dir()) + "/src/kernels/saxpy.cu",
+    std::string(mcpp::manifest_dir()) + "/src/cpu/saxpy.cpp",
+};
+```
+
+Entries are merged by name, so the two halves produce one set of declarations.
+Two definitions of one name that declare it **differently** are refused, naming
+both files and both signatures:
+
+```
+mcpp.tools.island: two definitions of `scale_device` declare it differently.
+  src/kernels/saxpy.c
+    int scale_device(float a, float* out, unsigned n)
+  src/cpu/saxpy.c
+    int scale_device(float a, float* out, double n)
+  C language linkage does not mangle, so these never meet at the link:
+  whichever one is in the artifact reads its arguments by its own signature.
+```
+
+Nothing else in the toolchain catches that. The two halves are never in one
+translation unit and never in one link, and C linkage does not mangle, so a
+build with disagreeing halves is clean and the artifact reads its arguments by
+whichever signature it was compiled with. `scan` is the only point at which both
+texts exist at once.
 
 **The declaration still exists once.** Without this, a project writes it twice --
 in a header, and again wherever the C++ side reaches it. C language linkage does

tests/island-interface/build.mcpp

@@ -16,8 +16,14 @@ import mcpp.tools.island;
 // linkage does not mangle, so two copies that disagree are one symbol: the link
 // is clean and each side reads the arguments by its own ABI.
 int main() {
+    // BOTH HALVES, UNCONDITIONALLY. Which one the build compiles is the
+    // manifest's decision and this program does not repeat it: the declarations
+    // are the same either way, and `scan` merges them by entry name. A build
+    // program that asked `mcpp::accel()` here would be a second copy of a
+    // decision the manifest already states -- and the copy that goes stale.
     const std::vector<std::string> islands{
         std::string(mcpp::manifest_dir()) + "/src/kernels/saxpy.c",
+        std::string(mcpp::manifest_dir()) + "/src/cpu/saxpy.c",
     };
     for (auto const& f : islands) mcpp::rerun_if_changed(f.c_str());
 
@@ -28,6 +34,10 @@ int main() {
 
     const auto entries = mcpp::tools::island::scan(islands, opt);
     if (!entries) return 1;
+    // TWO, not four. Both files mark the same two entry points, and a merge
+    // that appended instead would declare each one twice -- which the module
+    // rejects as a redefinition and the header does not, so the count is the
+    // criterion that catches it in both surfaces.
     if (entries->size() != 2) {
         std::cerr << std::format("expected two marked entry points, found {}\n",
                                  entries->size());

tests/island-interface/mcpp.toml

@@ -21,6 +21,7 @@ name        = "island-interface"
 namespace   = "example"
 version     = "0.1.0"
 description = "An extern \"C\" island reached through a generated module"
+accelerators = ["vulkan"]
 
 [language]
 standard   = "c++23"
@@ -30,8 +31,21 @@ import_std = true
 [build-dependencies.mcpp]
 plugins = { path = "../..", features = ["tools-island"], host-module = true }
 
+# ONE BOUNDARY, TWO IMPLEMENTATIONS, AND EXACTLY ONE OF THEM IN ANY LINK.
+#
+# This is the arrangement every seam in `examples/09-heterogeneous` has, and it
+# is what the generator has to serve without the project writing a condition:
+# `build.mcpp` hands `scan` both files unconditionally, because both exist on
+# disk in either build, and the manifest decides which one is compiled.
 [build]
-sources = ["src/*.cpp", "src/kernels/*.c"]
+accel   = "vulkan1.2"
+sources = ["src/*.cpp"]
+
+[target.'cfg(accelerator = "vulkan")'.build]
+sources = ["src/kernels/*.c"]
+
+[target.'cfg(not(accelerator = "vulkan"))'.build]
+sources = ["src/cpu/*.c"]
 
 [targets.island-interface]
 kind = "bin"

tests/island-interface/src/cpu/saxpy.c

@@ -0,0 +1,30 @@
+/* The host half of the same boundary.
+ *
+ * The signatures here and in `../kernels/saxpy.c` are the same declarations,
+ * and that is a property nothing else in the toolchain checks: C language
+ * linkage does not mangle, exactly one of these files is in any link, and two
+ * that disagreed would each read the arguments its own way.
+ *
+ * `mcpp::tools::island::scan` is handed both, so it is the one place where both
+ * texts exist at once -- and it refuses a disagreement there, with both file
+ * names, instead of leaving it to the run.
+ *
+ * No include here either. The generated boundary header arrives through the
+ * compiler's forced-include flag, the same way it reaches the device half. */
+
+MCPP_EXPORT_C
+int saxpy_device(float a, const float* x, const float* y, float* out, unsigned n) {
+    for (unsigned i = 0; i < n; ++i) out[i] = a * x[i] + y[i];
+    return 0;
+}
+
+MCPP_EXPORT_C
+int scale_device(float a, float* out, unsigned n) {
+    for (unsigned i = 0; i < n; ++i) out[i] = a * out[i];
+    return 0;
+}
+
+/* The host half has its own internals, and they must not reach the boundary
+ * any more than the device half's do. */
+static int host_only_helper(int x) { return x - 1; }
+int internal_device(int x) { return host_only_helper(x); }

tools/island.cppm

@@ -159,9 +159,25 @@ inline std::string entry_name(std::string_view decl) {
 // point that produced no declaration would leave the island defining a function
 // nothing declares, and the consumer's failure would be an unresolved name in a
 // different file.
+//
+// SEVERAL SOURCES MAY DEFINE THE SAME ENTRY POINT, AND THEY MUST AGREE.
+//
+// That is the ordinary shape of a seam: a device island and a host fallback
+// define one boundary, and exactly one of them is in any given link. A build
+// program hands both to this function and gets one set of declarations back,
+// so it does not have to ask which build it is in.
+//
+// Two definitions of one name whose declarations differ are REFUSED here,
+// naming both files. Nothing else in the toolchain catches that: C language
+// linkage does not mangle, so the two never meet at the link, and whichever
+// one is present reads its arguments by its own idea of the signature. This is
+// the only point at which both texts exist at once.
 inline std::optional<std::vector<std::string>>
 scan(std::span<const std::string> sources, const options& opt) {
     std::vector<std::string> entries;
+    // Where each entry was found, for the disagreement diagnostic. Parallel to
+    // `entries`, which is the return value and cannot carry it.
+    std::vector<std::string> origin;
     for (auto const& src : sources) {
         std::ifstream in(src);
         if (!in) {
@@ -226,7 +242,33 @@ scan(std::span<const std::string> sources, const options& opt) {
                     flat += c;
                 }
             }
-            if (!flat.empty()) entries.push_back(std::move(flat));
+            if (flat.empty()) continue;
+
+            // MERGED BY ENTRY NAME. A second definition of a name already seen
+            // is either the same declaration -- the seam's two halves agreeing,
+            // which is the expected case -- or a disagreement that has to stop
+            // the build here.
+            const auto name = entry_name(flat);
+            std::size_t seen = entries.size();
+            for (std::size_t k = 0; k < entries.size(); ++k)
+                if (entry_name(entries[k]) == name) { seen = k; break; }
+
+            if (seen == entries.size()) {
+                entries.push_back(std::move(flat));
+                origin.push_back(src);
+                continue;
+            }
+            if (entries[seen] == flat) continue;   // both halves agree
+
+            std::cerr << std::format(
+                "mcpp.tools.island: two definitions of `{}` declare it differently.\n"
+                "  {}\n    {}\n"
+                "  {}\n    {}\n"
+                "  C language linkage does not mangle, so these never meet at the "
+                "link:\n  whichever one is in the artifact reads its arguments by its "
+                "own signature.\n",
+                name, origin[seen], entries[seen], src, flat);
+            return std::nullopt;
         }
     }
     return entries;