Skip to content

Unnamed raw cpp_options assignments reach make but are invisible to everything that keys on names #1250

Description

@jgabry

Unnamed raw cpp_options assignments reach make but are invisible to everything that keys on names

Found during the design review for #1238. I asked Claude to write up the report below.


cpp_options accepts unnamed entries, which are passed to make verbatim. They reach the build and affect the artifact, but they are invisible to every part of cmdstanr that reasons about options by name, because an unnamed list entry's name is "".

The rule that fixes it is #1254 §3 ("Only named assignments configure a build"): every unnamed cpp_options entry on a build call is rejected, named entries are canonicalized to their make spelling on entry, and after normalization a name must match ^[A-Za-z_][A-Za-z0-9_]*$. cmdstan_make_local() is untouched, since make/local is a file where += is real makefile syntax. Stage 1 (#1258), and a prerequisite for the record: #1254 §4's last-assignment-wins canonicalization is sound only once raw operators cannot reach make.

Reproduction

Threading, enabled through the raw spelling:

mod <- cmdstan_model(stan_file, cpp_options = list("STAN_THREADS=TRUE"))
fit <- mod$sample(threads_per_chain = 4)
#> Warning: 'threads_per_chain' is set but the model was not compiled with
#> 'cpp_options = list(stan_threads = TRUE)' so 'threads_per_chain' will have
#> no effect!

The model is threaded, but assert_valid_threads() warns that it is not. This is the user-visible symptom of #765, reachable today through a different spelling.

A user header, same shape but worse:

mod <- cmdstan_model(stan_file, cpp_options = list("USER_HEADER=my.hpp"))
reaches make as:              USER_HEADER=my.hpp     # so it IS compiled in
parsed_cpp_options():         in neither assignments nor opaque
resolve_user_header() sees:   FALSE

The header is compiled in while the entire header machinery is bypassed: no --allow-undefined, no existence check, and none of the $check_syntax() / $format() behaviour fixed in #1235.

Cause

Three accessors key on names(cpp_options), and an unnamed entry has no name: cpp_option_value() (R/cpp_opts.R:261), resolve_user_header() (:195-196) and validate_cpp_options() (:165). Meanwhile cpp_options_to_compile_flags() passes unnamed entries straight through (:139).

An adjacent defect in the same function: parsed_cpp_options() lower-cases assignment names at :100, so cpp_options_disagree(list("foo=1"), list(foo = 1)) is FALSE although named foo emits FOO=1 and raw "foo=1" emits foo=1, and Make variables are case-sensitive. The rejection moots it: every entry that survives validation is a named, canonical assignment.

Why the operators cannot be supported

A lone command-line assignment collapses to =, but two assignments to the same variable retain their operator semantics (GNU Make 3.81):

make 'FOO=x'                -> FOO=[x]       # alone: all four collapse
make 'FOO+=x'               -> FOO=[x]
make 'FOO=base' 'FOO+=x'    -> FOO=[base x]  # in combination: they do not
make 'FOO=base' 'FOO?=x'    -> FOO=[base]
make 'FOO+=x'   'FOO+=y'    -> FOO=[x y]
make 'FOO+=x'   'FOO:=y'    -> FOO=[y]

Supporting that means preserving and interpreting an ordered assignment program. -f other.mk and --eval= set variables without looking like assignments, so Make's own flags are rejected on the same terms (#1254 §3 carries the measurement).

Checklist

  • A new assert_valid_cpp_options(), called from the shared build implementation before the configuration is used, pairing with assert_valid_stanc_options() (R/model.R:2562). Normalize, then check, so USER_HEADER and STANCFLAGS are matched as literals
  • Inspect unnamed source entries before cpp_options_to_compile_flags() erases their origin; the matcher must be broader than parsed_cpp_options()'s ^[A-Za-z_][A-Za-z0-9_]*=
  • Named keys held to ^[A-Za-z_][A-Za-z0-9_]*$, or list("CXXFLAGS+" = "-march=native") reaches make as a live += (confirmed)
  • Error text specific to what was supplied (Design note: v1.0 compilation state and C++ options #1254 §3): plain NAME=value suggests list(NAME = value); += and relatives point at cmdstan_make_local(); USER_HEADER points at user_header =; a Make flag points at the makefile route (MAKEFLAGS += -j4, include other.mk, force_recompile for -B)
  • Delete validate_cpp_options() (R/cpp_opts.R:151) and its tests (test-cpp_opts.R:24-37): dead code whose one behaviour warns that a logical FALSE turns an option on, which cpp_options = list(stan_threads = FALSE) enables threading instead of disabling it #1251 reverses
  • Delete exe_info_reflects_cpp_options(): with the tolower() gone its intersection is empty and it returns TRUE unconditionally, and Design note: v1.0 compilation state and C++ options #1254 §7 makes cpp_options beside exe_file = an error so nothing reaches it
  • STAN_VERSION needs no special handling and is recorded and compared like any other named entry (Design note: v1.0 compilation state and C++ options #1254 §3)
  • Do not regress STAN_THREADS in make/local not respected due to capitalisation conflict #765: cpp_option_value() keys on the R list names, independent of parsed_cpp_options(), verified

Test matrix

  • every assignment-shaped raw entry is rejected whatever its casing: "FOO=1", "foo=1", "STAN_THREADS=TRUE", and the message names the equivalent named form
  • "USER_HEADER=h" points at the user_header argument, "STANCFLAGS=..." at stanc_options
  • list(user_header = h), list(USER_HEADER = h) and list(User_Header = h) are one rejection; test arbitrary casing
  • list("CXXFLAGS+" = "-march=native") is rejected
  • "-j4", "-f other.mk", "--eval=STAN_OPENCL=1" are rejected and the message names the makefile route
  • cpp_options_disagree(list(STAN_VERSION = "9.9"), list(STAN_VERSION = "8.8")) is TRUE
  • nothing calls exe_info_reflects_cpp_options(); cpp_options beside exe_file = errors
  • list(stan_threads = FALSE) emits the empty assignment and disables threading (cpp_options = list(stan_threads = FALSE) enables threading instead of disabling it #1251), with no warning claiming the opposite
  • cmdstan_make_local(cpp_options = list("CXXFLAGS+= -march=native")) keeps working (man/install_cmdstan.Rd:137-142)

Documentation this creates

The natural migration from list("CXXFLAGS+= -x") is list(CXXFLAGS = "-x"), and it silently discards whatever the user has in make/local. In CmdStan 2.39.0 plain CXXFLAGS += appears exactly once across the tree, in make/local itself. Worth documenting as an observation about current makefiles rather than a guarantee. #570 supported the named USER_HEADER spelling on purpose; #1254 §3 retires it too, so both forms point at the same error.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions