[PATCH] D157331: [clang] Implement C23
yabinc added inline comments.
Comment at: clang/lib/Headers/stdckdint.h:13
+
+#if defined(__GNUC__)
+#define ckd_add(R, A, B) __builtin_add_overflow((A), (B), (R))
enh wrote:
> enh wrote:
> > enh wrote:
> > > ZijunZhao wrote:
> > > > enh wrote:
> > > > > is this ever _not_ set for clang?
> > > > https://github.com/llvm/llvm-project/blob/main/clang/lib/Headers/stdbool.h#L23
> > > > I think it is set?
> > > i get an error from
> > > ```
> > > /tmp$ cat x.c
> > > #if defined(__GNUC__)
> > > #error foo
> > > #endif
> > > ```
> > > regardless of whether i compile with -std=c11 or -std=gnu11.
> > > neither -ansi nor -pedantic seem to stop it either.
> > it does look like it _should_ be possible to not have it set though?
> > llvm/llvm-project/clang/lib/Frontend/InitPreprocessor.cpp has:
> > ```
> > if (LangOpts.GNUCVersion != 0) {
> > // Major, minor, patch, are given two decimal places each, so 4.2.1
> > becomes
> > // 40201.
> > unsigned GNUCMajor = LangOpts.GNUCVersion / 100 / 100;
> > unsigned GNUCMinor = LangOpts.GNUCVersion / 100 % 100;
> > unsigned GNUCPatch = LangOpts.GNUCVersion % 100;
> > Builder.defineMacro("__GNUC__", Twine(GNUCMajor));
> > Builder.defineMacro("__GNUC_MINOR__", Twine(GNUCMinor));
> > Builder.defineMacro("__GNUC_PATCHLEVEL__", Twine(GNUCPatch));
> > Builder.defineMacro("__GXX_ABI_VERSION", "1002");
> >
> > if (LangOpts.CPlusPlus) {
> > Builder.defineMacro("__GNUG__", Twine(GNUCMajor));
> > Builder.defineMacro("__GXX_WEAK__");
> > }
> > }
> > ```
> /me wonders whether the right test here is actually `#if
> __has_feature(__builtin_add_overflow)` (etc)...
>
> but at this point, you definitely need an llvm person :-)
From
https://clang.llvm.org/docs/LanguageExtensions.html#checked-arithmetic-builtins,
we can check them with
__has_builtin(__builtin_add_overflow) && __has_builtin(__builtin_sub_overflow)
&& __has_builtin(__builtin_mul_overflow).
I saw some code also checks if __GNUC__ >= 5:
// The __GNUC__ checks can not be removed until we depend on GCC >= 10.1
// which is the first version that returns true for
__has_builtin(__builtin_add_overflow)
#if __GNUC__ >= 5 || __has_builtin(__builtin_add_overflow)
I guess we don't need to support real gcc using this header here. So maybe only
checking __has_builtin is enough?
By the way, if __builtin_add_overflow may not appear on some targets, do we
need to modify tests to specify triple like "-triple "x86_64-unknown-unknown""
in
https://github.com/llvm/llvm-project/blob/main/clang/test/CodeGen/builtins-overflow.c#L5
?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157331/new/
https://reviews.llvm.org/D157331
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D157331: [clang] Implement C23
yabinc accepted this revision. yabinc added a comment. This revision is now accepted and ready to land. LGTM, but please wait for a couple of days in case others wish to review Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157331/new/ https://reviews.llvm.org/D157331 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D135896: [clang] Fix clang version check in SARIF diagnostics test
yabinc added a comment. Thanks @aaron.ballman. I don’t have commit access, can you land this patch for me? Please use “Yabin Cui yab...@google.com” to commit the change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135896/new/ https://reviews.llvm.org/D135896 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D135896: [clang] Fix clang version check in SARIF diagnostics test
yabinc created this revision.
Herald added a project: All.
yabinc requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
This is to allow future clang versions and use of LLVM_VERSION_PATCH.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D135896
Files:
clang/test/Frontend/sarif-diagnostics.cpp
Index: clang/test/Frontend/sarif-diagnostics.cpp
===
--- clang/test/Frontend/sarif-diagnostics.cpp
+++ clang/test/Frontend/sarif-diagnostics.cpp
@@ -64,5 +64,5 @@
// CHECK:
{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"{{[0-9]+}}","name":""},{"defaultConfiguration":
// CHECK:
{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"{{[0-9]+}}","name":""},{"defaultConfiguration":
// CHECK: {"enabled":true,"level":"error","rank":50},"fullDescription":
-// CHECK:
{"text":""},"id":"{{[0-9]+}}","name":""}],"version":"16.0.0"}}}],"version":"2.1.0"}
+// CHECK:
{"text":""},"id":"{{[0-9]+}}","name":""}],"version":"{{[0-9]+\.[0-9]+\.[0-9]+}}"}}}],"version":"2.1.0"}
// CHECK: 2 warnings and 6 errors generated.
\ No newline at end of file
Index: clang/test/Frontend/sarif-diagnostics.cpp
===
--- clang/test/Frontend/sarif-diagnostics.cpp
+++ clang/test/Frontend/sarif-diagnostics.cpp
@@ -64,5 +64,5 @@
// CHECK: {"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"{{[0-9]+}}","name":""},{"defaultConfiguration":
// CHECK: {"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"{{[0-9]+}}","name":""},{"defaultConfiguration":
// CHECK: {"enabled":true,"level":"error","rank":50},"fullDescription":
-// CHECK: {"text":""},"id":"{{[0-9]+}}","name":""}],"version":"16.0.0"}}}],"version":"2.1.0"}
+// CHECK: {"text":""},"id":"{{[0-9]+}}","name":""}],"version":"{{[0-9]+\.[0-9]+\.[0-9]+}}"}}}],"version":"2.1.0"}
// CHECK: 2 warnings and 6 errors generated.
\ No newline at end of file
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
