[PATCH] D32265: Add __CLANG_ATOMIC__LOCK_FREE macros for use in MSVC compatibility mode.

jfb added a comment. In https://reviews.llvm.org/D32265#731710, @EricWF wrote: > In https://reviews.llvm.org/D32265#731709, @jfb wrote: > > > Is it a goal to support Microsoft's STL with this? If so, how does MSVC's > > STL implement `is_always_lock_free` at the moment? CL 19 2017 RTW doesn't

[PATCH] D32265: Add __CLANG_ATOMIC__LOCK_FREE macros for use in MSVC compatibility mode.

jfb added a comment. It looks like Billy is going to do something somewhat similar when he rewrites it: https://twitter.com/jfbastien/status/855168230918307840 For now it's kinda `#define IS_LOCK_FREE ¯\_(ツ)_/¯` https://reviews.llvm.org/D32265 ___

[PATCH] D32265: Add __CLANG_ATOMIC__LOCK_FREE macros for use in MSVC compatibility mode.

jfb added a comment. Is it a goal to support Microsoft's STL with this? If so, how does MSVC's STL implement `is_always_lock_free` at the moment? CL 19 2017 RTW doesn't seem to have anything ? Presumably they'll have to do *something*.

[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers

jfb added a comment. In https://reviews.llvm.org/D42933#1092077, @smeenai wrote: > In https://reviews.llvm.org/D42933#1092048, @rjmccall wrote: > > > I agree that the format-specifier checker is not intended to be a > > portability checker. > I don't disagree with the original intent, but

[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers

jfb added a comment. In https://reviews.llvm.org/D42933#1091809, @jyknight wrote: > I also think that special casing these two specifiers doesn't make sense. The > problem is a general issue -- and one I've often found irritating. This exact > same situation comes up all the time in non-Darwin

[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers

jfb added a comment. In https://reviews.llvm.org/D42933#1090286, @smeenai wrote: > Note that the alignment matters in addition to the size. Sure, but AFAICT from `./lib/Basic/Targets/*` the alignment is also specified properly, is it not? > The pattern I've seen internally is people using

[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers

jfb added a comment. Herald added a reviewer: javed.absar. I was just looking at this, and I think @arphaman's patch is pretty much the right approach (with 2 suggested fixes below). I don't think the code we're currently warning on is broken: a user code has `NSInteger` with `%zd` or

[PATCH] D22711: Diagnose invalid failure memory orderings.

jfb requested changes to this revision. jfb added a comment. This revision now requires changes to proceed. Given http://wg21.link/p0418 I think this requires an update. I don't think the old behavior is worth supporting in older `-std=` versions, unless we're worried that it would make code

[PATCH] D23041: Un-XFAIL GCC atomics.align

jfb added a comment. Herald added subscribers: christof, aheejin. In https://reviews.llvm.org/D23041#632708, @EricWF wrote: > Have you filed a bug against GCC regarding its current behavior? > > Also it seems like a bad idea to add `-fabi-version=6`, since it selects an > older ABI version and

[PATCH] D46613: _Atomic of empty struct shouldn't assert

jfb updated this revision to Diff 145858. jfb added a comment. - Assert on zero alignment, instead of making it always byte-aligned. Repository: rC Clang https://reviews.llvm.org/D46613 Files: lib/AST/ASTContext.cpp test/CodeGen/c11atomics-ios.c test/CodeGen/c11atomics.c Index:

[PATCH] D46613: _Atomic of empty struct shouldn't assert

This revision was automatically updated to reflect the committed changes. jfb marked an inline comment as done. Closed by commit rC331845: _Atomic of empty struct shouldnt assert (authored by jfb, committed by ). Changed prior to commit: https://reviews.llvm.org/D46613?vs=145858=145859#toc

[PATCH] D46613: _Atomic of empty struct shouldn't assert

jfb created this revision. jfb added reviewers: arphaman, rjmccall. Herald added subscribers: cfe-commits, aheejin. An _Atomic of an empty struct is pretty silly. In general we just widen empty structs to hold a byte's worth of storage, and we represent size and alignment as 0 internally and

[PATCH] D46613: _Atomic of empty struct shouldn't assert

jfb marked an inline comment as done. jfb added inline comments. Comment at: lib/AST/ASTContext.cpp:1965 + Width = Target->getCharWidth(); + Align = Target->getCharWidth(); +} else if (Width <= Target->getMaxAtomicPromoteWidth()) { rjmccall wrote:

[PATCH] D45470: Emit an error when include after

jfb added a comment. In https://reviews.llvm.org/D45470#1092212, @vsapsai wrote: > Here is another approach that should emit an error only when mixing headers > causes compilation problems. > > Have no ideas how to test the change. `-verify` doesn't work with fatal errors > and libcxx doesn't

[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers

jfb added a comment. In https://reviews.llvm.org/D42933#1093503, @smeenai wrote: > Yeah, I think we all agree now that a portability warning isn't really > tractable. Note that even for the warnings that motivated this diff, they > should have only fired if `size_t` and NSInteger had separate

[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers

jfb added a comment. In https://reviews.llvm.org/D42933#1091234, @aaron.ballman wrote: > In https://reviews.llvm.org/D42933#1090268, @jfb wrote: > > > I was just looking at this, and I think @arphaman's patch is pretty much > > the right approach (with 2 suggested fixes below). > > > > I don't

[PATCH] D47096: CodeGen: block capture shouldn't ICE

jfb added a comment. In https://reviews.llvm.org/D47096#1105455, @rjmccall wrote: > `children()` is actually defined at the `Stmt` level, and if you look at how > it's implemented on e.g. `IfStmt`, you can see that it visits all of the > child `Stmt`s, including the if-condition. So it should

[PATCH] D47096: CodeGen: block capture shouldn't ICE

jfb updated this revision to Diff 147648. jfb added a comment. - move test Repository: rC Clang https://reviews.llvm.org/D47096 Files: lib/CodeGen/CGDecl.cpp test/CodeGenCXX/block-capture.cpp Index: test/CodeGenCXX/block-capture.cpp

[PATCH] D47096: CodeGen: block capture shouldn't ICE

jfb added a comment. In https://reviews.llvm.org/D47096#1105492, @rjmccall wrote: > Test case should go in test/CodeGenCXX. Also, there already is a > `blocks.cpp` there. I moved it, but didn't merge with the existing block.cpp because it just checks for not crashing. I'd rather make sure

[PATCH] D47096: CodeGen: block capture shouldn't ICE

jfb updated this revision to Diff 147647. jfb added a comment. - Follow John's suggestion. Repository: rC Clang https://reviews.llvm.org/D47096 Files: lib/CodeGen/CGDecl.cpp test/CodeGen/block-capture.cpp Index: test/CodeGen/block-capture.cpp

[PATCH] D47096: CodeGen: block capture shouldn't ICE

jfb closed this revision. jfb added a comment. r332801 Repository: rC Clang https://reviews.llvm.org/D47096 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

jfb created this revision. jfb added reviewers: ahatanak, vsapsai, alexshap, aaron.ballman, javed.absar, jfb, rjmccall. Herald added subscribers: cfe-commits, aheejin, kristof.beyls. Pick https://reviews.llvm.org/D42933 back up, and make NSInteger/NSUInteger with %zu/%zi specifiers on Darwin

[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type

jfb added a comment. This is on by default for any version of C? AFAICK `_Accum` isn't on the C17 draft that I have, I'd expect to have to specify a command-line flag pertaining to TR 18037 to get this. At a minimum I'd be OK having it with the GNU variant of C, but not the `__ANSI_C__` one.

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

jfb marked an inline comment as done. jfb added inline comments. Comment at: test/FixIt/fixit-format-ios-nopedantic.m:21 + printf("test 4: %zd %zd", getNSInteger(), getNSInteger()); +} alexshap wrote: > maybe i'm missing smth, but i don't see any verification

[PATCH] D47229: Make atomic non-member functions as nonnull

jfb created this revision. jfb added a reviewer: arphaman. Herald added subscribers: cfe-commits, aheejin. As a companion to libc++ patch https://reviews.llvm.org/D47225, mark builtin atomic non-member functions which accept pointers as nonnull. The atomic non-member functions accept pointers

[PATCH] D47225: Add nonnull; use it for atomics

jfb created this revision. jfb added reviewers: arphaman, EricWF. Herald added subscribers: cfe-commits, christof, aheejin. The atomic non-member functions accept pointers to std::atomic / std::atomic_flag as well as to the non-atomic value. These are all dereferenced unconditionally when

[PATCH] D47229: Make atomic non-member functions as nonnull

jfb added a comment. Fixed by r333290. Repository: rL LLVM https://reviews.llvm.org/D47229 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47229: Make atomic non-member functions as nonnull

jfb added a comment. In https://reviews.llvm.org/D47229#1112549, @jakehehrlich wrote: > This is causing breaks in fuchsia, > > Code that looks like this > > uintptr_t last_unlogged = >atomic_load_explicit(_tail, memory_order_acquire); >do { >if (last_unlogged == 0) >

[PATCH] D47225: Add nonnull; use it for atomics

jfb added a comment. GCC in libcxx-libcxxabi-x86_64-linux-ubuntu-cxx03 seems to mis-handle ATOMIC_VAR_INIT: File /home/llvm-builder/llvm-buildslave-root/libcxx-libcxxabi-x86_64-linux-ubuntu-cxx03/llvm/projects/libcxx/test/libcxx/atomics/diagnose_nonnull.fail.cpp Line 20: non-aggregate

[PATCH] D47225: Add nonnull; use it for atomics

This revision was automatically updated to reflect the committed changes. Closed by commit rL25: Add nonnull; use it for atomics (authored by jfb, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D47225 Files:

[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type

jfb added a comment. Can you also add a test for `_Bool _Accum`. Also, `-enable-fixed-point -x c++` failing. Comment at: lib/AST/ExprConstant.cpp:7361 +case BuiltinType::ULongAccum: + // GCC does not cover FIXED_POINT_TYPE in it's switch stmt and defaults to +

[PATCH] D47229: Make atomic non-member functions as nonnull

jfb added a comment. Addressed nit. Repository: rC Clang https://reviews.llvm.org/D47229 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47229: Make atomic non-member functions as nonnull

jfb updated this revision to Diff 148458. jfb marked an inline comment as done. jfb added a comment. - Address nit. Repository: rC Clang https://reviews.llvm.org/D47229 Files: lib/Sema/SemaChecking.cpp test/Sema/atomic-ops.c Index: test/Sema/atomic-ops.c

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

jfb updated this revision to Diff 148467. jfb marked an inline comment as done. jfb added a comment. - Merge format-size-spec-nsinteger Repository: rC Clang https://reviews.llvm.org/D47290 Files: include/clang/Analysis/Analyses/FormatString.h include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

jfb marked an inline comment as done. jfb added inline comments. Comment at: test/SemaObjC/format-size-spec-nsinteger.m:18 + NSUInteger j = 0; + NSLog(@"max NSInteger = %zi", i); // CHECK: values of type 'NSInteger' should not be used as format arguments; add an explicit

[PATCH] D47229: Make atomic non-member functions as nonnull

jfb updated this revision to Diff 148504. jfb marked an inline comment as done. jfb added a comment. - Address nit. - Change suggested by Richard Repository: rC Clang https://reviews.llvm.org/D47229 Files: lib/Sema/SemaChecking.cpp test/Sema/atomic-ops.c Index: test/Sema/atomic-ops.c

[PATCH] D47225: Add nonnull; use it for atomics

jfb added a comment. Ping! clang side landed in https://reviews.llvm.org/rL333246 Repository: rCXX libc++ https://reviews.llvm.org/D47225 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D47229: Make atomic non-member functions as nonnull

This revision was automatically updated to reflect the committed changes. Closed by commit rL333246: Make atomic non-member functions as nonnull (authored by jfb, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D47229 Files:

[PATCH] D47229: Make atomic non-member functions as nonnull

jfb added inline comments. Comment at: lib/Sema/SemaChecking.cpp:3497 +else if (Form == Copy || Form == Xchg) { + if (!IsC11 && !IsN) +// The value pointer is always dereferenced, a nullptr is undefined. rsmith wrote: > arphaman

[PATCH] D47096: CodeGen: block capture shouldn't ICE

jfb created this revision. jfb added a reviewer: rjmccall. Herald added subscribers: cfe-commits, aheejin. When a lambda capture captures a __block in the same statement, the compiler asserts out because isCapturedBy assumes that an Expr can only be a BlockExpr, StmtExpr, or if it's a Stmt then

[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers

jfb added a comment. Based on the cfe-dev discussion we'll want to handle the case of NSInteger with `%z` format on Darwin separately from other attempts at portability warnings in printf formats. I'll therefore re-post this patch

[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type

jfb added a comment. > Actually, scratch that. We will be enabling it since GCC does. Will update > this and other relevant C++ related code appropriately. Could you also add tests which mix _Accum with volatile, _Atomic, _Complex, constexpr, inline? Repository: rC Clang

[PATCH] D47096: CodeGen: block capture shouldn't ICE

jfb added a comment. In https://reviews.llvm.org/D47096#1105368, @rjmccall wrote: > RecursiveASTVisitor instantiations are huge. Can you just make the function > take a Stmt and then do the first few checks if it happens to be an Expr? I'm not super-familiar with the code, so I might be

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

jfb added a comment. In https://reviews.llvm.org/D47290#1126443, @aaron.ballman wrote: > In https://reviews.llvm.org/D47290#1125028, @aaron.ballman wrote: > > > Okay, that's fair, but the vendor-specific type for my Windows example is > > spelled `DWORD`. I'm really worried that this special

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

jfb marked an inline comment as done. jfb added a comment. In https://reviews.llvm.org/D47290#1113365, @aaron.ballman wrote: > This is relaxing `-Wformat` and making users instead write > `-Wformat-pedantic` to get the strong guarantees, which is the opposite of > what I thought the consensus

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

jfb updated this revision to Diff 148737. jfb marked 2 inline comments as done. jfb added a comment. - Fix variable capitalization. Repository: rC Clang https://reviews.llvm.org/D47290 Files: include/clang/Analysis/Analyses/FormatString.h include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

jfb added a comment. Addressed comments. Repository: rC Clang https://reviews.llvm.org/D47290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type

jfb added inline comments. Comment at: include/clang/Basic/TokenKinds.def:393 +// ISO/IEC JTC1 SC22 WG14 N1169 Extension +KEYWORD(_Accum , KEYALL) + ebevhan wrote: > I believe that having KEYALL will enable the keyword even if

[PATCH] D47557: Filesystem tests: un-confuse write time

jfb created this revision. jfb added reviewers: EricWF, mclow.lists, aemerson. Herald added subscribers: cfe-commits, christof. The filesystem test was confused about access versus write / modification time. The spec says: file_time_type last_write_time(const path& p, error_code& ec)

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

jfb added a comment. In https://reviews.llvm.org/D47290#1113422, @aaron.ballman wrote: > In https://reviews.llvm.org/D47290#1113412, @jfb wrote: > > > In https://reviews.llvm.org/D47290#1113365, @aaron.ballman wrote: > > > > > This is relaxing `-Wformat` and making users instead write > > >

[PATCH] D47557: Filesystem tests: un-confuse write time

jfb updated this revision to Diff 149398. jfb added a comment. - Add back directory test Repository: rCXX libc++ https://reviews.llvm.org/D47557 Files: test/std/experimental/filesystem/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp Index:

[PATCH] D47618: __c11_atomic_load's _Atomic can be const

jfb created this revision. jfb added a reviewer: rsmith. Herald added a subscriber: cfe-commits. C++11 onwards specs the non-member functions atomic_load and atomic_load_explicit as taking the atomic by const (potentially volatile) pointer. C11, in its infinite wisdom, decided to drop the

[PATCH] D47618: __c11_atomic_load's _Atomic can be const

jfb added a comment. Note that I don't touch the OpenCL __constant stuff, because the separate address space means this can be actually read-only, which means you can't cmpxchg for very wide reads. Repository: rC Clang https://reviews.llvm.org/D47618

[PATCH] D47557: Filesystem tests: un-confuse write time

This revision was automatically updated to reflect the committed changes. Closed by commit rL333723: Filesystem tests: un-confuse write time (authored by jfb, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D47557 Files:

[PATCH] D47613: Mark __c11_atomic_load as const

jfb created this revision. jfb added reviewers: EricWF, mclow.lists. Herald added subscribers: cfe-commits, christof. C++11 onwards specs the non-member functions atomic_load and atomic_load_explicit as taking the atomic by const (potentially volatile) pointer. C11, in its infinite wisdom,

[PATCH] D47557: Filesystem tests: un-confuse write time

jfb updated this revision to Diff 149377. jfb added a comment. - Remove access time checks, simplify existing check, after talking to EricWF on IRC. Repository: rCXX libc++ https://reviews.llvm.org/D47557 Files:

[PATCH] D47613: Mark __c11_atomic_load as const

This revision was automatically updated to reflect the committed changes. Closed by commit rL333776: Mark __c11_atomic_load as const (authored by jfb, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D47613 Files:

[PATCH] D46024: [clang-format] Add SpaceBeforeCpp11BracedList option.

jfb added a comment. In https://reviews.llvm.org/D46024#1121242, @rkirsling wrote: > FWIW, please note that this space-before-brace style is not specific to > WebKit; CppCoreGuidelines exhibits it as well: > >

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

This revision was automatically updated to reflect the committed changes. Closed by commit rL335393: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin (authored by jfb, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM

[PATCH] D46024: [clang-format] Add SpaceBeforeCpp11BracedList option.

jfb accepted this revision. jfb added a comment. This revision is now accepted and ready to land. On the WebKit side this lgtm. Let's leave some time for clang-format folks to chime in. Repository: rC Clang https://reviews.llvm.org/D46024 ___

[PATCH] D46386: Adding __atomic_fetch_min/max intrinsics to clang

jfb added inline comments. Comment at: lib/Sema/SemaChecking.cpp:3098 + case AtomicExpr::AO__atomic_fetch_umax: +IsMinMax = true; +Form = Arithmetic; Should `__sync_fetch_and_min` and others also set `IsMinMax`? Repository: rC Clang

[PATCH] D45470: Emit an error when mixing and

jfb added a comment. This isn't bad, so I'd go with it, but separately I imagine that we could implement the suggestion in http://wg21.link/p0943 and expose it even before C++20? Not sure we do this much, but I'd argue that before that fix stdatomic.h is just useless anyways, so it's a fine

[PATCH] D47618: __c11_atomic_load's _Atomic can be const

This revision was automatically updated to reflect the committed changes. Closed by commit rL338743: __c11_atomic_loads _Atomic can be const (authored by jfb, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D47618 Files:

[PATCH] D49771: CodeGen: use non-zero memset when possible for automatic variables

jfb added a comment. In https://reviews.llvm.org/D49771#1183641, @mehdi_amini wrote: > > I'm worried, however, about generating a bunch more code than needed from > > clang in the hopes that the compiler will clean it up later. > > Isn't a strong design component of clang/LLVM? Clang does not

[PATCH] D49771: CodeGen: use non-zero memset when possible for automatic variables

jfb added a comment. In https://reviews.llvm.org/D49771#1181008, @mehdi_amini wrote: > I'm curious: isn't the kind of optimization we should expect LLVM to provide? Maybe? It seems obvious to do here since we know we'll probably want to be doing it, and I have another patch I'm working on

[PATCH] D49771: CodeGen: use non-zero memset when possible for automatic variables

jfb added a comment. In https://reviews.llvm.org/D49771#1183562, @mehdi_amini wrote: > In https://reviews.llvm.org/D49771#1183380, @jfb wrote: > > > In https://reviews.llvm.org/D49771#1181008, @mehdi_amini wrote: > > > > > I'm curious: isn't the kind of optimization we should expect LLVM to > >

[PATCH] D47618: __c11_atomic_load's _Atomic can be const

jfb updated this revision to Diff 157761. jfb added a comment. - Add constant AS pointer test. Repository: rC Clang https://reviews.llvm.org/D47618 Files: lib/Sema/SemaChecking.cpp test/Sema/atomic-ops.c test/SemaOpenCL/atomic-ops.cl Index: test/SemaOpenCL/atomic-ops.cl

[PATCH] D47618: __c11_atomic_load's _Atomic can be const

jfb marked an inline comment as done. jfb added a comment. Give the comments, I think this is ready to commit. Repository: rC Clang https://reviews.llvm.org/D47618 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D44671: [libcxx] Enable static libcxxabi linking on Darwin

jfb added a comment. Were the concerns expressed in https://reviews.llvm.org/D8017 addressed? Repository: rCXX libc++ https://reviews.llvm.org/D44671 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

jfb added a comment. In https://reviews.llvm.org/D45639#1192849, @beanz wrote: > Adding @jfb since this is his domain now too. @ldionne is the libc++ expert :-) Repository: rC Clang https://reviews.llvm.org/D45639 ___ cfe-commits mailing list

[PATCH] D50361: [NFC] Test automatic variable initialization

jfb created this revision. Herald added subscribers: cfe-commits, dexonsmith. r337887 started using memset for automatic variable initialization where sensible. A follow-up discussion leads me to believe that we should better test automatic variable initialization, and that there are probably

[PATCH] D50361: [NFC] Test automatic variable initialization

jfb added a comment. I'm honestly not sure there's anything to review here since it's just showing us what the current behavior is. LMK if I'm not testing something that I should. I'd much rather test current behavior as one patch first, because then the follow-ups show a clear before / after

[PATCH] D50361: [NFC] Test automatic variable initialization

This revision was automatically updated to reflect the committed changes. Closed by commit rC339089: [NFC] Test automatic variable initialization (authored by jfb, committed by ). Changed prior to commit: https://reviews.llvm.org/D50361?vs=159397=159448#toc Repository: rC Clang

[PATCH] D50361: [NFC] Test automatic variable initialization

jfb added a comment. Further fixes in r339090 and r339093. Repository: rC Clang https://reviews.llvm.org/D50361 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50719: [libc++] Fix incorrect definition of TEST_HAS_C11_FEATURES

jfb accepted this revision. jfb added a comment. This revision is now accepted and ready to land. From `__ISO_C_VISIBLE >= 2011` it looks like this tries to test C11 features regardless of the C++ version. That's probably fine, but we're walking this line where only C++17 really guarantees C11

[PATCH] D50549: [libcxx] [test] Repair thread unsafety in thread tests

jfb added a comment. CC some sanitizer folks. https://reviews.llvm.org/D50549 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50994: Add a new flag and attributes to control static destructor registration

jfb added inline comments. Comment at: clang/include/clang/AST/Decl.h:1472 + /// Do we need to emit an exit-time destructor for this variable? + bool isNoDestroy(const ASTContext &) const; This is only valid for static variables, right? It's probably better

[PATCH] D50994: Add a new flag and attributes to control static destructor registration

jfb added inline comments. Comment at: clang/include/clang/AST/Decl.h:1472 + /// Do we need to emit an exit-time destructor for this variable? + bool isNoDestroy(const ASTContext &) const; rsmith wrote: > jfb wrote: > > This is only valid for static

[PATCH] D50994: Add a new flag and attributes to control static destructor registration

jfb accepted this revision. jfb added inline comments. Comment at: clang/include/clang/AST/Decl.h:1472 + /// Do we need to emit an exit-time destructor for this variable? + bool isNoDestroy(const ASTContext &) const; erik.pilkington wrote: > jfb wrote: > >

[PATCH] D50993: [clangd] Increase stack size of the new threads on macOS

jfb added a comment. Isn't this duplicating code in lib/Support/Unix/Threading.inc with a different implementation? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50993 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D51084: Implement -Watomic-implicit-seq-cst

jfb created this revision. jfb added a reviewer: rjmccall. Herald added subscribers: cfe-commits, dexonsmith. _Atomic and __sync_* operations are implicitly sequentially-consistent. Some codebases want to force explicit usage of memory order instead. This warning allows them to know where

[PATCH] D51170: [libc++] Remove race condition in std::async

jfb accepted this revision. jfb added inline comments. This revision is now accepted and ready to land. Comment at: libcxx/include/future:556 bool __has_value() const {return (__state_ & __constructed) || (__exception_ != nullptr);} I'm not

[PATCH] D51084: Implement -Watomic-implicit-seq-cst

jfb added a comment. Updated. Repository: rC Clang https://reviews.llvm.org/D51084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51084: Implement -Watomic-implicit-seq-cst

jfb updated this revision to Diff 162484. jfb marked 2 inline comments as done. jfb added a comment. - Address John's comments: diagnose at beginning, and don't check isIgnored manually. Repository: rC Clang https://reviews.llvm.org/D51084 Files:

[PATCH] D51084: Implement -Watomic-implicit-seq-cst

jfb added inline comments. Comment at: lib/Sema/SemaChecking.cpp:10668 + if (Source->isAtomicType() || Target->isAtomicType()) +S.Diag(E->getBeginLoc(), diag::warn_atomic_implicit_seq_cst); + rjmccall wrote: > jfb wrote: > > rjmccall wrote: > > > Why would

[PATCH] D51084: Implement -Watomic-implicit-seq-cst

jfb added inline comments. Comment at: lib/Sema/SemaChecking.cpp:10668 + if (Source->isAtomicType() || Target->isAtomicType()) +S.Diag(E->getBeginLoc(), diag::warn_atomic_implicit_seq_cst); + rjmccall wrote: > Why would the target be an atomic type? And if

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

jfb added inline comments. Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:124 +// A CompileCommand that can be applied to another file. Any instance of this +// object is invalid after std::move() from it. struct TransferableCommand { This comment

[PATCH] D51314: Parse compile commands lazily in InterpolatingCompilationDatabase

jfb requested changes to this revision. jfb added inline comments. This revision now requires changes to proceed. Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:201 + + CommandTraits computeTraits() const { +CommandTraits Result; It's not

[PATCH] D47618: __c11_atomic_load's _Atomic can be const

jfb added a subscriber: yaxunl. jfb added inline comments. Comment at: lib/Sema/SemaChecking.cpp:3361 +if ((Form != Load && Form != LoadCopy && AtomTy.isConstQualified()) || AtomTy.getAddressSpace() == LangAS::opencl_constant) { Diag(DRE->getLocStart(),

[PATCH] D49458: Support implicit _Atomic struct load / store

jfb created this revision. jfb added a reviewer: dexonsmith. Herald added a subscriber: cfe-commits. Using _Atomic to do implicit load / store is just a seq_cst atomic_load / atomic_store. Stores currently assert in Sema::ImpCastExprToType with 'can't implicitly cast lvalue to rvalue with this

[PATCH] D46112: Allow _Atomic to be specified on incomplete types

jfb added a comment. In https://reviews.llvm.org/D46112#1113557, @aaron.ballman wrote: > In https://reviews.llvm.org/D46112#1098243, @rsmith wrote: > > > In https://reviews.llvm.org/D46112#1096893, @aaron.ballman wrote: > > > > > In https://reviews.llvm.org/D46112#1091981, @efriedma wrote: > > >

[PATCH] D49458: Support implicit _Atomic struct load / store

jfb added a comment. I'm sure there are other bugs around `_Atomic`, please file a bug an CC me if that's the case. I'll commit this fix for now. Repository: rC Clang https://reviews.llvm.org/D49458 ___ cfe-commits mailing list

[PATCH] D49458: Support implicit _Atomic struct load / store

This revision was automatically updated to reflect the committed changes. Closed by commit rL337410: Support implicit _Atomic struct load / store (authored by jfb, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D49458 Files:

[PATCH] D49771: CodeGen: use non-zero memset when possible for automatic variables

jfb created this revision. jfb added a reviewer: dexonsmith. Herald added a subscriber: cfe-commits. Right now automatic variables are either initialized with bzero followed by a few stores, or memcpy'd from a synthesized global. We end up encountering a fair amount of code where memcpy of

[PATCH] D49771: CodeGen: use non-zero memset when possible for automatic variables

jfb updated this revision to Diff 157191. jfb marked 2 inline comments as done. jfb added a comment. - Use short to test padding between array elements. - Define enum class storage type; swap order of if / else to make it more readable. Repository: rC Clang https://reviews.llvm.org/D49771

[PATCH] D49771: CodeGen: use non-zero memset when possible for automatic variables

jfb marked an inline comment as done. jfb added a comment. Addressed all comments. Comment at: lib/CodeGen/CGDecl.cpp:956-957 +class BytePattern { + uint8_t Val; + enum class ValueType { Specific, Any, None } Type; + BytePattern(ValueType Type) : Type(Type) {}

[PATCH] D49771: CodeGen: use non-zero memset when possible for automatic variables

This revision was automatically updated to reflect the committed changes. Closed by commit rL337887: CodeGen: use non-zero memset when possible for automatic variables (authored by jfb, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM

[PATCH] D51084: Implement -Watomic-implicit-seq-cst

jfb added a comment. In https://reviews.llvm.org/D51084#1216913, @rjmccall wrote: > It says the type of the assignment expression, not the type of the LHS. > > C11 [6.5.16]p2: "The type of an assignment expression is the type the left > operand would have after lvalue conversion." > > C11

[PATCH] D51084: Implement -Watomic-implicit-seq-cst

jfb updated this revision to Diff 163577. jfb added a comment. - Don't diagnose initialization, only assignment. Repository: rC Clang https://reviews.llvm.org/D51084 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaChecking.cpp test/Sema/atomic-implicit-seq_cst.c

[PATCH] D51084: Implement -Watomic-implicit-seq-cst

jfb updated this revision to Diff 164235. jfb marked 4 inline comments as done. jfb added a comment. - Address comments. Repository: rC Clang https://reviews.llvm.org/D51084 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaChecking.cpp test/Sema/atomic-implicit-seq_cst.c

  1   2   3   4   5   6   >