Compilation issues on macOS at CRAN repositories

[eddelbuettel]

Environment

toml++ 3.2.0 rolled forward to commit d8bb717 (six days ago) as discussed in the most recent comments in #178

Compiler:

See the overview page at https://cloud.r-project.org/web/checks/check_results_RcppTOML.html for the aggregated results. We fail on current R ("r-release") and the previous R ("r-oldrel") on the macos-x86_64 platform. If we follow the link there we get to https://cloud.r-project.org/web/checks/check_flavors.html#r-release-macos-x86_64 and it states Apple LLVM version 10.0.0 (clang-1000.10.44.4); GNU Fortran (GCC) 8.2.0

I have now been told that "Apple and LLVM clang have numbering systems which diverged years ago." but I do not have anything more

C++ standard mode:

C++17. We hardwire it https://github.com/eddelbuettel/rcpptoml/blob/master/src/Makevars#L2 and each compilation attempt gets it. The compiler on the machine may change, those macos-x86_64 machines have been held back on purpose to compile binaries for the widest possible set of macOS machines.

Target arch:

macOS

Library configuration overrides:

As per the discussion in #178 we now set -DTOML_ENABLE_FLOAT16=0 unconditinally but nothing else.

Relevant compilation flags:

None.

Describe the bug

Compilation failure. Fuller logs eg at https://www.r-project.org/nosvn/R.check/r-release-macos-x86_64/RcppTOML-00install.html and a partial quote is here.

