[PATCH] D65706: [docs] document -Weveything more betterer

jfb created this revision. jfb added a reviewer: aaron.ballman. Herald added subscribers: cfe-commits, dexonsmith, jkorous. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D65706 Files: clang/docs/UsersManual.rst Index:

[PATCH] D65545: Handle some fs::remove failures

jfb added inline comments. Comment at: clang/lib/Frontend/CompilerInstance.cpp:647-649 + if (std::error_code EC = llvm::sys::fs::remove(OF.TempFilename)) +getDiagnostics().Report(diag::err_fe_error_removing) + << OF.TempFilename << EC.message();

[PATCH] D65545: Handle some fs::remove failures

jfb updated this revision to Diff 212899. jfb marked 11 inline comments as done. jfb added a comment. - Return llvm::Error from ASTUnit::Save - Update per comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65545/new/

[PATCH] D65545: Handle some fs::remove failures

jfb created this revision. jfb added reviewers: Bigcheese, bruno, arphaman, vsapsai. Herald added subscribers: llvm-commits, cfe-commits, dexonsmith, jkorous, hiraditya. Herald added projects: clang, LLVM. We have data showing that some modules builds fail in rare cases. We're therefore

[PATCH] D65458: [NFC] Remove LLVM_ALIGNAS

This revision was automatically updated to reflect the committed changes. Closed by commit rL367383: [NFC] Remove LLVM_ALIGNAS (authored by jfb, committed by ). Herald added a subscriber: kristina. Changed prior to commit: https://reviews.llvm.org/D65458?vs=212368=212496#toc Repository: rL

[PATCH] D65493: Modernize atomic detection and usage

jfb updated this revision to Diff 212479. jfb added a comment. - Remove from cmake Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65493/new/ https://reviews.llvm.org/D65493 Files: clang-tools-extra/clangd/CMakeLists.txt

[PATCH] D65493: Modernize atomic detection and usage

jfb created this revision. jfb added reviewers: rnk, Bigcheese, __simt__. Herald added subscribers: llvm-commits, lldb-commits, cfe-commits, kadircet, arphaman, dexonsmith, jkorous, hiraditya, mgorny. Herald added projects: clang, LLDB, LLVM. Some of the cmake checks are obsolete and make

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

jfb added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5609 +def warn_mul_in_bool_context : Warning< + "'*' in bool context, maybe you mean '&&'?">, + InGroup; xbolva00 wrote: > jfb wrote: > > aaron.ballman wrote: > > > xbolva00

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

jfb added inline comments. Comment at: test/Sema/warn-int-in-bool-context.c:26 + r = a << 7; // expected-warning {{'<<' in boolean context; did you mean '<'?}} + r = ONE << b; // expected-warning {{'<<' in boolean context; did you mean '<'?}} + xbolva00

[PATCH] D55895: NFC: simplify Darwin environment handling

This revision was automatically updated to reflect the committed changes. Closed by commit rG42c9f3c9116c: [NFC] simplify Darwin environment handling (authored by jfb). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55895/new/

[PATCH] D55895: NFC: simplify Darwin environment handling

jfb updated this revision to Diff 212417. jfb added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55895/new/ https://reviews.llvm.org/D55895 Files: clang/lib/Driver/ToolChains/Darwin.cpp Index:

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

jfb added inline comments. Comment at: test/Sema/warn-int-in-bool-context.c:26 + r = a << 7; // expected-warning {{'<<' in boolean context; did you mean '<'?}} + r = ONE << b; // expected-warning {{'<<' in boolean context; did you mean '<'?}} + xbolva00

[PATCH] D65458: [NFC] Remove LLVM_ALIGNAS

jfb created this revision. jfb added a reviewer: rnk. Herald added subscribers: llvm-commits, cfe-commits, dexonsmith, jkorous, mgorny. Herald added projects: clang, LLVM. The minimum compilers support all have alignas, and we don't use LLVM_ALIGNAS anywhere anymore. This also removes an MSVC

[PATCH] D65249: [NFC] use C++11 in AlignOf.h, remove AlignedCharArray

jfb closed this revision. jfb added a comment. https://reviews.llvm.org/rL367282 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65249/new/ https://reviews.llvm.org/D65249 ___ cfe-commits mailing list

