ci: Gate the compiler warnings the glue answers for, in rcc and in each

.github/workflows/custom/after-install/action.yml

@@ -32,6 +32,16 @@ runs:
       run: Rscript tools/split-aaa-tests.R --check
       shell: bash
 
+    # The compiler-warning gate (handbook/build/warnings/): compiles the glue
+    # in src/ with the gate's warning flags, in a makefile of its own rather
+    # than ~/.R/Makevars, which R CMD check would read too. Fails on a warning
+    # in a glue file and only counts those in src/vendor/. Seconds, and the
+    # answer is the same in every matrix entry, so it runs in the smoke test only.
+    - name: Check that the glue compiles without warnings
+      if: github.job == 'rcc-smoke' && runner.os != 'Windows'
+      run: scripts/warnings.sh glue
+      shell: bash
+
     # The roclet that links the C manual from the reference pages, named in
     # DESCRIPTION's Roxygen field, is vendored in tools/igraph.r2cdocs
     # (handbook/architecture/r-layer/documentation/). Only the smoke test

handbook/README.md

@@ -11,7 +11,7 @@ What stays outside the tree, intent and evidence, and where each lives, is [`met
 
 * [`usage/`](usage/): installing the package and its flavors, and the deprecated dotted function names
 * [`architecture/`](architecture/): the R layer, the C glue, the vendored igraph C library, and the Stimulus code generator
-* [`build/`](build/): the source build and its configuration
+* [`build/`](build/): the source build and its configuration, and the compiler warnings it may raise
 * [`testing/`](testing/): the suite, snapshots, guards, and reverse dependencies
 * [`branches/`](branches/): the series and their refs, the mirrors, the flavors, the invariants
 * [`operations/`](operations/): vendoring, CI, releases, review, triage, and the site

handbook/build/README.md

@@ -3,3 +3,4 @@
 Turning the tree into an installed package.
 
 * [`source-build/`](source-build/): `configure`, the Makevars, and the build artifacts
+* [`warnings/`](warnings/): what the compiler may say about the glue and the vendored code, and the gate that checks it

handbook/build/warnings/README.md

@@ -0,0 +1,65 @@
+# Warnings
+
+What the compiler may say about the code this repository owns, and where that is checked.
+[`scripts/warnings.sh`](/scripts/warnings.sh) is the check, and its header says how to run it.
+
+**The glue compiles without warnings; the vendored code is counted, never enforced.**
+A warning belongs to the file it points at, not to the translation unit that raised it,
+so a glue file that includes an igraph header does not answer for what that header says.
+
+* **Glue** is `src/` outside `src/vendor/`: the hand-written C and C++,
+  `src/rinterface.c`, whose generator and definitions live here ([`architecture/stimulus/`](/handbook/architecture/stimulus/README.md)),
+  and `src/cpp11.cpp`.
+  A warning there is fixed here, at its cause: in the source, or in the generator for a generated file.
+* **Vendored** is `src/vendor/`: the igraph C library and what it vendors in turn, ARPACK's Fortran among it.
+  A warning there is fixed upstream, in igraph/igraph, and arrives with the commit the series vendors
+  ([`operations/vendoring/model/`](/handbook/operations/vendoring/model/README.md)).
+  The gate never fails on it, so the series replays upstream as it is.
+* **External** is a header outside the checkout, R's or cpp11's, and is counted like the vendored code.
+
+The one answer never available is silencing a single site with a `#pragma` or a cast that only hides the warning.
+
+## The flags
+
+`-Wall -pedantic` is what CRAN's own GCC checks compile with, and the gate starts there.
+`-Wextra` is added, because on the glue it costs nothing today.
+Two categories are then switched off, each for a reason the glue cannot remove:
+
+* **`-Wunused-parameter`.**
+  Callbacks implement signatures igraph and R dictate, the RNG and attribute handlers among them.
+* **`-Wcast-function-type`.**
+  R registers native routines through `DL_FUNC`, a cast `cpp11::cpp_register()` writes for every entry point in `src/cpp11.cpp`.
+
+ISO C's conversions between function and object pointers are dropped from the report,
+the exemption R CMD check makes for them too:
+igraph keeps an R closure behind `igraph_function_pointer_t`, so the glue cannot avoid one.
+
+**None of this reaches `src/Makevars` or `~/.R/Makevars`.**
+CRAN rejects `-Wno-*` in a package's own flags,
+and a user Makevars is read by every later build in the job, R CMD check's among them.
+The gate compiles the glue objects through a makefile of its own and leaves the package build alone.
+
+## What R CMD check already catches
+
+R CMD check scans the install log for a fixed list of *significant warnings*
+(`tools:::.check_packages`: implicit declarations, `-Wformat`, `-Wstrict-prototypes` and a few more),
+and the `check` step fails on any WARNING, vendored code included.
+It sees only the flags the job's R was built with, which are fewer than CRAN's.
+A full build under CRAN's `-Wall -pedantic` raises none of the significant ones today,
+so the gate here is for the warnings CRAN prints without failing on.
+
+## Where it runs
+
+* **On every pull request**, in the `rcc` smoke test, through `custom/after-install`.
+  The answer is the same in every matrix entry, so it runs in one.
+* **On every series commit**, as the `warnings` gate of [`scripts/rcc-one.sh`](/scripts/rcc-one.sh)
+  ([`operations/ci/per-commit/`](/handbook/operations/ci/per-commit/README.md)).
+  A vendored commit that makes the glue warn, a renamed or deprecated declaration say,
+  is repaired on the R side like any other breakage the walk surfaces
+  ([`operations/vendoring/adaptation/`](/handbook/operations/vendoring/adaptation/README.md)).
+* **Locally**, `scripts/warnings.sh` for the glue in seconds,
+  and `scripts/warnings.sh vendored` for the whole vendored tree, which lists its warnings and always succeeds.
+
+The gate is GCC's, and CI's GCC is newer than a development box's,
+so a run that is clean locally is evidence rather than proof.
+Clang is welcome as a second opinion, and is not wired into the gate.

scripts/rcc-one.sh

@@ -23,6 +23,7 @@
 #   | style                 | gate `style`                                    |
 #   | update-snapshots      | gate `snapshots`, incl. the snapshot-<sha> branch |
 #   | roxygenize            | gate `roxygen`                                  |
+#   | custom/after-install  | gate `warnings`, scripts/warnings.sh glue       |
 #   | render-readme         | skipped upstream too (no Config/cynkra/render-readme) |
 #   | commit                | gate `clean` -- workflow_dispatch fails on any diff |
 #   | check                 | gate `check`                                    |
@@ -43,7 +44,7 @@
 #
 # Environment variables:
 #   EACH_GATES            - space-separated subset of
-#                           "regen flavor style snapshots roxygen clean check pkgdown"
+#                           "regen flavor style snapshots roxygen clean warnings check pkgdown"
 #                           (default: all of them, parity with rcc-smoke)
 #   EACH_SNAPSHOT_BRANCH  - publish the snapshot-<sha>-rcc-smoke-null branch when
 #                           the snapshots gate changed anything
@@ -66,7 +67,7 @@
 
 set -uo pipefail
 
-ALL_GATES="regen flavor style snapshots roxygen clean check pkgdown"
+ALL_GATES="regen flavor style snapshots roxygen clean warnings check pkgdown"
 GATES="${EACH_GATES:-${ALL_GATES}}"
 SNAPSHOT_BRANCH="${EACH_SNAPSHOT_BRANCH:-${GITHUB_ACTIONS:-false}}"
 STAGE_DIR="${EACH_STAGE_DIR:-}"
@@ -191,6 +192,7 @@ stage_budget() {
     snapshots) echo 1800 ;;  # the snapshot tests: ~350 s
     roxygen) echo 900 ;;     # ~60 s, after installing the vendored roclet
     clean) echo 900 ;;
