Re: svn commit: r322875 - head/sys/dev/nvme

[Ed Schouten]

2017-08-28 11:02 GMT+02:00 Mark Millard :
> Based on the same main.cc as before . . .
>
> g++7 -std=c++98 main.cc
> g++7 -Wpedantic -std=c++98 main.cc
> g++7 -std=c++03 main.cc
> g++7 -Wpedantic -std=c++03 main.cc
>
> no longer complain (so no error, no
> warning).

Perfect! I've committed this change as r322965. Thanks for testing!


-- 
Ed Schouten 
Nuxi, 's-Hertogenbosch, the Netherlands
KvK-nr.: 62051717
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: svn commit: r322875 - head/sys/dev/nvme

[Mark Millard]

On 2017-Aug-27, at 11:54 PM, Ed Schouten  wrote:

> 2017-08-25 14:53 GMT+02:00 Ed Schouten :
>> 2017-08-25 9:46 GMT+02:00 Mark Millard :
>>> It appears that at least 11.1-STABLE -r322807 does not handle
>>> -std=c++98 styles of use of _Static_assert for g++7 in that
>>> g++7 reports an error:
>> 
>> Maybe we need to do something like this?
>> 
>> Index: sys/sys/cdefs.h
>> ===
>> --- sys/sys/cdefs.h (revision 322887)
>> +++ sys/sys/cdefs.h (working copy)
>> @@ -294,7 +294,7 @@
>> #if (defined(__cplusplus) && __cplusplus >= 201103L) || \
>> __has_extension(cxx_static_assert)
>> #define _Static_assert(x, y) static_assert(x, y)
>> -#elif __GNUC_PREREQ__(4,6)
>> +#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__)
> 
> Could you let me know whether this patch fixes the build for you? If
> so, I'll commit it!

As a variant of stable/11 -r322807 . . .

buildworld and buildkernel seem to work fine.
(I did not try any port [re-]builds.)

Based on the same main.cc as before . . .

g++7 -std=c++98 main.cc
g++7 -Wpedantic -std=c++98 main.cc
g++7 -std=c++03 main.cc
g++7 -Wpedantic -std=c++03 main.cc

no longer complain (so no error, no
warning).

clang++ -Wpedantic -std=c++11 main.cc
clang++ -Wpedantic -std=c++98 main.cc
clang++ -Wpedantic -std=c++03 main.cc

each still give the warning but no error.

g++7 -Wpedantic -std=c++11 main.cc
g++7 -std=c++11 main.cc
clang++ -std=c++11 main.cc
clang++ -std=c++98 main.cc
clang++ -std=c++03 main.cc

are still silent, no errors, no warnings.

Note that clang here is version 4 --the
same as in my original report that had the
g++7 rejection example. This is because of
the stable/11 context that I used. (An
intended MFC had been listed.)

If needed I could probably try under some
version of head (and so test clang version
5).

===
Mark Millard
markmi at dsl-only.net

___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: svn commit: r322875 - head/sys/dev/nvme

[Ed Schouten]

Mark,

2017-08-25 14:53 GMT+02:00 Ed Schouten :
> 2017-08-25 9:46 GMT+02:00 Mark Millard :
>> It appears that at least 11.1-STABLE -r322807 does not handle
>> -std=c++98 styles of use of _Static_assert for g++7 in that
>> g++7 reports an error:
>
> Maybe we need to do something like this?
>
> Index: sys/sys/cdefs.h
> ===
> --- sys/sys/cdefs.h (revision 322887)
> +++ sys/sys/cdefs.h (working copy)
> @@ -294,7 +294,7 @@
>  #if (defined(__cplusplus) && __cplusplus >= 201103L) || \
>  __has_extension(cxx_static_assert)
>  #define _Static_assert(x, y) static_assert(x, y)
> -#elif __GNUC_PREREQ__(4,6)
> +#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__)

Could you let me know whether this patch fixes the build for you? If
so, I'll commit it!

-- 
Ed Schouten 
Nuxi, 's-Hertogenbosch, the Netherlands
KvK-nr.: 62051717
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: svn commit: r322875 - head/sys/dev/nvme

[Warner Losh]

On Fri, Aug 25, 2017 at 6:53 AM, Ed Schouten  wrote:

