Ease bindings to other languages providing internal interface for relaying messages and errors and taking care of sanitizers issues by astamm · Pull Request #608 · stevengj/nlopt

astamm · 2025-04-17T20:48:50Z

This is useful for providing nlopt to the R community while satisfying CRAN requirements.

Specifically, CMakeLists.txt gains the option USE_R_COMPATIBILITY:

option (USE_R_COMPATIBILITY "compile with R compatibility functions only" OFF)

which, when turned ON, triggers the following definition:

if (USE_R_COMPATIBILITY)
  add_definitions(-DCRAN_COMPATIBILITY)
endif ()

The variable CRAN_COMPATIBILITY is then used in source files to prevent the use of functions forbidden by CRAN such as stderr, abort, printf.

This PR provides the necessary fixes for the C API. There are also known uses of std::cerr and std::cout notably in the STOGO implementation that could also be avoided using CRAN_COMPATIBILITY. It might later be good to make also the C++ API R-safe.

…ions.

astamm · 2025-04-17T20:50:06Z

@aadler I noticed you forked nlopt as well last month. Just pinging you into this PR in case you had the same idea.

astamm · 2025-04-17T21:20:31Z

This PR should address #604.

…ling.

astamm · 2025-04-22T14:41:06Z

Last commit brings a new FindR.cmake file to properly set notably R_INCLUDE_DIR so that, instead of disabling verbosity and error handling, we can replace the R-incompatible functions with their R-compatible counterparts (e.g. Rprintf(), REprintf(), Rf_error()).

astamm · 2025-04-22T14:46:55Z

So instead of a simple flag option of the type NLOPT_R = ON/OFF, the CMakeLists.txt now gains a variable NLOPT_R_BINDIR which is empty by default:

set (NLOPT_R_BINDIR "" CACHE STRING "Directory containing the R executable")

When set to the directory containing the R executable, it triggers:

if (NOT "${NLOPT_R_BINDIR}" STREQUAL "")
  find_package(R REQUIRED)
  target_include_directories(${nlopt_lib} PRIVATE ${R_INCLUDE_DIR})
  target_compile_definitions(${nlopt_lib} PRIVATE NLOPT_R)
endif ()

which makes all headers from R's C API available to nlopt code and defines NLOPT_R which can be used to switch from e.g. printf() to Rprintf() in the source code.

aitap

I understand that the perfect is the enemy of the good, but since it seems that nlopt needs to go over its [f]printfs anyway, wouldn't it be better served by a layer of indirection? The optimisation methods would call an internal interface nlopt_log(opt, "format", args...), and the caller of the library would supply appropriate callbacks (such as Rprintf) to make sure that the user sees the output. The defaults for the callback (printf) could be disabled at compile time. This would avoid duplication. (Imagine if another interface needs to introduce an #ifndef NLOPT_SOME_OTHER_LANGUAGE in the same place for the same reason.)

Calling R[E]printf is not exactly safe, by the way. For example, on Rgui.exe, this checks for interrupts and would longjmp() out of the calling frame if an interrupt is found. This will at least leak memory, or wreak complete havoc if code ever becomes multi-threaded.

aitap · 2025-04-28T06:37:59Z

+  find_package(R REQUIRED)
+  target_include_directories(${nlopt_lib} PRIVATE ${R_INCLUDE_DIR})
+  target_compile_definitions(${nlopt_lib} PRIVATE NLOPT_R)
+endif ()


Will this link on Windows without an explicit reference to R.dll?

I will double check but I am pretty sure I checked against win-builder machines. I will do it again to be sure.

aitap · 2025-04-28T06:40:59Z