+    warnings) echo 600 ;;    # recompiles the ten glue files: ~10 s
     check) echo 2700 ;;      # rebuilds the package, then the suite again: ~400 s
     pkgdown) echo 2700 ;;    # every reference page and article: ~650 s
     *) echo 3600 ;;
@@ -596,6 +598,13 @@ gate_clean() {
   return 1
 }
 
+# The compiler-warning gate (handbook/build/warnings/): the glue must compile
+# without warnings under the gate's flags; what src/vendor/ says is counted, so
+# a vendored commit that brings a new upstream warning does not turn red here.
+gate_warnings() {
+  scripts/warnings.sh glue
+}
+
 # Mirrors r-lib/actions/check-r-package@v2 as invoked by
 # cynkra/cynkratemplate's check action.
 gate_check() {

scripts/warnings.sh

@@ -0,0 +1,128 @@
+#!/bin/bash
+# The compiler-warning gate: compile part of src/ with the gate's warning flags
+# and judge what the compiler says about the files this repository owns.
+# Handbook: handbook/build/warnings/README.md
+#
+# Usage:
+#   scripts/warnings.sh [glue]    # compile the glue, fail on a glue warning
+#   scripts/warnings.sh vendored  # compile everything else, report and exit 0
+#
+# A warning belongs to the file it points at, not to the translation unit that
+# raised it: a glue file is not answerable for what an igraph header says.
+# So every warning is attributed to a scope by its path,
+# only the glue's fail the gate, and the others are counted.
+#
+# The flags reach this one compile through a makefile of its own,
+# never src/Makevars (CRAN rejects -Wno-* there)
+# and never ~/.R/Makevars (every later build in the job would read it,
+# R CMD check among them, and turn these warnings into its own).
+# The user Makevars is included, so ccache's wrappers still apply.
+#
+# Linux and macOS only: it needs the src/Makevars that ./configure writes.
+
+set -euo pipefail
+
+cd "$(dirname "$0")/.."
+root="$(pwd)"
+
+scope="${1:-glue}"
+case "${scope}" in
+  glue | vendored) ;;
+  *)
+    echo "Usage: scripts/warnings.sh [glue|vendored]" >&2
+    exit 2
+    ;;
+esac
+
+# The reasons for each flag are the handbook page's.
+WARNINGS="-Wall -Wextra -pedantic -Wno-unused-parameter -Wno-cast-function-type"
+
+tmp="$(mktemp -d)"
+trap 'rm -rf "${tmp}"' EXIT
+
+user_makevars="${R_MAKEVARS_USER:-${HOME}/.R/Makevars}"
+{
+  [ -f "${user_makevars}" ] && echo "include ${user_makevars}"
+  # What R CMD INSTALL puts on the command line for the LinkingTo packages.
+  Rscript -e 'l <- tools:::.get_requires_from_package_db(read.dcf("DESCRIPTION")[1, ], "LinkingTo")
+    cat("CLINK_CPPFLAGS =", paste0("-I\"", vapply(l, function(p) system.file("include", package = p), ""), "\""), "\n")'
+  for v in CFLAGS CXXFLAGS CXX11FLAGS CXX14FLAGS CXX17FLAGS CXX20FLAGS FFLAGS; do
+    echo "${v} += ${WARNINGS}"
+  done
+  echo 'print-%: ; @echo "objects: $($*)"'
+} > "${tmp}/warnings.mk"
+
+[ -f src/Makevars ] || ./configure
+
+r_make() {
+  (cd src && R CMD make -f Makevars -f "$(R RHOME)/etc/Makeconf" -f "${tmp}/warnings.mk" "$@")
+}
+
+# The object lists as src/Makevars has them, one per line.
+# Make may say other things on the way (src/deps.mk refreshes the .dd files),
+# and everything here is later passed to `rm`, so only *.o words are kept.
+objects_of() {
+  r_make -s "${@/#/print-}" | sed -n 's/^objects: //p' | tr ' ' '\n' | grep -E '^[A-Za-z0-9_./-]+\.o$' | sort -u
+}
+
+objects_of GLUE_C_SOURCES GLUE_CPP_SOURCES > "${tmp}/glue.txt"
+if [ "${scope}" = glue ]; then
+  objects="$(cat "${tmp}/glue.txt")"
+else
+  objects="$(objects_of OBJECTS | grep -vxF -f "${tmp}/glue.txt")"
+fi
+if [ -z "${objects}" ]; then
+  echo "No ${scope} objects in src/Makevars; nothing was judged."
+  exit 1
+fi
+
+# Removed first, so that every object is compiled and so speaks;
+# ccache replays a cached compile's warnings, so a warm cache hides nothing.
+# shellcheck disable=SC2086
+(cd src && rm -f ${objects})
+# shellcheck disable=SC2086
+r_make -j"$(nproc)" ${objects} > "${tmp}/build.log" 2>&1 || {
+  cat "${tmp}/build.log"
+  echo "The compile failed; see above."
+  exit 1
+}
+
+# One line per distinct warning, paths relative to src/ where they are ours.
+# R CMD check itself exempts ISO C's function pointer conversions
+# (tools:::.check_packages), and the igraph API stores R closures behind
+# igraph_function_pointer_t, so the glue cannot avoid them; they are dropped.
+grep -E '^[^ ].*:[0-9]+:[0-9]+: warning:' "${tmp}/build.log" |
+  sed -e "s|^${root}/src/||" -e "s|^${root}/||" -e 's|^src/||' |
+  grep -vE 'warning: *ISO C forbids.*function pointer' |
+  sort -u > "${tmp}/warnings.txt" || true
+
+# vendored: src/vendor/, including the lexers whose #line points back there;
+# external: anything outside the checkout (R's headers, cpp11's, the system's);
+# glue: the rest of src/.
+awk -F: '
+  { s = ($1 ~ /(^|\/)vendor\//) ? "vendored" : ($1 ~ /^\//) ? "external" : "glue"; print s "\t" $0 }
+' "${tmp}/warnings.txt" > "${tmp}/scoped.txt"
+
+fortran="$(grep -c '^Warning:' "${tmp}/build.log" || true)"
+
+echo "Compiled $(echo "${objects}" | wc -w) objects with ${WARNINGS}."
+for s in glue vendored external; do
+  echo "  $(grep -c "^${s}	" "${tmp}/scoped.txt" || true) warnings in ${s} files"
+done
+[ "${scope}" = vendored ] && echo "  ${fortran} Fortran warnings (vendored ARPACK)"
+
+if [ "${scope}" = vendored ]; then
+  cut -f2- "${tmp}/scoped.txt"
+  exit 0
+fi
+
+if grep -q '^glue	' "${tmp}/scoped.txt"; then
+  echo
+  echo "The glue must compile without warnings (handbook/build/warnings/README.md):"
+  grep '^glue	' "${tmp}/scoped.txt" | cut -f2-
+  echo
+  echo "Context from the compiler:"
+  cat "${tmp}/build.log"
+  exit 1
+fi
+echo "The glue compiles without warnings."

src/rinterface.c

@@ -9783,7 +9783,7 @@ SEXP R_igraph_maximal_cliques_subset(SEXP graph, SEXP subset, SEXP outfile, SEXP
   igraph_vector_int_t c_subset;
   igraph_vector_int_list_t c_res;
   igraph_integer_t c_no;
-  FILE* c_outfile;
+  FILE* c_outfile = 0;
   igraph_integer_t c_min_size;
   igraph_integer_t c_max_size;
   SEXP res;
@@ -13992,7 +13992,7 @@ SEXP R_igraph_write_graph_gml(SEXP graph, SEXP outstream, SEXP options, SEXP id,
   FILE* c_outstream;
   igraph_write_gml_sw_t c_options;
   igraph_vector_t c_id;
-  const char* c_creator;
+  const char* c_creator = 0;
 
                                         /* Convert input */
   Rz_SEXP_to_igraph(graph, &c_graph);
@@ -18085,7 +18085,7 @@ SEXP R_igraph_fundamental_cycles(SEXP graph, SEXP start, SEXP bfs_cutoff, SEXP w
                                         /* Declarations */
   igraph_t c_graph;
   igraph_vector_int_list_t c_basis;
-  igraph_integer_t c_start;
+  igraph_integer_t c_start = 0;
   igraph_integer_t c_bfs_cutoff;
   igraph_vector_t c_weights;
   SEXP basis;
@@ -18437,7 +18437,7 @@ SEXP R_igraph_random_spanning_tree(SEXP graph, SEXP vid) {
                                         /* Declarations */
   igraph_t c_graph;
   igraph_vector_int_t c_res;
-  igraph_integer_t c_vid;
+  igraph_integer_t c_vid = 0;
   SEXP res;
 
   SEXP r_result;
@@ -18565,9 +18565,8 @@ SEXP R_igraph_is_bipartite_coloring(SEXP graph, SEXP types) {
   igraph_bool_t c_res;
 
   SEXP res;
-  SEXP mode;
 
-  SEXP r_result, r_names;
+  SEXP r_result;
                                         /* Convert input */
   Rz_SEXP_to_igraph(graph, &c_graph);
   Rz_SEXP_to_vector_bool(types, &c_types);
@@ -18954,7 +18953,7 @@ SEXP R_igraph_invalidate_cache(SEXP graph) {
 SEXP R_igraph_vertex_path_from_edge_path(SEXP graph, SEXP start, SEXP edge_path, SEXP mode) {
                                         /* Declarations */
   igraph_t c_graph;
-  igraph_integer_t c_start;
+  igraph_integer_t c_start = 0;
   igraph_vector_int_t c_edge_path;
   igraph_vector_int_t c_vertex_path;
   igraph_neimode_t c_mode;

tools/py-stimulus/src/stimulus/generators/r.py

@@ -523,26 +523,50 @@ def chunk_declaration(self, desc: FunctionDescriptor) -> str:
         def do_par(spec: ParamSpec) -> str:
             type_desc = self.get_type_descriptor(spec.type)
             try:
-                return type_desc.declare_c_variable(f"c_{spec.name}", mode=spec.mode)
+                decl = type_desc.declare_c_variable(f"c_{spec.name}", mode=spec.mode)
             except NoSuchTypeError:
                 return f"/* {spec.name} has no corresponding C type */"
 
+            # An optional input passed by value is only assigned when it is
+            # not NULL, and only read when it is not NULL either (see
+            # chunk_call()), but the compiler cannot tell that the two
+            # Rf_isNull() calls agree and warns with -Wmaybe-uninitialized.
+            call = type_desc.get("CALL", "%C%")
+            if isinstance(call, dict):
+                call = call.get(spec.mode_str, "")
+            if (
+                spec.is_optional
+                and spec.is_input
+                and not spec.is_output
+                and call == "%C%"
+                and decl.endswith(";")
+                and "=" not in decl
+            ):
+                decl = decl[:-1] + " = 0;"
+            return decl
+
+        def has_c_type(spec: ParamSpec) -> bool:
+            return self.get_type_descriptor(spec.type).get_c_type(spec.mode) is not None
+
         inout = [do_par(spec) for spec in desc.iter_parameters()]
+        # Only the outputs with a C counterpart are converted and returned
+        # (see chunk_outconv()); declaring the others leaves unused variables.
         out = [
             f"SEXP {spec.name};"
             for spec in desc.iter_parameters()
-            if spec.mode is ParamMode.OUT
+            if spec.mode is ParamMode.OUT and has_c_type(spec)
         ]
 
-        retpars = [spec.name for spec in desc.iter_output_parameters()]
+        all_retpars = list(desc.iter_output_parameters())
+        retpars = [spec for spec in all_retpars if has_c_type(spec)]
 
         return_type_desc = self.get_type_descriptor(desc.return_type)
-        if retpars or return_type_desc.name in ("VOID", "ERROR"):
+        if all_retpars or return_type_desc.name in ("VOID", "ERROR"):
             retdecl = ""
         else:
             retdecl = return_type_desc.declare_c_variable("c_result")
 
-        if len(retpars) == 0 and desc.return_type == "ERROR":
+        if len(all_retpars) == 0 and desc.return_type == "ERROR":
             res = "\n".join(inout + out + [retdecl])
         elif len(retpars) <= 1:
             res = "\n".join(inout + out + [retdecl] + ["SEXP r_result;"])