``` clang++ -mmacosx-version-min=10.13 -std=gnu++17 -I"/Library/Frameworks/R.framework/Resources/include" -DNDEBUG -I../inst/include -DTOML_ENABLE_FLOAT16=0 -I'/Volumes/Builds/packages/high-sierra-x86_64/Rlib/4.2/Rcpp/include' -I/usr/local/include -fPIC -Wall -g -O2 -c parse.cpp -o parse.o In file included from parse.cpp:22: ../inst/include/toml++/toml.h:16:1: warning: unknown warning group '-Wctad-maybe-unsupported', ignored [-Wunknown-warning-option] TOML_DISABLE_SPAM_WARNINGS; ^ ../inst/include/toml++/impl/preprocessor.h:455:2: note: expanded from macro 'TOML_DISABLE_SPAM_WARNINGS' TOML_PRAGMA_CLANG_GE_9(diagnostic ignored "-Wctad-maybe-unsupported") \ ^ ../inst/include/toml++/impl/preprocessor.h:201:38: note: expanded from macro 'TOML_PRAGMA_CLANG_GE_9' #define TOML_PRAGMA_CLANG_GE_9(decl) TOML_PRAGMA_CLANG(decl) ^ ../inst/include/toml++/impl/preprocessor.h:196:33: note: expanded from macro 'TOML_PRAGMA_CLANG' #define TOML_PRAGMA_CLANG(decl) _Pragma(TOML_MAKE_STRING(clang decl)) ^ :3:27: note: expanded from here clang diagnostic ignored "-Wctad-maybe-unsupported" ^ In file included from parse.cpp:22: In file included from ../inst/include/toml++/toml.h:44: ../inst/include/toml++/impl/path.h:45:11: error: use of undeclared identifier '__builtin_launder' return TOML_LAUNDER(reinterpret_cast(s.bytes)); ^ ../inst/include/toml++/impl/std_new.h:13:25: note: expanded from macro 'TOML_LAUNDER' #define TOML_LAUNDER(x) __builtin_launder(x) ^ ../inst/include/toml++/impl/path.h:61:5: note: in instantiation of function template specialization 'toml::v3::path_component::get_as >' requested here get_as(value_storage_)->~basic_string(); ^ ../inst/include/toml++/impl/path.h:45:11: error: use of undeclared identifier '__builtin_launder' return TOML_LAUNDER(reinterpret_cast(s.bytes)); ^ ../inst/include/toml++/impl/std_new.h:13:25: note: expanded from macro 'TOML_LAUNDER' #define TOML_LAUNDER(x) __builtin_launder(x) ^ ../inst/include/toml++/impl/path.h:68:12: note: in instantiation of function template specialization 'toml::v3::path_component::get_as' requested here return *get_as(value_storage_); ^ ../inst/include/toml++/impl/path.h:45:11: error: use of undeclared identifier '__builtin_launder' return TOML_LAUNDER(reinterpret_cast(s.bytes)); ^ ../inst/include/toml++/impl/std_new.h:13:25: note: expanded from macro 'TOML_LAUNDER' #define TOML_LAUNDER(x) __builtin_launder(x) ^ ../inst/include/toml++/impl/path.h:158:12: note: in instantiation of function template specialization 'toml::v3::path_component::get_as' requested here return *get_as(value_storage_); ^ ../inst/include/toml++/impl/path.h:45:11: error: use of undeclared identifier '__builtin_launder' return TOML_LAUNDER(reinterpret_cast(s.bytes)); ^ ../inst/include/toml++/impl/std_new.h:13:25: note: expanded from macro 'TOML_LAUNDER' #define TOML_LAUNDER(x) __builtin_launder(x) ^ ../inst/include/toml++/impl/path.h:180:12: note: in instantiation of function template specialization 'toml::v3::path_component::get_as >' requested here return *get_as(value_storage_); ^ In file included from parse.cpp:22: In file included from ../inst/include/toml++/toml.h:51: ../inst/include/toml++/impl/table.h:54:12: error: use of undeclared identifier '__builtin_launder' return TOML_LAUNDER(reinterpret_cast(proxy_)); ^ ../inst/include/toml++/impl/std_new.h:13:25: note: expanded from macro 'TOML_LAUNDER' #define TOML_LAUNDER(x) __builtin_launder(x) ^ ../inst/include/toml++/impl/table.h:135:11: note: in instantiation of member function 'toml::v3::impl::table_iterator::get_proxy' requested here return get_proxy(); ^ ../inst/include/toml++/impl/parser.inl:3185:36: note: in instantiation of member function 'toml::v3::impl::table_iterator::operator->' requested here if (pit != parent->end() && pit->first == segment) ^ In file included from parse.cpp:22: In file included from ../inst/include/toml++/toml.h:51: ../inst/include/toml++/impl/table.h:54:12: error: use of undeclared identifier '__builtin_launder' return TOML_LAUNDER(reinterpret_cast(proxy_)); ^ ../inst/include/toml++/impl/std_new.h:13:25: note: expanded from macro 'TOML_LAUNDER' #define TOML_LAUNDER(x) __builtin_launder(x) ^ ../inst/include/toml++/impl/table.h:129:12: note: in instantiation of member function 'toml::v3::impl::table_iterator::get_proxy' requested here return *get_proxy(); ^ ../inst/include/toml++/impl/toml_formatter.inl:37:24: note: in instantiation of member function 'toml::v3::impl::table_iterator::operator*' requested here for (auto&& [k, v] : tbl) ^ 1 warning and 6 errors generated. ```

Steps to reproduce (or a small repro code sample)

You would need a mac set up for R. I can easily submit modified version.

Additional information

None. But I am CCing @s-u who has been looking after macOS builds (and more) for R for 20+ years and is the one behind the -mmacosx-version-min=10.13 choice. He lets me get away with setting it to -mmacosx-version-min=10.13 in one other package which (and maybe that matters) also uses C++17. "Maybe" that would help here too. But it is also possibly that the tests for TOML_CLANG need an accomodation. I, as a mere R package developer, have no input in what CRAN uses. It would be tremendous to get this squared away as RcppTOML with toml++ compiles swimmingly on all other platforms.

[eddelbuettel]

I should add that the R package, as shipped, using your sources, also builds perfectly fine on the macOS builds at a parallel setup provided by rOpenSci at r-universe.dev: https://eddelbuettel.r-universe.dev/RcppTOML

The grid of builds is perfectly fine and complete there (screenshot for convenience):

Let's see what light @s-u can shed on this when he has a moment.