> 2017-08-25 9:46 GMT+02:00 Mark Millard :
> > It appears that at least 11.1-STABLE -r322807 does not handle
> > -std=c++98 styles of use of _Static_assert for g++7 in that
> > g++7 reports an error:
>
> Maybe we need to do something like this?
>
> Index: sys/sys/cdefs.h
> ===
> --- sys/sys/cdefs.h (revision 322887)
> +++ sys/sys/cdefs.h (working copy)
> @@ -294,7 +294,7 @@
>  #if (defined(__cplusplus) && __cplusplus >= 201103L) || \
>  __has_extension(cxx_static_assert)
>  #define _Static_assert(x, y) static_assert(x, y)
> -#elif __GNUC_PREREQ__(4,6)
> +#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__)



This looks good to my eye, but my level of C++ pedantic knowledge is
suboptimal.

Warner
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: svn commit: r322875 - head/sys/dev/nvme

[Ed Schouten]

2017-08-25 9:46 GMT+02:00 Mark Millard :
> It appears that at least 11.1-STABLE -r322807 does not handle
> -std=c++98 styles of use of _Static_assert for g++7 in that
> g++7 reports an error:

Maybe we need to do something like this?

Index: sys/sys/cdefs.h
===
--- sys/sys/cdefs.h (revision 322887)
+++ sys/sys/cdefs.h (working copy)
@@ -294,7 +294,7 @@
 #if (defined(__cplusplus) && __cplusplus >= 201103L) || \
 __has_extension(cxx_static_assert)
 #define _Static_assert(x, y) static_assert(x, y)
-#elif __GNUC_PREREQ__(4,6)
+#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__)


-- 
Ed Schouten 
Nuxi, 's-Hertogenbosch, the Netherlands
KvK-nr.: 62051717
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: svn commit: r322875 - head/sys/dev/nvme

[Mark Millard]

On 2017-Aug-25, at 12:14 AM, David Chisnall  wrote:

> On 25 Aug 2017, at 07:32, Mark Millard  wrote:
>> 
>> As I remember _Static_assert is from C11, not
>> the older C99.
> 
> In pre-C11 dialects of C, _Static_assert is an identifier reserved for the 
> implementation.  sys/cdefs.h defines it to generate a zero-length array if 
> the condition is true or a negative-length array if it is false, emulating 
> the behaviour (though giving less helpful error messages)
> 
>> 
>> As I understand head/sys/dev/nvme/nvme.h use by
>> C++ code could now reject attempts to use
>> _Static_assert .
> 
> In C++, _Static_assert is an identifier reserved for the implementation, but 
> in C++11 or newer static_assert is a keyword.  sys/cdefs.h defines 
> _Static_assert to static_assert for newer versions of C++ and defines it to 
> the C-before-11-compatible version for C++-before-11.
> 
> TL;DR: We have gone to a lot of effort to ensure that these keywords work in 
> all C/C++ dialects, please use them, please report bugs if you find a case 
> where they don’t work.

It appears that at least 11.1-STABLE -r322807 does not handle
-std=c++98 styles of use of _Static_assert for g++7 in that
g++7 reports an error:

# uname -apKU
FreeBSD hzFreeBSD11S 11.1-STABLE FreeBSD 11.1-STABLE  r322807  amd64 amd64 
1101501 1101501

# more main.cc
#include "/usr/include/sys/cdefs.h"
_Static_assert(1,"Test");
int main(void)
{
   return 0;
}

# g++7 -std=c++98 main.cc
main.cc:2:15: error: expected constructor, destructor, or type conversion 
before '(' token
 _Static_assert(1,"Test");
   ^

So it appears that as stands the _Static_assert
implementation requires a more modern C++ standard
vintage.


With the likes of -Wpedantic clang++ from 11.1-STABLE
-r322807 reports a warning:

# clang++ -Wpedantic -std=c++11 main.cc
main.cc:2:1: warning: _Static_assert is a C11-specific feature 
[-Wc11-extensions]
_Static_assert(1,"Test");
^
1 warning generated.

# clang++ -Wpedantic -std=c++98 main.cc
In file included from main.cc:1:
/usr/include/sys/cdefs.h:852:27: warning: variadic macros are a C99 feature 
[-Wvariadic-macros]
#define __locks_exclusive(...) \
  ^
. . . (more such macro reports) . . .
main.cc:2:1: warning: _Static_assert is a C11-specific feature 
[-Wc11-extensions]
_Static_assert(1,"Test");
^
11 warnings generated.

By contrast "g++7 -Wpedantic -std=c++11 main.cc" is silent about it.


