aaron.ballman added a comment.
Herald added a project: All.
Thanks for working on this! It's worth noting that
http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2935.pdf was what was adopted
by WG14 a few weeks ago, so you should be checking against that one, not the
initial version of the paper.
It's also worth noting that your changes are partially covering
http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2934.pdf which was adopted at
the same meeting (setting `Opts.Bool` to true means you get `bool` and `true`
and `false`); I would note that in the commit message, because I don't think
it's worth trying to separate out `bool`, `true`, and `false` from one another
in the implementation. But it is worth adding the deprecation warning to use of
`_Bool` from that paper and removing the macro for `bool` in C2x mode from
`<stdbool.h>` as part of these changes in this patch.
I think you're missing some test coverage for the preprocessor in the patch.
e.g.,
#if true != 1
#error "oops"
#endif
#if false != 0
#error "oops"
#endif
And I don't see the changes to the <stdbool.h> header which is now obsolete in
C2x.
Oh, and the change should be added to the release notes.
================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3247
// OpenCL and C++ both have bool, true, false keywords.
+ Opts.Bool = Opts.OpenCL || Opts.CPlusPlus || Opts.C2x;
----------------
Comment is now stale.
================
Comment at: clang/test/Sema/c2x-bool.c:1
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c2x -x c -fsyntax-only
-verify %s
+
----------------
Is the triple necessary?
================
Comment at: clang/test/Sema/c2x-bool.c:14
+_Static_assert(false == (bool)+0);
+
+
----------------
You should also add a test to verify that the predefined constants are usable
as an integer constant expression:
```
char c1[true]; // good
_Static_assert(sizeof(c1) == sizeof(char[1]));
char c2[false]; // zero-sized array declaration
```
(This covers the changes in 6.6.)
================
Comment at: clang/test/Sema/c2x-bool.c:15
+
+
+static void *f = false; // expected-warning {{to null from a constant boolean
expression}}
----------------
Another test case that's needed is for the integer promotion rules:
```
_Static_assert(_Generic(+true, bool : 0, unsigned int : 0, signed int : 1,
default : 0));
struct S {
bool b : 1;
} s;
_Static_assert(_Generic(+s.b, bool : 0, unsigned int : 0, signed int : 1,
default : 0));
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120244/new/
https://reviews.llvm.org/D120244
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
