[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 fo

[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 d

[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&id=159448#toc Repository: rC Clang https://r

[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] 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] 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 http://lists.llvm.org/cgi-bin/mailman/li

[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] 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 s

[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 t

[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 variables

[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: > > rs

[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 http:/

[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 implicit

[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 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#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#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 AFAI

[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 let

[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 u

[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: test/

[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] 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 shouldn't assert (authored by jfb, committed by ). Changed prior to commit: https://reviews.llvm.org/D46613?vs=145858&id=145859#toc

[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 t

[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] 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 doing

[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 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 th

[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] 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 lowere

[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 to

[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] 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 https://reviews.ll

[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 war

[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 in

[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 cast

[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] 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 wrot

[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: cfe/trunk/li

[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 http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c

[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(&unlogged_tail, memory_order_acquire); >do { >if (last_unlogged == 0)

[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] 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: libcxx/trunk/include/__c

[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 type

[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 w

[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 lib/

[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] 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 > > > `-Wf

[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 auditi

[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: include/clang/Basic/DiagnosticSemaKinds.td

[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] 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 clear

[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] 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] 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 t

[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 [6.3.2

[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 test/S

[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

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

jfb added inline comments. Comment at: lib/Sema/SemaChecking.cpp:10974 + if (E->IgnoreParenImpCasts()->getType()->isAtomicType()) +return; CheckImplicitConversion(S, E->IgnoreParenImpCasts(), S.Context.BoolTy, CC); rjmccall wrote: > Can you explain this o

[PATCH] D51752: NFC: move isRepeatedBytePattern from clang to Constant

jfb created this revision. Herald added subscribers: cfe-commits, dexonsmith. This code was in CGDecl.cpp and really belongs to Constant. This will allow me to use it in a subsequent patch. LLVM part of this patch: https://reviews.llvm.org/D51751 Repository: rC Clang https://reviews.llvm.org

[PATCH] D51752: NFC: move isRepeatedBytePattern from clang to Constant

jfb added a comment. In https://reviews.llvm.org/D51752#1226462, @MatzeB wrote: > - I'm not a good person to review clang changes. > - I assume the review lacks the changes on the llvm side? > - On the LLVM side this feels like something for `Analysis/ValueTracking.h` > in fact I already see `Va

[PATCH] D51752: NFC: move isRepeatedBytePattern from clang to Constant

jfb added a comment. In https://reviews.llvm.org/D51752#1226465, @jfb wrote: > In https://reviews.llvm.org/D51752#1226462, @MatzeB wrote: > > > - I'm not a good person to review clang changes. > > - I assume the review lacks the changes on the llvm side? > > - On the LLVM side this feels like som

[PATCH] D51752: NFC: move isRepeatedBytePattern from clang to Constant

jfb added a comment. In https://reviews.llvm.org/D51752#1226474, @MatzeB wrote: > In https://reviews.llvm.org/D51752#1226462, @MatzeB wrote: > > > - I assume the review lacks the changes on the llvm side? > > > Oops missed the link to the llvm changes in the description... At least in > theory y

[PATCH] D51752: NFC: move isRepeatedBytePattern from clang to Constant

jfb updated this revision to Diff 164481. jfb added a comment. - Use isBytewiseValue instead. Repository: rC Clang https://reviews.llvm.org/D51752 Files: lib/CodeGen/CGDecl.cpp Index: lib/CodeGen/CGDecl.cpp === --- lib/CodeGe

[PATCH] D51752: NFC: deduplicate isRepeatedBytePattern from clang to LLVM's isBytewiseValue

jfb added a comment. Will wait on https://reviews.llvm.org/D51751 before committing, otherwise clang tests will start failing (because current `isRepeatedBytePattern` isn't capable enough). Repository: rC Clang https://reviews.llvm.org/D51752 _

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

This revision was automatically updated to reflect the committed changes. Closed by commit rC341860: Implement -Watomic-implicit-seq-cst (authored by jfb, committed by ). Changed prior to commit: https://reviews.llvm.org/D51084?vs=164235&id=164746#toc Repository: rC Clang https://reviews.ll

[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(), diag::

[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 non-z

[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 F

[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 https://reviews.llvm.org/D49771

[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 http://lists.llvm.org/cgi-

[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 whic

[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] 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 tr

[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_load's _Atomic can be const (authored by jfb, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D47618 Files: cfe/trunk/lib/S

[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 -fno-fixed-poin

[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) noexcept;

[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: test/std/experimental/filesystem/fs.op.funcs/fs.op.last_write_time/last_writ

[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, decid

[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 const,

[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

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: test/std/experimental/filesystem/fs

[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: libcxx/trunk/te

[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: libcxx/trunk/include/ato

[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: > > http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es23-prefer-the--initial

[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 cas

[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 https://reviews.llvm.org/

[PATCH] D52949: [Diagnostics] Implement -Wsizeof-pointer-div

jfb added a comment. Can you add tests with arrays? The error message doesn't really say what to do instead. What's wrong? What's correct instead? In C++17 and later we should suggest using `std::size` for some cases instead. https://reviews.llvm.org/D52949 __

[PATCH] D53154: [CodeGen] Handle extern references to OBJC_CLASS_$_*

jfb added a comment. Overall this seems fine, but I'm no ObjC expert either so I'll defer to @rjmccall Comment at: clang/lib/CodeGen/CGObjCMac.cpp:7193 + if (!GV || GV->getType() != ObjCTypes.ClassnfABITy->getPointerTo()) { +auto *NewGV = new llvm::GlobalVariable(ObjCType

[PATCH] D54604: Automatic variable initialization

jfb marked an inline comment as done. jfb added inline comments. Comment at: cfe/trunk/include/clang/Driver/Options.td:1657 + " | pattern">, Values<"uninitialized,pattern">; +def enable_trivial_var_init_zero : Joined<["-"], "enable-trivial-auto-var-init-zero-knowing-it-will-be-

[PATCH] D56760: Add a new builtin: __builtin_dynamic_object_size

jfb added a comment. I pointed out this review to Jonathan Wakely, who posted it to the GCC list . Jakub replied with: > The current modes (0-3) certainly must not be changed and must return a > constant, that is what huge amounts of code in the

[PATCH] D56760: Add a new builtin: __builtin_dynamic_object_size

jfb added a comment. In D56760#1368216 , @rsmith wrote: > In D56760#1368054 , @erik.pilkington > wrote: > > > So it seems like the GCC people want to keep `__builtin_object_size` static. > > > I don't see any evide

[PATCH] D54604: Automatic variable initialization

jfb added a comment. In D54604#1375514 , @glider wrote: > Not sure if a similar problem was mentioned already or not, but the following > program: Yes please file a bug. Seems like the optimizer should sink the store enough that it appears only once on

  1   2   3   4   5   6   >