[PATCH] D64742: Allow using -ftrivial-auto-var-init=zero in C mode without extra flags

jfb added a comment. In D64742#1606244 , @glider wrote: > As a data point, Linus Torvalds suggested that we need a similar feature for > GCC so that the "kernel C standard" mandates zero-initialization for locals: > https://lkml.org/lkml/2019/7/28/206

[PATCH] D63139: [Diagnostics] Implement -Wswitch-unreachable

jfb added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8192 +def warn_unreachable_stmt_in_switch : Warning< + "statement will be never executed">, InGroup>; def warn_bool_switch_condition : Warning< aaron.ballman wrote: > I thought

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

jfb added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5609 +def warn_mul_in_bool_context : Warning< + "'*' in bool context, maybe you mean '&&'?">, + InGroup; aaron.ballman wrote: > xbolva00 wrote: > > aaron.ballman wrote: > > > I

[PATCH] D65249: [NFC] use C++11 in AlignOf.h, remove AlignedCharArray

jfb added a comment. In D65249#1605523 , @rnk wrote: > I have concerns that some of the patches that you landed prior to this will > cause issues with old versions of MSVC, but in isolation, this is fine, and > if anyone complains, we can address it on

[PATCH] D65249: [NFC] use C++11 in AlignOf.h, remove AlignedCharArray

jfb added a comment. In D65249#1603431 , @BillyONeal wrote: > (In fact I observe many patterns in this review like: > > enum { Foo = alignof(void*); } > aligned_storage_t<1234, Foo> x; > > and then a bunch of casting to treat it as a char buffer; if it

[PATCH] D65249: [NFC] use C++11 in AlignOf.h, remove AlignedCharArray

jfb updated this revision to Diff 212252. jfb added a comment. Significantly simplify the patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65249/new/ https://reviews.llvm.org/D65249 Files: llvm/include/llvm/Support/AlignOf.h

[PATCH] D64146: [ConstExprPreter][WIP] Initial patch for the constexpr interpreter

jfb added a comment. In D64146#1604717 , @nand wrote: > > How do you intend to represent pointers cast to integer types? Allocating > > 64 bits of state for a 64-bit integer is insufficient to model that case. > > Is this ever going to be allowed in

[PATCH] D65249: [NFC] use C++11 in AlignOf.h

jfb added a comment. In D65249#1603278 , @BillyONeal wrote: > > @BillyONeal do you know if 19.11 has the aligned_storage issue on x86? > > aligned_storage is still capped at what the library can fake with unions. It > would be an ABI break to change it

[PATCH] D65249: [NFC] use C++11 in AlignOf.h

jfb added a subscriber: BillyONeal. jfb added a comment. In D65249#1603133 , @rnk wrote: > I still think this concern applies: > https://reviews.llvm.org/D64417#1576675 > > I'm not sure how to track down an 19.11 release to test if we can pass >

[PATCH] D64656: Ensure placeholder instruction for cleanup is created

jfb reopened this revision. jfb added a comment. This revision is now accepted and ready to land. Reverted in r367051, it's causing failures:

[PATCH] D65254: Allow prefetching from non-zero address spaces

This revision was automatically updated to reflect the committed changes. Closed by commit rL367032: Allow prefetching from non-zero address spaces (authored by jfb, committed by ). Changed prior to commit: https://reviews.llvm.org/D65254?vs=211637=211774#toc Repository: rL LLVM CHANGES

[PATCH] D65254: Allow prefetching from non-zero address spaces

jfb created this revision. Herald added subscribers: llvm-commits, cfe-commits, dexonsmith, jkorous, kbarton, hiraditya, javed.absar, nemanjai. Herald added projects: clang, LLVM. This is useful for targets which have prefetch instructions for non-default address spaces.

[PATCH] D65249: [NFC] use C++11 in AlignOf.h

jfb added a comment. This is the main event: https://reviews.llvm.org/D65249#change-IWk6CtRl45h6 The rest is side-show. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65249/new/ https://reviews.llvm.org/D65249

[PATCH] D65249: [NFC] use C++11 in AlignOf.h

