Regression(?): isfinite() is leaked to global namespace

[juj]

Previously, we have been using LLVM 15 to compile to WebAssembly via Emscripten 3.1.38.

Attempting to update to newer Emscripten 4.0.19, which carries an update to LLVM 20.1.4 libcxx, the following code, which used to compile before with the LLVM 15 branch, no longer does:

a.cpp

#include <cmath>

// C23: isfinite() is a macro, so undef it from existence
#ifdef isfinite
#undef isfinite
#endif

namespace math {
  int isfinite(float x) {
    return ((unsigned int)x & 0x7F800000u) != 0x7F800000u;
  }
}

int main() {
  using namespace math;
  isfinite(1.f);
}

This produces an error

a.cpp:16:3: error: call to 'isfinite' is ambiguous
   16 |   isfinite(1.f);
      |   ^~~~~~~~
C:\emsdk\emscripten\main\cache\sysroot/include/c++/v1\__math/traits.h:71:83: note: candidate function
   71 | [[__nodiscard__]] inline _LIBCPP_CONSTEXPR_SINCE_CXX23 _LIBCPP_HIDE_FROM_ABI bool isfinite(float __x) _NOEXCEPT {
      |                                                                                   ^
a.cpp:9:7: note: candidate function
    9 |   int isfinite(float x) {
      |       ^
1 error generated.

It looks like the above code stopped compiling after this commit: 7f2bd53

I am not 100% language spec lawyer here.. Is the program a.cpp malformed and the compilation error allowed by the spec?

The C++ standard does not state that a function isfinite() should exist in global scope, but only a std::isfinite() namespaced version should exist? (https://isocpp.org/files/papers/N4860.pdf page 1215 has bool std::isfinite(float x);)

The C23 standard talks about a isfinite() macro (although gives its declaration in a pseudo-function declaration form)

so the above code should not conflict on a global function from the C standard?

Hence, it does seem to me like it would be a regression bug that the above code a.cpp no longer compiles?

[juj]

It looks like the above code stopped compiling after this commit: 7f2bd53

Actually, it might not have been that commit. That commit seems to have added a std::__math::isfinite().

But rather, it seems to be these code lines:

llvm-project/libcxx/include/math.h

Lines 413 to 420 in c6f501d

	_LIBCPP_END_NAMESPACE_STD
	using std::__math::fpclassify;
	using std::__math::signbit;
	// The MSVC runtime already provides these functions as templates
	# ifndef _LIBCPP_MSVCRT
	using std::__math::isfinite;

that bring an isfinite function into the global scope outside the std namespace.

That was brought on in this commit: 6553608#diff-17340ba58474e9634bb16b0f501b6fe9da2308198430c29d717abb44f4a01b04R409

Should the using std::__math::isfinite; statement have been before the _LIBCPP_END_NAMESPACE_STD line?

[frederick-vs-ja]

Hence, it does seem to me like it would be a regression bug that the above code a.cpp no longer compiles?

I think the a.cpp is never guaranteed to compile (at least since C++11). Per [support.c.headers.other]/1 and [cmath.syn], C++'s <math.h> does place the isfinite function(s) in the global namespace. And I believe [headers]/5 allows <cmath> to also provide isfinite in the global namespace.

Also, you need to use std::bit_cast (or something like, including the underlying __builtin_bit_cast intrinsic) instead of the cast in your isfinite.

[juj]

Thank you so much for the references here. This is greatly appreciated.

Per [support.c.headers.other]/1 and [cmath.syn], C++'s <math.h> does place the isfinite function(s) in the global namespace.

Trying to parse the directive in the first link, condensing it to the relevant part here, it says

Every C header [...], each of which has a name of the form <name.h>, behaves as if each name placed in the standard
library namespace by the corresponding <cname> header is placed within the global namespace scope

To me, this means that if <cmath> has a name std::isfinite, then <math.h> should behave as if std::isfinite is placed in the global namespace scope.

But note that the example code in a.cpp does #include <cmath> and not #include <math.h>. The above statement does not say that the header <cmath> should be allowed to provide a global namespace isfinite()?

---

And I believe [headers]/5 allows to also provide isfinite in the global namespace.

In this link, the text says

the contents of each header cname is the same as that of the corresponding header name.h as specified in the C
standard library. In the C++ standard library, however, the declarations (**except for names which are defined as
macros in C**) are within namespace scope of the namespace std.

and

It is unspecified whether these names [...] are first declared within the global namespace scope and are then
injected into namespace std by explicit using-declaration.

The second block seems to indeed be interpretable to state that it is left open whether e.g. <cmath> would first provide global scope functions, and then pull them into namespace std via a using, or vice versa.

But, as per the screenshot in the first post, the C23 standard (and also C99 standard) states that isfinite is a macro, so it would fall under the exception noted in **except for names which are defined as macros in C** in the first block of text?

The exception here to me reads like accounting for the fact that #define macros cannot exist in a namespace.

To me it seems that in C standard, isfinite is a macro. In C++ standard, it gets turned into e.g. a std::isfinite() (template) function, but the exception in bold above does not seem to allow C++ to leak that function back into the global namespace?

[frederick-vs-ja]

But, as per the screenshot in the first post, the C23 standard (and also C99 standard) states that isfinite is a macro, so it would fall under the exception noted in **except for names which are defined as macros in C** in the first block of text?

Looks like that the current wording is somehow ambiguous. The intent seems to be that given macros don't belong to any namespace, it's impossible to make them declared in namespace std.

I've just opened cplusplus/draft#8579 for this. An LWG issue will possibly be needed.

[jwakely]

The "defined as macros" wording is only supposed to apply to e.g.

#define assert(...)

not function-like things that have to be defined as macros because C can't overload functions. This needs to be clarified in the C++ standard.

Your original example code has undefined behaviour because of the #undef, which is disallowed by [macro.names]:

A translation unit that includes a standard library header shall not #define or #undef names declared in
any standard library header.

The implementation should undef isfinite before defining the overloaded C++ versions of it.

[jwakely]

Your original example code has undefined behaviour ...

And even without the #undef it's allowed to be rejected, because <cmath> (and every <cxxx> header) is allowed to declare names in the global namespace as well as in std. So this is not a libc++ bug.