[eddelbuettel]

One more piece of information: a friend installed the package from source without a hitch on current intel-based macOS machine. So this points again to the CRAN setup rather than to toml++ so please don't rush and let's wait til we hear from @s-u.

[marzer]

Ah, Apple clang. The black sheep of the clang family. 😅

So this points again to the CRAN setup rather than to toml++ so please don't rush and let's wait til we hear from @s-u

Alrighty :)

[s-u]

According to the R discussion this points to a bug in TOML++ treating Apple clang based clang 6 as clang 10 which it is not. Manually changing TOML_CLANG to the correct version in toml++/impl/preprocessor.h seems to fix the problem.

[eddelbuettel]

Thanks for looking into it, Simon. Can you suggest another condition Mark could or should use to adjust the value? I presume we are talking about refining this

#ifdef __clang__
#define TOML_CLANG __clang_major__
#else

to a more fine-grained setting? As I understand it / you document it the clang++ is actually version 10.0.0 so one would presumably need another define such as __APPLE__ per one usual source ? Better yet does your clang++ have a define we could hone in on? Or should I take that off Mark's shoulders and have package-level configure checks overriding his otherwise working defaults?

[eddelbuettel]

@s-u a diff would be welcome. What is "the correct version" ? A first naive trial from where just failed.

[s-u]

You'll have to ask the TOML++ team - I don't know since only they know what is the condition they are expecting. https://en.wikipedia.org/wiki/Xcode (Toolchain version history table) is a good reference listing the corresponding Apple clang and LLVM clang versions (the columns "LLVM" and "Clang version string").

[s-u]

Just to clarify - I was only trying to find out if it would work if TOML_CLANG was manually set to the actual LLVM version which is presumably what was assumed in TOML and the answer is yes. Currently, TOML_CLANG it is not set to the LLVM version, but rather the Apple version. Unfortunately, to my best knowledge Apple clang does not provide any information on the LLVM version used, so there are essentially two possible solutions: either map the Apple version to the corresponding LLVM version when setting TOML_CLANG, or (and at least one instance in the TOML sources seems to indicate such intention) write conditions based on the Apple version such as

#if (!defined(__APPLE__) && TOML_CLANG >= 10) || TOML_CLANG >= 12

which works because Apple clang 12 is based on LLVM 10. Again, TOML's team call - there seem to be several different versions checked and I don't know if they are actually clang related or run-time related or what. As it stands now, the conditions simply don't work when Apple compilers are used, because they check the wrong version.

[marzer]

Thanks for looking into this gentlemen, I'll do some investigating of my own later today and come up with something.

[marzer]

Ah, OK, so the crux of the issue is that apple clang hijacks the values for __clang_major__ and friends and pins those to something matching the corresponding XCode release, rather than the upstream version of LLVM it was based on. That's... annoying. The proposed solution remapping the TOML_CLANG version by checking defined(__APPLE__) seems sensible to me; for my purposes it only needs to go up to about (actual) LLVM 11 and then the difference doesn't matter, so it should be fairly simple. I'll try that as a tentative fix and go from there.

[marzer]

d00464a7bcce9fe9c71f5a0fc567e6f72b8e758f is a tentative fix for this, but I have no way to test it on apple clang myself @eddelbuettel so I'll have to wait and see how you go with it.

[eddelbuettel]

Thumbs up -- when I carry https://github.com/marzer/tomlplusplus/commit/d00464a7bcce9fe9c71f5a0fc567e6f72b8e758f over (along with what came before, by rsync'ing -- I can now build and test using the CRAN-setuo macos variant at rhub which should correspond to the (somewhat dated) setup @s-u uses at CRAN. (And I should iterate again that we already had no issue on current macOS as used on macos-arm, nor on current macOS with intel as seen by a friend doing a local installation from source). So I can roll with this.

One minor and final (and not urgent) wish: CRAN Repository Policy prohibits compiler diagnostic suppression so I have comment out three lines with #pragma clang diagnostic. It's not a huge deal: I have to it with the (sizeable parts of Boost I put into) BH too but if you could put another #define around those I could set that.

