Bug 276738 - clang: static_assert conflicts with -std=c++98 -pedantic-errors
Summary: clang: static_assert conflicts with -std=c++98 -pedantic-errors
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 14.0-STABLE
Hardware: Any Any
: --- Affects Only Me
Assignee: Warner Losh
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-01-30 19:43 UTC by Henri Menke
Modified: 2024-04-01 17:21 UTC (History)
6 users (show)

See Also:


Attachments
Dockerfile (668 bytes, text/plain)
2024-01-31 08:05 UTC, Henri Menke
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Henri Menke 2024-01-30 19:43:33 UTC
Clang cannot compile the following file

#include <stdint.h>
#include <algorithm>

with the command

c++ -std=c++98 -pedantic-errors -c test.cpp

with an error similar to

In file included from /usr/include/c++/v1/algorithm:1712:
In file included from /usr/include/c++/v1/memory:842:
In file included from /usr/include/c++/v1/__algorithm/move.h:12:
/usr/include/c++/v1/__algorithm/iterator_operations.h:101:9: error: too many arguments provided to function-like macro invocation
        "It looks like your iterator's `iterator_traits<It>::reference` does not match the return type of "
        ^
/usr/include/sys/cdefs.h:284:9: note: macro '_Static_assert' defined here
#define _Static_assert(x, y)    __Static_assert(x, __COUNTER__)
        ^
Comment 1 Mark Millard 2024-01-31 01:38:54 UTC
static_assert was added to C++ in C++ 11: after C++ 98.

The /usr/include/c++/v1/__algorithm/iterator_operations.h code:

  template <class _Iter>
  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14
  static void __validate_iter_reference() {
    static_assert(is_same<__deref_t<_Iter>, typename iterator_traits<__remove_cvref_t<_Iter> >::reference>::value,
        "It looks like your iterator's `iterator_traits<It>::reference` does not match the return type of "
        "dereferencing the iterator, i.e., calling `*it`. This is undefined behavior according to [input.iterators] "
        "and can lead to dangling reference issues at runtime, so we are flagging this.");
  }

is not compliant with the C++98 standard. But . . .

https://libcxx.llvm.org reports:

QUOTE
libc++ is a new implementation of the C++ standard library, targeting C++11 and above.

Features and Goals
Correctness as defined by the C++11 standard.
END QUOTE

So: Not a bug. FreeBSD gave up on C++98 when it decided to be
based, in part, on libc++ .
Comment 2 Henri Menke 2024-01-31 08:05:51 UTC
Created attachment 248087 [details]
Dockerfile

Thanks for taking a look. While the documentation states that it targets mainly C++11 and above it doesn't say anywhere explicitly that C++98 is unsupported. In fact, there is a subpage on the extended C++03 support in libc++: https://libcxx.llvm.org/DesignDocs/ExtendedCXX03Support.html

The reason why I believe that this is a bug in FreeBSD, is that using upstream clang-17 with the exact same example on a Linux system does not produce the error (see Dockerfile attached).

Finally I'd like to point out that this problem has been reported on other issue trackers as well:
https://github.com/google/benchmark/issues/1593
https://github.com/Mbed-TLS/mbedtls/issues/3693
https://stackoverflow.com/questions/70848614/getting-static-assert-is-a-c11-specific-feature-with-std-c99-on-freebsd

However, if you decide that C++98 with libc++ is unsupported on FreeBSD, please patch clang to throw an appropriate error message.
Comment 3 Henri Menke 2024-01-31 08:56:22 UTC
I have narrowed this down a bit more. The following code doesn't preprocess correctly

#include <cassert>
//#undef _Static_assert
static_assert(random_template<int, char>, "oops");

When running the preprocessor (with -E) I get

$ c++ -std=c++98 -pedantic-errors -E test.cpp >/dev/null
test.cpp:3:43: error: too many arguments provided to function-like macro invocation
    3 | static_assert(random_template<int, char>, "oops");
      |                                           ^
/usr/include/sys/cdefs.h:282:9: note: macro '_Static_assert' defined here
  282 | #define _Static_assert(x, y)    __Static_assert(x, __COUNTER__)
      |         ^
1 error generated.

If I uncomment the #undef _Static_assert it preprocesses correctly and upon compiling it gives the expected compiler error that static_assert is not available.
Comment 4 Warner Losh freebsd_committer freebsd_triage 2024-01-31 09:41:01 UTC
For C, there's certain features that we try to make available in a compatible way to old standards (eg __restrict to compile it away). I think the logic here's not right for old C++ standards.