jfb created this revision. jfb added a reviewer: chandlerc. Herald added subscribers: llvm-commits, lldb-commits, cfe-commits, dexonsmith, jkorous, hiraditya, javed.absar. Herald added projects: clang, LLDB, LLVM. jfb added a comment. This is the main event:

[PATCH] D64666: [Sema] Enable -Wimplicit-float-conversion for integral to floating point precision loss

jfb accepted this revision. jfb added a comment. Great, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64666/new/ https://reviews.llvm.org/D64666 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D64666: [Sema] Enable -Wimplicit-float-conversion for integral to floating point precision loss

jfb added a comment. In D64666#1599524 , @ziangwan wrote: > In D64666#1598194 , @jfb wrote: > > > Thanks, the update looks good. > > > > One thing I just noticed: if I have a codebase with > >

[PATCH] D64666: [Sema] Enable -Wimplicit-float-conversion for integral to floating point precision loss

jfb added a comment. Thanks, the update looks good. One thing I just noticed: if I have a codebase with `-Wimplicit-float-conversion` it seems like updating clang will automatically turn the two new warnings on, correct? That seems good, but also difficult to roll out because I can't turns

[PATCH] D64666: [Sema] Enable -Wimplicit-float-conversion for integral to floating point precision loss

jfb requested changes to this revision. jfb added a comment. Herald added a subscriber: dexonsmith. In D64666#1597193 , @aaron.ballman wrote: > In D64666#1596660 , @xbolva00 wrote: > > > I think we should warn in

[PATCH] D64742: Allow using -ftrivial-auto-var-init=zero in C mode without extra flags

jfb added a comment. A lots of folks from the original discussion insisted on this as a compromise. I'd like to make sure they see and approve of this, they might have requests for e.g. specific performance numbers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D64676: Support __seg_fs and __seg_gs on x86