===
Mark Millard
markmi at dsl-only.net

___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: svn commit: r322875 - head/sys/dev/nvme

[Ed Schouten]

2017-08-25 8:32 GMT+02:00 Mark Millard :
>> # g++49 main.cc
>> main.cc:2:15: error: expected constructor, destructor, or type conversion 
>> before '(' token
>>  _Static_assert(1,"Test");

Yeah, that's because GCC is such a pain in the neck compiler that it
doesn't want to expose these C11 keywords when building C++, even
though they are in the reserved namespace (_[A-Z]). GCC would be
permitted to expose these and still comply to standards. Doing so
would make things so much easier for operating system implementors,
like us. Clang does get it right, in my opinion.

We should just extend  to define _Static_assert() when
using GCC in C++ mode (if we're not doing so already).

-- 
Ed Schouten 
Nuxi, 's-Hertogenbosch, the Netherlands
KvK-nr.: 62051717
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: svn commit: r322875 - head/sys/dev/nvme

[David Chisnall]

On 25 Aug 2017, at 07:32, Mark Millard  wrote:
> 
> As I remember _Static_assert is from C11, not
> the older C99.

In pre-C11 dialects of C, _Static_assert is an identifier reserved for the 
implementation.  sys/cdefs.h defines it to generate a zero-length array if the 
condition is true or a negative-length array if it is false, emulating the 
behaviour (though giving less helpful error messages)

> 
> As I understand head/sys/dev/nvme/nvme.h use by
> C++ code could now reject attempts to use
> _Static_assert .

In C++, _Static_assert is an identifier reserved for the implementation, but in 
C++11 or newer static_assert is a keyword.  sys/cdefs.h defines _Static_assert 
to static_assert for newer versions of C++ and defines it to the 
C-before-11-compatible version for C++-before-11.

TL;DR: We have gone to a lot of effort to ensure that these keywords work in 
all C/C++ dialects, please use them, please report bugs if you find a case 
where they don’t work.

David

___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: svn commit: r322875 - head/sys/dev/nvme

[Mark Millard]

> Author: imp
> Date: Fri Aug 25 04:33:06 2017
> New Revision: 322875
> URL: 
> https://svnweb.freebsd.org/changeset/base/322875
> 
> 
> Log:
>   Use _Static_assert
>   
>   These files are compiled in userland too, so we can't use sys/systm.h
>   and rely on CTASSERT. Switch to using _Static_assert instead.
>   
>   MFC After: 3 days
>   Sponsored by: Netflix
> 
> Modified:
>   head/sys/dev/nvme/nvme.h
>   head/sys/dev/nvme/nvme_util.c

As I remember _Static_assert is from C11, not
the older C99.

As I understand head/sys/dev/nvme/nvme.h use by
C++ code could now reject attempts to use
_Static_assert .

There have been at least one old bugzilla report
for such. An example is 205453 (back around
2015-Dec).

>From back then:

> # more main.cc
> #include "/usr/include/sys/cdefs.h"
> _Static_assert(1,"Test");
> int main(void)
> {
> return 0;
> }
> 
> For example:
> 
> # g++49 main.cc
> main.cc:2:15: error: expected constructor, destructor, or type conversion 
> before '(' token
>  _Static_assert(1,"Test");
> . . .
> g++49, g++5, and powerpc64-portbld-freebsd11.0-g++ all reject the above 
> source the same way that libcxxrt/guard.cc compiles are rejected during 
> powerpc64-portbld-freebsd11.0-g++ based buildworld lib32 -m32 compiles. 
> 
> gcc49, gcc5, and powerpc64-portbld-freebsd11.0-gcc all accept the above 
> instead (when in main.c instead of main.cc so it is handle as C code), with 
> or without the include. _Static_assert is specific to C11 and is not part of 
> C++. It takes explicit definitions to make the syntax acceptable as C++.
> 
> Note: clang++ (3.7) accepts the use of the C11 _Static_assert, with or 
> without the include, going well outside the C++ language definition.
> 
> . . .
> 
> Fixed in r297299 .

(The context was a C++ file head/contrib/libcxxrt/guard.cc so C++'s
static_assert was used instead and -std=c++11 was added for the
library in question [libcxxrt].)

Unless head/sys/dev/nvme/nvme.h is not to be used from
C++ code: use of _Static_assert in the header would appear
to be a problem.

===
Mark Millard
markmi at dsl-only.net

___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"