So the trouble is that llvm's
/usr/include/c++/v1/__config
#    define static_assert(...) _Static_assert(__VA_ARGS__)
when compiling for < C++11 (at least that's my reading of it, it's a twisty maze so it may be in a branch not taken. That's the only place I could see any redirection that would be affected by the #undef in the example.

sys/cdefs.h defines _Static_assert in an old-school C way for C++ < C++11, so this feature is visible, but badly implemented for C++.

I suspect that both from reading this code and the #undef in the example means we should simply not define _Static_assert for C++ at all. The compiler will either implement it or not, depending on its wishes and give an error if not. assert.h already tries to do the right thing by not defining it for C++ at all.
Comment 5 Mark Millard 2024-01-31 19:22:07 UTC
(In reply to Henri Menke from comment #2)

My notes on interpreting this:

"it doesn't say anywhere explicitly that C++98 is unsupported.":
listing everything not supported is highly unusual and would be
a big effort. Normally if support is not mentioned, it is not
supported and what is explicitly supported is explicitly mentioned.

The lack of a page https://libcxx.llvm.org/DesignDocs/ExtendedCXX98Support.html
is also highly suggestive of lack of C++98 support.

Even https://libcxx.llvm.org/DesignDocs/ExtendedCXX03Support.html says
for C++03:

QUOTE
Clang provides a large subset of C++11 in C++03 as an extension.
END QUOTE

(Again, an explicit mention of the partial support.)

But C++03 is after C++ 98 as well and that note does not imply
anythign for the C++98 status.

I fully agree that having a error report that was in terms of
static_assert notation would be far better error handling than
what now happens. But an error report of some kind should be
what happens as far as I can tell.

Being C++ backwards compatible is messier than for C generally.
It is unlikely FreeBSD would taken on such for C++ subjects where
upstream does not. Quality of error messages where FreeBSD code
contributes to why the message is odd is something FreeBSD is
more likely to deal with.
Comment 6 Mark Johnston freebsd_committer freebsd_triage 2024-02-09 19:28:06 UTC
Warner said he'd take a closer look.
Comment 7 Dimitry Andric freebsd_committer freebsd_triage 2024-02-09 19:48:52 UTC
It looks a lot like our cdefs.h figures that in pedantic mode, the compiler does not support static_assert as a builtin, so it chooses the macro version instead. 

And for some reason the macro implementation is not compatible with the way it is used in libc++ headers, namely with two arguments.

I haven't looked too deeply into this, but I think that with -pedantic you get what you asked for. :)  In fact, the whole program is pedantically invalid if you don't use C++11 or later!
Comment 8 Basil Zubko 2024-04-01 17:21:07 UTC
(In reply to Warner Losh from comment #4)
>means we should simply not define _Static_assert for C++ at all.
Tried some invasive and dirty sanity test of this hypothesis in the 15-CURRENT, by commenting out all of the relevant static assert section in cdefs.h (lines 237-251):

/*
#if !__has_extension(c_static_assert)
#if (defined(__cplusplus) && __cplusplus >= 201103L) || \
    __has_extension(cxx_static_assert)
#define	_Static_assert(x, y)	static_assert(x, y)
#elif __GNUC_PREREQ__(4,6) && !defined(__cplusplus)
/* Nothing, gcc 4.6 and higher has _Static_assert built-in */
#elif defined(__COUNTER__)
#define	_Static_assert(x, y)	__Static_assert(x, __COUNTER__)
#define	__Static_assert(x, y)	___Static_assert(x, y)
#define	___Static_assert(x, y)	typedef char __assert_ ## y[(x) ? 1 : -1] \
				__unused
#else
#define	_Static_assert(x, y)	struct __hack
#endif
#endif
*/

It does seem to work indeed, at least in the case of the bug, that redirected me here in the first place: https://github.com/google/benchmark/issues/1593
The benchmark seems to be building and passing the test fine in this scenario.

However, the change appears to be somewhat breaking - I have some anecdotal evidence (general feeling, nothing scientific atm) that the compilation of the C++ code (LLVM and such) got slower to some extent after this change, and some apps I tested started crashing (KDE/kwin?) or behaving erratically. Just giving an initial glimpse here, for someone more familiar with the topic to chime in.