This revision was automatically updated to reflect the committed changes. Closed by commit rL366028: Support __seg_fs and __seg_gs on x86 (authored by jfb, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D64676: Support __seg_fs and __seg_gs on x86

jfb updated this revision to Diff 209645. jfb added a comment. - Fix test order Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64676/new/ https://reviews.llvm.org/D64676 Files: clang/docs/LanguageExtensions.rst clang/lib/Basic/Targets/X86.cpp

[PATCH] D64675: WIP: Disable optimization in emitStoresForConstant

jfb added a comment. I would keep single stores because it's pretty silly codegen to copy from a global for that. I would also verify that `-O0` isn't affected too badly by this change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64675/new/

[PATCH] D64675: WIP: Disable optimization in emitStoresForConstant

jfb added a comment. That's interesting. If `MemCpyOpt` has picked up the slack and clang can be simpler here, then let's remove clang's "intelligence". I do want to make sure that it handles all the cases this handles though, and does indeed generate great code on both x86-64 and ARM64. I

[PATCH] D64676: Support __seg_fs and __seg_gs on x86

jfb created this revision. Herald added subscribers: cfe-commits, dexonsmith, jkorous. Herald added a project: clang. GCC supports named address spaces macros: https://gcc.gnu.org/onlinedocs/gcc/Named-Address-Spaces.html clang does as well with address spaces:

[PATCH] D64666: Allow Clang -Wconversion, -Wimplicit-float-conversion warns about integer type -> floating point type implicit conversion precision loss.

jfb added a comment. I think you want to default-ignore the "may lose precision" warnings, but not the ones that you know always lose precision. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64666/new/ https://reviews.llvm.org/D64666

[PATCH] D64597: CodeGet: Init 32bit pointers with 0xFFFFFFFF

jfb added a comment. In D64597#1582944 , @hubert.reinterpretcast wrote: > In D64597#1581605 , @pcc wrote: > > > The problem with `0x` on 32-bit is that it is likely to be a valid > > address. > > > > When

[PATCH] D64146: [ConstExprPreter][WIP] Initial patch for the constexpr interpreter

jfb added inline comments. Comment at: clang/lib/AST/ExprVM/Compiler.cpp:85 +/* isArray */ false, +/* isGlobal */ false); + ParamDescriptors.insert({ParamOffset,

[PATCH] D64597: CodeGet: Init 32bit pointers with 0xAAAAAAAA

jfb added a comment. In D64597#1581605 , @pcc wrote: > The problem with `0x` on 32-bit is that it is likely to be a valid > address. > > When I discussed this with JF I proposed a pointer initialization of > `0x` which he agreed to. This

[PATCH] D64382: Use getMostFrequentByte to decide if should used memset+stores

jfb added inline comments. Comment at: clang/lib/CodeGen/CGDecl.cpp:865 +/// initializer with equal or fewer than NumStores scalar stores. +static bool canEmitStoresForInitAfterMemset(CodeGenModule , +llvm::Constant *Init,

[PATCH] D64262: Bitstream reader: Fix undefined behavior seen after rL364464

jfb accepted this revision. jfb added a comment. This revision is now accepted and ready to land. Thanks for catching this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64262/new/ https://reviews.llvm.org/D64262

[PATCH] D63518: BitStream reader: propagate errors

jfb marked an inline comment as done. jfb added inline comments. Comment at: clang-tools-extra/trunk/clang-doc/BitcodeReader.cpp:619 +// FIXME check that the enum is in range. +auto Code = static_cast(MaybeCode.get()); + jfb wrote: > bjope wrote: > >

[PATCH] D63518: BitStream reader: propagate errors

jfb marked 2 inline comments as done. jfb added inline comments. Comment at: clang-tools-extra/trunk/clang-doc/BitcodeReader.cpp:619 +// FIXME check that the enum is in range. +auto Code = static_cast(MaybeCode.get()); + bjope wrote: > This has caused

[PATCH] D64164: [clang][ARM] Fix msvc arm{64} builtins to use int on LLP64 systems.

jfb accepted this revision. jfb added inline comments. This revision is now accepted and ready to land. Comment at: test/CodeGen/ms-intrinsics-other.c:212 + return _InterlockedAdd(Addend, Value); +} + Looks like only `cmpxchg` preserves `volatile`? That's not

[PATCH] D64128: [CodeGen] Generate llvm.ptrmask instead of inttoptr(and(ptrtoint, C)) if possible.

jfb added a comment. I'm not sure I understand all the implications, and why that would / wouldn't be valid. Should this be an builtin that can be called from C++ directly? Comment at: clang/lib/CodeGen/CGExprScalar.cpp:2034 + (AllOnes <<

[PATCH] D59254: [RFC] Implementation of Clang randstruct

jfb added a comment. In D59254#1566895 , @shawnl wrote: > My point is that most languages these days that intend to be compiled to > machine code want compatibility with the C ABI, and randstruct will be part > of that (and can be made compatible

[PATCH] D59254: [RFC] Implementation of Clang randstruct

jfb added a comment. In D59254#1566711 , @shawnl wrote: > I think the essential functionality of this patch should be in LLVM and not > Clang, so that all front-ends can benefit. Too many generally useful things > are in Clang when they should be in

[PATCH] D59254: [RFC] Implementation of Clang randstruct

jfb added a comment. Do you intend on moving this forward? It seems like a Good Thing overall, and I'd love to help review some more. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59254/new/ https://reviews.llvm.org/D59254

[PATCH] D60974: Clang IFSO driver action.

jfb added subscribers: arphaman, ributzka, Bigcheese. jfb added a comment. In D60974#1565621 , @plotfi wrote: > In D60974#1565577 , @jfb wrote: > > > Looking at the code quickly, I'm not sure that this should be in

[PATCH] D60974: Clang IFSO driver action.

jfb added a comment. Looking at the code quickly, I'm not sure that this should be in clang itself. It sounds like a better fit for a clang-based tool, and not clang. Why does it need to be part of clang? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D60974: Clang IFSO driver action.

jfb added a comment. In D60974#1565541 , @plotfi wrote: > In D60974#1565527 , @jfb wrote: > > > In D60974#1565517 , @jfb wrote: > > > > > In D60974#1565054

[PATCH] D60974: Clang IFSO driver action.

jfb added a comment. In D60974#1565517 , @jfb wrote: > In D60974#1565054 , @plotfi wrote: > > > So I think I know what may be going on on your end. The llvm-readelf in > > your path I believe might be the wrong

[PATCH] D60974: Clang IFSO driver action.

jfb added a comment. In D60974#1565054 , @plotfi wrote: > So I think I know what may be going on on your end. The llvm-readelf in your > path I believe might be the wrong lllvm-readelf (llvm-readelf-7). Are you > sure you built llvm-readelf from git?

[PATCH] D63943: ObjC: Squeeze in one more bit for the count of protocols in 'id' types

jfb added a comment. DO you have a test for that many protocols? That way when we add a diagnostic we know it won't fail. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63943/new/ https://reviews.llvm.org/D63943 ___

[PATCH] D63518: BitStream reader: propagate errors

jfb marked 2 inline comments as done. jfb added a subscriber: hans. jfb added inline comments. Comment at: llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp:205 + return MaybeBitCode.takeError(); +switch (unsigned BitCode = MaybeBitCode.get()) { default: // Default

[PATCH] D63518: BitStream reader: propagate errors

jfb added a comment. I think this is ready to go! I rebased and ran `check-all` for LLVM / clang / clang-tools-extras and everything passes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63518/new/ https://reviews.llvm.org/D63518

[PATCH] D63518: BitStream reader: propagate errors

jfb added a comment. I've addressed Lang's comments and the re-audited all the new FIXME instances. This patch now actually drops errors on the floor instead of implicitly erroring out unless the error path is tested (which is what I had before). This hides bugs, but I left FIXMEs everywhere

[PATCH] D63518: BitStream reader: propagate errors

jfb added inline comments. Comment at: clang-tools-extra/clang-doc/BitcodeReader.cpp:611-618 +Expected MaybeCode = Stream.ReadCode(); +if (!MaybeCode) { + // FIXME this drops the error on the floor. + consumeError(MaybeCode.takeError()); +} + +//

[PATCH] D63518: BitStream reader: propagate errors

jfb added a comment. In D63518#1558197 , @lhames wrote: > I haven't had a chance to audit the whole patch yet, but in general the error > suppression idioms are unsafe (though maybe no more so than the existing > code?). > > I would be inclined to audit

[PATCH] D63518: BitStream reader: propagate errors

jfb marked 8 inline comments as done. jfb added inline comments. Comment at: clang-tools-extra/clang-doc/BitcodeReader.cpp:527-529 + if (llvm::Error Err = readSubBlock(BlockOrCode, I)) { +if (llvm::Error Skipped = Stream.SkipBlock()) + return Skipped;

[PATCH] D63518: BitStream reader: propagate errors

jfb added a comment. I did a build of all LLVM monorepo projects, and only LLVM / clang / clang-tools-extra are impacted. I therefore ran tests as follows, and they all pass: rm -rf debug ; mkdir debug && (cd debug && cmake -G Ninja ../llvm -DLLVM_ENABLE_ASSERTIONS=ON

[PATCH] D63518: WIP BitStream reader: propagate errors

jfb added a comment. All of `check-llvm` now passes. I'll look at other users of this stuff in the monorepo, compile and run their tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63518/new/ https://reviews.llvm.org/D63518

[PATCH] D63518: WIP BitStream reader: propagate errors

jfb added a comment. `check-clang` now passes all tests, so the patch is pretty much ready to review. I'll get started on the other parts of LLVM that use this API. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63518/new/

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

jfb added inline comments. Comment at: lib/Sema/SemaExpr.cpp:10929 +// Do not diagnose if xor keyword is used. +if (ExprStr.find("xor") != llvm::StringRef::npos) + return; aaron.ballman wrote: > xbolva00 wrote: > > xbolva00 wrote: > > > jfb wrote: >

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

jfb added inline comments. Comment at: lib/Sema/SemaExpr.cpp:10929 +// Do not diagnose if xor keyword is used. +if (ExprStr.find("xor") != llvm::StringRef::npos) + return; xbolva00 wrote: > Quuxplusone wrote: > > xbolva00 wrote: > > > jfb wrote: > >

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

jfb added inline comments. Comment at: include/clang/Basic/DiagnosticGroups.td:508 def Varargs : DiagGroup<"varargs">; +def XorUsedAsPow : DiagGroup<"xor-used-as-pow">; Does this match the naming that GCC ended up with? Comment at:

[PATCH] D61879: WIP: Prototype of DSE optimizations for -ftrivial-auto-var-init

jfb added a comment. Hello! I'm still interested in this, have you been able to make any progress? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61879/new/ https://reviews.llvm.org/D61879 ___

[PATCH] D63518: WIP BitStream reader: propagate errors

jfb added a comment. In D63518#1550827 , @bruno wrote: > Hi JF. Thanks for working on this, nice improvement to error handling! > > The overall approach is pretty solid and should prevent a lot of red herring > while investigating hard to reproduce

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

jfb added a comment. In D63423#1550725 , @lebedev.ri wrote: > I've always been frustrated at how clang just gives up as soon as it sees a > macro. > At best that should be controlled with some command-line flag. This patch is not the place to change

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

jfb added a comment. In D63423#1550705 , @aaron.ballman wrote: > In D63423#1550697 , @jfb wrote: > > > What I meant with macros was that I don't think we should warn on: > > > > #define LEGIT(a, b) ({ work work

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

jfb added a comment. What I meant with macros was that I don't think we should warn on: #define LEGIT(a, b) ({ work work work; a ^ b; work work work; }) LEGIT(10, 5); If the constants are inline in the macros then sure, warn there. Basically: if you literally wrote `CONSTANT ^ CONSTANT`

[PATCH] D63518: WIP BitStream reader: propagate errors

jfb added a comment. This is still a work in progress. I still have to debug clang tests that fail, and compile / fix more than clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63518/new/ https://reviews.llvm.org/D63518

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

jfb added inline comments. Comment at: test/Sema/warn-xor-as-pow.c:45 + // expected-note@-1 {{replace expression with '(2) ^ 32' to silence this warning}} + res = 2 ^ 64; // expected-warning {{result of '2 ^ 64' is 66; maybe you mean '1<<64', but shift count >= width of

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

jfb added a comment. Why have `.c` and `.cpp` tests? Comment at: test/Sema/warn-xor-as-pow.c:43 + res = TWO_ULL ^ 16; + res = 2 ^ 32; // expected-warning {{result of '2 ^ 32' is 34; maybe you mean '1<<32', but shift count >= width of type}} + // expected-note@-1 {{replace

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

jfb added a comment. What does the suggested for for `2 ^ 32` look like? I hope it's not `1 << 32` :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 ___ cfe-commits mailing list

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

jfb added a comment. I think you want to avoid warning inside macros as well, say: #define AWESOME(x, y) ({ blah blah blah; x ^ y; blah }) AWESOME(2, 10); // probably not a bug Comment at: test/SemaCXX/warn-xor-as-pow.cpp:13 +res = a ^ b; +res = 2 ^ 0; +res

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

jfb added a comment. As a reference: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90885 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

jfb added a comment. In D62825#1542377 , @rsmith wrote: > (You might argue that it's ridiculous to require that `nullptr_t` have the > same size and alignment as `void*` but not have the same storage > representation as a null `void*`. I'd agree, and

[PATCH] D62962: Clang implementation of sizeless types

jfb added inline comments. Comment at: test/Sema/sizeless-1.c:66 + _Static_assert(__atomic_always_lock_free(1, _int8) == __atomic_always_lock_free(1, incomplete_ptr), ""); + _Static_assert(__atomic_always_lock_free(2, _int8) == __atomic_always_lock_free(2, incomplete_ptr),

[PATCH] D62960: SVE opaque type for C intrinsics demo

jfb added subscribers: rjmccall, jfb. jfb added a comment. Tests? Comment at: lib/AST/ItaniumMangle.cpp:2680 +break; +#include "clang/Basic/AArch64SVEACLETypes.def" } @rjmccall you probably should review this part. Repository: rC Clang CHANGES

[PATCH] D62962: Clang implementation of sizeless types

jfb added inline comments. Comment at: test/Sema/sizeless-1.c:66 + _Static_assert(__atomic_always_lock_free(1, _int8) == __atomic_always_lock_free(1, incomplete_ptr), ""); + _Static_assert(__atomic_always_lock_free(2, _int8) == __atomic_always_lock_free(2, incomplete_ptr),

[PATCH] D62962: Clang implementation of sizeless types

jfb added a comment. How do these types mangle? Comment at: test/Sema/sizeless-1.c:60 + (void)__atomic_is_lock_free(1, _int8); + (void)__atomic_always_lock_free(1, _int8); + What do you expect these to return? Repository: rC Clang CHANGES SINCE LAST

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

jfb added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:5447 + +case APValue::LValue: { + LValue LVal; rsmith wrote: > jfb wrote: > > rsmith wrote: > > > This will permit bitcasts from `nullptr_t`, providing a zero value. I > > > think

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

jfb added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:5447 + +case APValue::LValue: { + LValue LVal; rsmith wrote: > This will permit bitcasts from `nullptr_t`, providing a zero value. I think > that's wrong; the bit representation of

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

jfb added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9716 +def err_bit_cast_non_trivially_copyable : Error< + "__builtin_bit_cast %select{source|destination}0 type must be a trivially copyable">; +def err_bit_cast_type_size_mismatch : Error<

[PATCH] D61923: [GWP-ASan] Mutex implementation [2].

jfb added a comment. > In D61923#1502404 , @jfb wrote: > >> > We can't use `std::mutex` as we must be C-compatible >> >> Can you clarify what you mean by this? I don't understand. >> Have you asked on libcxx-dev? Is there interest in the libc++ community

[PATCH] D61923: [GWP-ASan] Mutex implementation [2].

jfb added a comment. In D61923#1502323 , @hctim wrote: > In D61923#1502245 , @jfb wrote: > > > Seems a shame to duplicate mutex again... Why can't use use the STL's > > version again? It doesn't allocate. > > > We

[PATCH] D60593: [GwpAsan] Introduce GWP-ASan.

jfb added a comment. In D60593#1498622 , @hctim wrote: > In D60593#1495428 , @gribozavr wrote: > > > What does GWP stand for? > > > Google Wide Profiling >

[PATCH] D61280: Variable auto-init: don't initialize aggregate padding of all aggregates

jfb added a comment. Follow-up test fix in http://llvm.org/r359636 Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61280/new/ https://reviews.llvm.org/D61280 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D61280: Variable auto-init: don't initialize aggregate padding of all aggregates

This revision was automatically updated to reflect the committed changes. Closed by commit rC359628: Variable auto-init: dont initialize aggregate padding of all aggregates (authored by jfb, committed by ). Changed prior to commit: https://reviews.llvm.org/D61280?vs=197188=197476#toc

[PATCH] D61280: Variable auto-init: don't initialize aggregate padding of all aggregates

jfb added a comment. This seems uncontroversial, I'm happy to make follow-up change if you have suggestions. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61280/new/ https://reviews.llvm.org/D61280 ___ cfe-commits

[PATCH] D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array

jfb added a comment. In D61165#1479937 , @rjmccall wrote: > Are you sure these are the right semantics for `nodestroy`? I think there's > a reasonable argument that we should not destroy previous elements of a > `nodestroy` array just because an

[PATCH] D61280: Variable auto-init: don't initialize aggregate padding of all aggregates

jfb added a comment. In D61280#1485115 , @rjmccall wrote: > I don't think the implication is supposed to be that padding is > zero-initialized or not depending on where in the aggregate it appears, but > it doesn't really matter, I don't think we're

[PATCH] D61280: Variable auto-init: don't initialize aggregate padding of all aggregates

jfb added a comment. In D61280#1485073 , @rjmccall wrote: > IIRC, C says *members* are initialized as if they were in static storage, > which might mean that their interior padding should be zeroed, but I don't > think says anything about the padding in

[PATCH] D61280: Variable auto-init: don't initialize aggregate padding of all aggregates

jfb created this revision. jfb added reviewers: glider, pcc, kcc. Herald added subscribers: cfe-commits, dexonsmith, jkorous. Herald added a project: clang. C guarantees that brace-init with fewer initializers than members in the aggregate will initialize the rest of the aggregate as-if it were

[PATCH] D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array

jfb added a comment. In D61165#1479937 , @rjmccall wrote: > Are you sure these are the right semantics for `nodestroy`? I think there's > a reasonable argument that we should not destroy previous elements of a > `nodestroy` array just because an

[PATCH] D55229: [COFF] Statically link certain runtime library functions

jfb added a comment. Looks like this might have broken bots: /Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/CodeGenCXX/runtime-dllstorage.cpp:121:26: error: CHECK-IA-DAG: expected string not found in input // CHECK-DYNAMIC-IA-DAG: declare

<    1   2   3   4   5   6   >