[marzer]

Only three lines? There's more warning suppression in the library than that, just that most of it uses the _Pragma() form. Surely those would be technically prohibited too? O_O

I see this:

Packages should not attempt to disable compiler diagnostics, nor to remove other diagnostic information such as symbols in shared objects.

That is just bad policy, because it presupposes that compiler diagnostics are always correct and infallible. In practice that is very much not the case, heh.

Oh well. Sure I can add something around those.

[eddelbuettel]

Yes I believe it is a regexp in the checker code R uses in R CMD check mode which hones in on gcc and clang uses of #pragma ... diagnostic so I am forced to do this somewhat brutish change:

edd@rob:~/git/rcpptoml(master)$ diff -ru ../tomlplusplus/include/toml++/ inst/include/toml++/
diff -ru ../tomlplusplus/include/toml++/toml.h inst/include/toml++/toml.h
--- ../tomlplusplus/include/toml++/toml.h       2023-01-29 07:10:34.151464261 -0600
+++ inst/include/toml++/toml.h  2023-01-29 07:16:11.867945604 -0600
@@ -24,12 +24,12 @@
 #pragma warning(disable : 4251) // dll exports for std lib types
 #endif
 #elif TOML_CLANG
-#pragma clang diagnostic ignored "-Wheader-hygiene"
+//#pragma clang diagnostic ignored "-Wheader-hygiene"
 #if TOML_CLANG >= 12
-#pragma clang diagnostic ignored "-Wc++20-extensions"
+//#pragma clang diagnostic ignored "-Wc++20-extensions"
 #endif
 #if TOML_CLANG == 13 && !defined(__APPLE__)
-#pragma clang diagnostic ignored "-Wreserved-identifier"
+//#pragma clang diagnostic ignored "-Wreserved-identifier"
 #endif
 #endif

edd@rob:~/git/rcpptoml(master)$ 

No biggie but a little tedious. (And I kind-of disagree with the policy, though I understand its intend, so it hurts a little each time. :wink: )

[marzer]

Hmmn actually it will probably suffice for me to just use the _Pragma() form for those too, that way you don't have to configure anything.

[eddelbuettel]

And all green at r-universe too so I will roll this up as 0.2.2. I'll close this now -- should anything 'unusual' come up again at CRAN (and I cannot test all permutations prior to uploading) I will report back.

Big thanks for the quick fix.

[marzer]

You may want to hold off on a new release - I'm about to make a new one of my own that includes a conformance fix. It'll be up later this afternoon.

[eddelbuettel]

Dang. Already shipped 0.2.2 ...

But I can ask CRAN to disregard and do a rebuild once you roll 3.2.1 or 3.3.0 or whichever it will be.

[marzer]

@eddelbuettel 3.3.0 is live now :)

here's hoping there's no more teething problems, heh 🤞🏼

[eddelbuettel]

Super -- will update, and I prompted CRAN so I hope we can base 0.2.2 on it. Will work on it in a bit.

[s-u]

@marzer cool, that's a nice fix. It's a useful piece of code to have around, I'm sure you're not the only one dealing with this. The latest version achieved parity with clang, so we can only hope that Apple stops changing the versions from now on ...

[eddelbuettel]

And we're all set. toml++ 3.3.0 is on CRAN as part of RcppTOML 0.2.2.

Thanks to everybody for the prompt and on-point help!

[eddelbuettel]

@s-u your two machines are lagging a little (along with one machine in Vienna). If you can give them a little nudge we can fill this in and confirm that all system are green (including the macOS one setting 10.13 as a build env):

[s-u]

"Patience you must have" ;)

The package has passed the last nightly build:

$ grep -E '(^Status|package.*version)' 4.2/RcppTOML.Rcheck/00check.log 
* this is package ‘RcppTOML’ version ‘0.2.2’
Status: OK

There are several cron-based rsyncs involved between the build machine, report server and CRAN, so it will get there in due time.