+else()
+set(R_PATH_LOC "${R_HOME_TEXT}/lib")
+MESSAGE(STATUS "R_PATH_LOC = ${R_PATH_LOC}")
+find_library(


At least on Linux, R defaults to --disable-R-shlib, so there won't be a libR.so in R.home('lib'). (But a distro-built R will usually be compiled with --enable-R-shlib because distros have a strong preference for dynamic linking.) This won't prevent you from linking and loading a shared library (because the dynamic loader will resolve the symbols from the ones exported by the R executable), but any example applications will fail to link.

Thanks for this feedback. Do you know how I could improve the FindR.cmake file to circumvent this issue?

aitap · 2025-04-28T06:43:38Z

+#ifndef NLOPT_R
 #define ASRT(c) if (!(c)) { fprintf(stderr, "DIRECT assertion failure at " __FILE__ ":%d -- " #c "\n", __LINE__); exit(EXIT_FAILURE); }
+#else
+#define ASRT(c) if (!(c)) { Rf_error("DIRECT assertion failure at " __FILE__ ":%d -- " #c "\n", __LINE__); }


Rf_error would longjmp out of the call frame, bypassing any destructors owned by nlopt and your own destructor. Could there be a way to return an error code from the function instead of this? Still, leaking some memory is better than crashing the process.

Originally I wanted to avoid the need to call for Rcpp or cpp11 functions within nlopt codebase because this would make a forced choice for R users to use one of them.

But I think it could make sense to force the use of cpp11 as I recall they carefully dealt with longjmps.

aitap · 2025-04-28T06:46:48Z

    extern const char *nlopt_set_errmsg(nlopt_opt opt, const char *format, ...)
 #ifdef __GNUC__
+#ifndef NLOPT_R
        __attribute__ ((format(printf, 2, 3)))


Are you sure this is needed? R CMD check goes through the names of the imports of the package shared library and compares them to entries of tools:::so_symbol_names_table, it doesn't look at the attributes.

Probably not needed indeed.

astamm · 2025-08-05T10:23:03Z

I understand that the perfect is the enemy of the good, but since it seems that nlopt needs to go over its [f]printfs anyway, wouldn't it be better served by a layer of indirection? The optimisation methods would call an internal interface nlopt_log(opt, "format", args...), and the caller of the library would supply appropriate callbacks (such as Rprintf) to make sure that the user sees the output. The defaults for the callback (printf) could be disabled at compile time. This would avoid duplication. (Imagine if another interface needs to introduce an #ifndef NLOPT_SOME_OTHER_LANGUAGE in the same place for the same reason.)

I agree with you. However, there are already existing bindings for many languages. I do not know how these have been achieved in the past. A reflection on this together with a layer of indirection to accommodate any past and future languages (maybe Julia aficionados) could be a long-term project on which we could work as a small group.

On a shorter term, nloptr does not pass CRAN checks anymore because in the case of it being built from included sources (which are the latest 2.10), CRAN now checks for calls to these functions (cerr, cout and co) and issues warning if these are used. In order to keep the included sources aligned with the main branch of nlopt, could we consider acceptable the currently proposed solution since it does not alter anyone's code unless (s)he specifically turn USE_R_COMPATIBILITY on?

Of course, I will answer the questions you raised in your review first.

Calling R[E]printf is not exactly safe, by the way. For example, on Rgui.exe, this checks for interrupts and would longjmp() out of the calling frame if an interrupt is found. This will at least leak memory, or wreak complete havoc if code ever becomes multi-threaded.

You are right indeed. I think posit folks have come up in their cpp11 package with ways to deal with this. I will investigate.

… avoid further source code modification when a new language wants to provide a binding.

…to downstream language the way output and error messages should be delayed or by fixing the code base directly to use C functions that are compatible with CRAN rules.

astamm · 2026-04-07T21:48:34Z

@aitap is this something more along the lines of what you had in mind? And to provide an example with the R language, I updated the sandbox package {nloptrbundled}: https://github.com/astamm/nloptrbundled.

Now there is no mention of R and the message and error relays are handled by downstream languages which seek to provide a binding.

The R package example passes all CRAN checks on windows, Mac and linux successfully.
The nlopt code base is not polluted with #ifdef everywhere, and it should ease bindings for other languages.

Let me know if this can be considered for the main branch.

…ed behaviors spotted by sanitizer.

…eger.

stevengj · 2026-04-09T17:30:32Z

-     double rho, *lambda, *mu;
-     double *restmp, *gradtmp;
-     nlopt_stopping *stop;
+  nlopt_func f;


All of these extraneous formatting and whitespace changes make this PR very difficult to review.

Oh my. So sorry about that. Automatic formatting in my IDE that I forgot to turn off. I can try to revert that.

I can't seem to separate well functional changes from formatting ones. What are the linting rules for nlopt? 4 spaces, no whitespaces, curly open braces on new line?

I see different conventions in different source files. Am I wrong? If we can agree on conventions, I can reformat everything accordingly.

Something like that should match the convention used in most files:

--- # Clang-format configuration derived from the observed style in bobyqa.c and # other NLopt C source files under src/libs/. # # Key characteristics: # - 4-space indentation (spaces in function bodies; tabs only for parameter- # list continuation alignment are left as-is via UseTab: ForIndentation) # - Linux brace style: function-definition opening brace on its own line, # control-flow opening brace on the same line # - No column-limit enforcement (f2c-generated code has very long lines) # - Pointer qualifier attached to the variable name (double *x) # - No include sorting (includes are grouped intentionally) Language: Cpp BasedOnStyle: LLVM # Indentation IndentWidth: 4 TabWidth: 8 UseTab: ForIndentation ContinuationIndentWidth: 4 # Braces — Linux style: function body brace on new line, rest on same line BreakBeforeBraces: Linux # Pointer / reference qualifier attached to the variable name PointerAlignment: Right # Line length — disabled; legacy f2c code has lines well beyond 80 columns ColumnLimit: 0 # Short constructs AllowShortFunctionsOnASingleLine: None AllowShortIfStatementsOnASingleLine: Never AllowShortLoopsOnASingleLine: false AllowShortBlocksOnASingleLine: Empty # Alignment — keep off to avoid churning legacy code AlignConsecutiveAssignments: None AlignConsecutiveDeclarations: None AlignTrailingComments: Kind: Never # Spaces SpaceAfterCStyleCast: false SpaceBeforeParens: ControlStatements SpaceInEmptyParentheses: false # Includes — do not sort; order is intentional in NLopt sources SortIncludes: Never IncludeBlocks: Preserve

Should I apply this everywhere? Would that suit you?

It can be stored in a .clang-format hidden file at the root of the project so that we always follow these conventions. Let me know if this is something you would consider.

However, whitespaces after commas are automatically (in the sense hardcoded) emitted by clang formatter.

Also, the formatting style differs from the one adopted by the f2c tool that has been used for translating Fortran code. Do you have strong formatting rules for the nlopt project? Defining such rules in a file like .clang-format as I propose would avoid future formatting issues we are facing here.

Here's what can be tried to avoid digging the problem even deeper:

Create a new branch from git merge-base master r-compatibility, let's call it master-formatted

Format it using the closest clang-format settings you can find

Format r-compatibility using the same settings

Compute the diff from master-formatted to formatted r-compatibility

Apply it on top of the original git merge-base master r-compatibility, solve the few remaining conflicts, restore the original code style, split into logical commits

This is probably easier than disentangling the formatting changes from semantic changes in the current branch.

On it, thanks for the tips!

Once found closest clang-format settings, don't we want to stick with it?
Do you already have strong formatting conventions for nlopt?

That's not for me to decide. I'm sorry I did not make it clear enough that I'm only giving advice (motivated by what I think is best for NLopt and the R community). In my experience, PRs that reformat the entire code base don't go well.

stevengj · 2026-04-10T11:44:35Z

Don't use clang-format or do any wholesale reformatting. (It could be quite reasonable to reformat nlopt using clang-format in a separate PR.) Right now it's not any consistent formatting for historical reasons, but addressing that is orthogonal to the current PR.

For this PR, use a different IDE if you have to — get an editor where you can make localized changes without doing wholesale reformatting. I'm simply not going to a merge a PR that includes 10,000 lines of extraneous formatting changes.

astamm · 2026-04-10T12:29:37Z

On it, on it.

astamm · 2026-04-10T16:38:58Z

We should be good now.

astamm · 2026-04-10T16:49:54Z

Btw, this PR also addresses #604 #611 #612 that I filed early on.

stevengj · 2026-04-10T17:13:20Z

+    {
+        double *volatile _v = w;
+        w = _v - 1;
+    }


This change seems unrelated to the topic of the PR?

You'll find this pattern repeatedly. It fixes GCC undefined behavior sanitizer issues that were spotted when I copy-pasted nlopt sources in the nloptrbundled package so I provided the fix. I believe it should improve compatibility with any downstream language that would like to provide a binding of nlopt. Hence, its presence here. The name of the PR though could be changed to better reflect what the provided solution actually does wrt the original intent.

astamm · 2026-04-20T13:35:05Z

Please let me know if I can do something else to get this PR through. It would be really helpful for downstream packages in other languages that provide bindings to nlopt. Thanks!

astamm · 2026-05-12T11:30:53Z

Can I do more to get this PR going on? Please let me know @stevengj. Thanks a lot!

astamm · 2026-06-21T21:36:12Z

Any update on this? What else can I do to move this PR forward please? I'd like to get a new release of nloptr out soon on CRAN with this improvement. Thanks.

Add cmake variable to enable compilation without R-incompatible funct…

a2a9a46

…ions.

astamm mentioned this pull request Apr 18, 2025

New warning about entry points that might terminate R in compiled code in R-devel astamm/nloptr#198

Open

astamm added 2 commits April 19, 2025 23:07

Fix remaining calls to printf.

98b762a

Added FindR.cmake to properly use R C API for printing and error hand…

09a49c6

…ling.

astamm mentioned this pull request Apr 23, 2025

[COBYLA/BOBYQA] Address sanitizer issue #611

Open

aitap reviewed Apr 28, 2025

View reviewed changes

astamm mentioned this pull request May 12, 2025

Don't leak local_opts astamm/nloptr#202

Closed

astamm added 3 commits April 7, 2026 17:24

Merge remote-tracking branch 'upstream/master' into r-compatibility

9514a6b

Relay diagnostic message output from algs via a callback mechanism to…

98c4986

… avoid further source code modification when a new language wants to provide a binding.

Remove the need to discover R install altogether either by deferring …

83c6463

…to downstream language the way output and error messages should be delayed or by fixing the code base directly to use C functions that are compatible with CRAN rules.

astamm added 3 commits April 8, 2026 16:39

Mainly use volatile pointer intermediate pattern to deal with undefin…

5a4dfa1

…ed behaviors spotted by sanitizer.

Fix warning in DIRECT due to initialized, assigned but never read int…

7d9ed55

…eger.

Change NOTE to WARNING in comments.

13d74cc

stevengj reviewed Apr 9, 2026

View reviewed changes

Should revert most code-style-only changes.

0e5493d

stevengj reviewed Apr 10, 2026

View reviewed changes

astamm changed the title ~~Add cmake variable to enable compilation without R-incompatible functions~~ Ease bindings to other languages providing callbacks for relaying messages and errors and taking caring of sanitizers issues Apr 11, 2026

astamm changed the title ~~Ease bindings to other languages providing callbacks for relaying messages and errors and taking caring of sanitizers issues~~ Ease bindings to other languages providing callbacks for relaying messages and errors and taking care of sanitizers issues Apr 11, 2026

astamm changed the title ~~Ease bindings to other languages providing callbacks for relaying messages and errors and taking care of sanitizers issues~~ Ease bindings to other languages providing internal interface for relaying messages and errors and taking care of sanitizers issues Apr 12, 2026

astamm mentioned this pull request Jul 1, 2026

Weaken test to improve flakiness astamm/nloptr#213

Open

Conversation

astamm commented Apr 17, 2025

Uh oh!

astamm commented Apr 17, 2025

Uh oh!

astamm commented Apr 17, 2025

Uh oh!

astamm commented Apr 22, 2025

Uh oh!

astamm commented Apr 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aitap left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

astamm commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

astamm commented Apr 7, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stevengj commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

astamm commented Apr 10, 2026

Uh oh!

astamm commented Apr 10, 2026

Uh oh!

astamm commented Apr 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

astamm commented Apr 20, 2026

Uh oh!

astamm commented May 12, 2026

Uh oh!

astamm commented Jun 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

astamm commented Apr 22, 2025 •

edited

Loading

astamm commented Aug 5, 2025 •

edited

Loading

stevengj commented Apr 10, 2026 •

edited

Loading