[PATCH] D64676: Support __seg_fs and __seg_gs on x86

2019-07-14 Thread JF Bastien via Phabricator via cfe-commits
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] D61106: [analyzer][UninitializedObjectChecker] PR41590: Regard _Atomic types as primitive

2019-04-25 Thread JF Bastien via Phabricator via cfe-commits
jfb added a comment. Have you checked what the C and C++ standards say about `_Atomic` / `std::atomic` initialization, and how they should actually be initialized by developers? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61106/new/

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

2019-04-25 Thread JF Bastien via Phabricator via cfe-commits
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

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

2019-07-01 Thread JF Bastien via Phabricator via cfe-commits
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] D60974: Clang IFSO driver action.

2019-07-01 Thread JF Bastien via Phabricator via cfe-commits
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.

2019-07-01 Thread JF Bastien via Phabricator via cfe-commits
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.

2019-07-01 Thread JF Bastien via Phabricator via cfe-commits
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.

2019-07-01 Thread JF Bastien via Phabricator via cfe-commits
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] D60974: Clang IFSO driver action.

2019-07-01 Thread JF Bastien via Phabricator via cfe-commits
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] D63518: BitStream reader: propagate errors

2019-06-25 Thread JF Bastien via Phabricator via cfe-commits
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

2019-06-25 Thread JF Bastien via Phabricator via cfe-commits
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] D59254: [RFC] Implementation of Clang randstruct

2019-07-01 Thread JF Bastien via Phabricator via cfe-commits
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] D59254: [RFC] Implementation of Clang randstruct

2019-07-02 Thread JF Bastien via Phabricator via cfe-commits
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

2019-07-02 Thread JF Bastien via Phabricator via cfe-commits
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] D66143: Don't use std::errc

2019-08-13 Thread JF Bastien via Phabricator via cfe-commits
jfb added a comment. In D66143#1627226 , @lebedev.ri wrote: > In D66143#1627221 , @ABataev wrote: > > > In D66143#1627195 , @thakis wrote: > > > > > As far as I know 4.8 is

[PATCH] D66045: Improve detection of same value in comparisons

2019-08-13 Thread JF Bastien via Phabricator via cfe-commits
jfb added inline comments. Comment at: test/SemaCXX/self-comparison.cpp:87 +}; +} // namespace member_tests rtrieu wrote: > jfb wrote: > > rtrieu wrote: > > > rtrieu wrote: > > > > jfb wrote: > > > > > The test only has `==`. Do other operators trigger? > > > >

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

2019-07-29 Thread JF Bastien via Phabricator via cfe-commits
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

2019-07-29 Thread JF Bastien via Phabricator via cfe-commits
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] D63139: [Diagnostics] Implement -Wswitch-unreachable

2019-07-29 Thread JF Bastien via Phabricator via cfe-commits
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] D65249: [NFC] use C++11 in AlignOf.h, remove AlignedCharArray

2019-07-29 Thread JF Bastien via Phabricator via cfe-commits
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] D63082: [Diagnostics] Added support for -Wint-in-bool-context

2019-07-29 Thread JF Bastien via Phabricator via cfe-commits
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] D65706: [docs] document -Weveything more betterer

2019-08-03 Thread JF Bastien via Phabricator via cfe-commits
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] D65706: [docs] document -Weveything more betterer

2019-08-03 Thread JF Bastien via Phabricator via cfe-commits
jfb updated this revision to Diff 213212. jfb added a comment. - Missing 'of' Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65706/new/ https://reviews.llvm.org/D65706 Files: clang/docs/UsersManual.rst Index: clang/docs/UsersManual.rst

[PATCH] D66240: Remove LVALUE / RVALUE workarounds

2019-08-14 Thread JF Bastien via Phabricator via cfe-commits
jfb added a comment. BTW, if this breaks stuff maybe it's better to do it one project at a time, and remove the helper at the very end. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66240/new/ https://reviews.llvm.org/D66240

[PATCH] D66240: Remove LVALUE / RVALUE workarounds

2019-08-14 Thread JF Bastien via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL368939: Remove LVALUE / RVALUE workarounds (authored by jfb, committed by ). Herald added a subscriber: kristina. Changed prior to commit: https://reviews.llvm.org/D66240?vs=215202=215275#toc

[PATCH] D66045: Improve detection of same value in comparisons

2019-08-10 Thread JF Bastien via Phabricator via cfe-commits
jfb added a comment. Does this work for unions as well? Comment at: test/Analysis/array-struct-region.cpp:31 + bool check() const { return this == this + 0; } + bool operator !() const { return this != this + 0; } Is this the only way? Or do casts also

[PATCH] D66046: Add new tautological compare warning for bitwise-or with a non-zero constant

2019-08-10 Thread JF Bastien via Phabricator via cfe-commits
jfb added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8161 + "bitwise or with non-zero value always evaluates to true">, + InGroup, DefaultIgnore; def warn_tautological_overlap_comparison : Warning< Why default ignore? CHANGES

[PATCH] D66043: Add to -Wparentheses case of bitwise-and ("&") and bitwise-or ("|") verses conditional operator ("?:")

2019-08-10 Thread JF Bastien via Phabricator via cfe-commits
jfb added inline comments. Comment at: test/Sema/parentheses.c:156 + + (void)(x ^ b ? 1 : 2); // no warning, ^ is often used as logical xor + (void)(x || b ? 1 : 2); // no warning, logical operator I don't understand why `^` is different. What do you mean by

[PATCH] D66044: Extend -Wtautological-overlap-compare to accept negative values and integer conversions

2019-08-10 Thread JF Bastien via Phabricator via cfe-commits
jfb added inline comments. Comment at: test/Sema/warn-unreachable.c:437 + if (~ 1) return; // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]] unaryOpNoFixit(); // expected-warning {{code will never be executed}} Why this change? CHANGES SINCE LAST ACTION

[PATCH] D66043: Add to -Wparentheses case of bitwise-and ("&") and bitwise-or ("|") verses conditional operator ("?:")

2019-08-12 Thread JF Bastien via Phabricator via cfe-commits
jfb added inline comments. Comment at: test/Sema/parentheses.c:156 + + (void)(x ^ b ? 1 : 2); // no warning, ^ is often used as logical xor + (void)(x || b ? 1 : 2); // no warning, logical operator rtrieu wrote: > jfb wrote: > > I don't understand why `^` is

[PATCH] D66046: Add new tautological compare warning for bitwise-or with a non-zero constant

2019-08-12 Thread JF Bastien via Phabricator via cfe-commits
jfb added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8161 + "bitwise or with non-zero value always evaluates to true">, + InGroup, DefaultIgnore; def warn_tautological_overlap_comparison : Warning< rtrieu wrote: > jfb wrote: > >

[PATCH] D66045: Improve detection of same value in comparisons

2019-08-12 Thread JF Bastien via Phabricator via cfe-commits
jfb added inline comments. Comment at: test/SemaCXX/self-comparison.cpp:87 +}; +} // namespace member_tests rtrieu wrote: > rtrieu wrote: > > jfb wrote: > > > The test only has `==`. Do other operators trigger? > > All the standard comparison operators will work

[PATCH] D66043: Add to -Wparentheses case of bitwise-and ("&") and bitwise-or ("|") verses conditional operator ("?:")

2019-08-12 Thread JF Bastien via Phabricator via cfe-commits
jfb accepted this revision. jfb added a comment. This revision is now accepted and ready to land. Herald added a subscriber: dexonsmith. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66043/new/ https://reviews.llvm.org/D66043 ___

[PATCH] D66044: Extend -Wtautological-overlap-compare to accept negative values and integer conversions

2019-08-12 Thread JF Bastien via Phabricator via cfe-commits
jfb added a comment. I haven't reviewed in depth, but generally this looks good. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66044/new/ https://reviews.llvm.org/D66044 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D66043: Add to -Wparentheses case of bitwise-and ("&") and bitwise-or ("|") verses conditional operator ("?:")

2019-08-12 Thread JF Bastien via Phabricator via cfe-commits
jfb added inline comments. Comment at: test/Sema/parentheses.c:156 + + (void)(x ^ b ? 1 : 2); // no warning, ^ is often used as logical xor + (void)(x || b ? 1 : 2); // no warning, logical operator rtrieu wrote: > jfb wrote: > > rtrieu wrote: > > > jfb

[PATCH] D66240: Remove LVALUE / RVALUE workarounds

2019-08-14 Thread JF Bastien via Phabricator via cfe-commits
jfb created this revision. Herald added subscribers: llvm-commits, cfe-commits, dexonsmith, jkorous. Herald added projects: clang, LLVM. LLVM_HAS_RVALUE_REFERENCE_THIS and LLVM_LVALUE_FUNCTION shouldn't be needed anymore because the minimum compiler versions support them. Repository: rG LLVM

[PATCH] D67023: Diagnose use of ATOMIC_VAR_INIT

2019-08-30 Thread JF Bastien via Phabricator via cfe-commits
jfb added a comment. Is atomic initialization now correct in all modes (including C++) without this macro? I don’t think we should diagnose until such a time because some code uses to macro to be portably correct. IIRC we only ended up fixing C++ in 20 with Nico’s paper (after Olivier and I

[PATCH] D67023: Diagnose use of ATOMIC_VAR_INIT

2019-09-03 Thread JF Bastien via Phabricator via cfe-commits
jfb added a comment. In D67023#1654704 , @aaron.ballman wrote: > In D67023#1654070 , @jfb wrote: > > > I refer you to http://wg21.link/p0883 > > I don’t think this diagnostic should be added to clang until p0883

[PATCH] D66822: Hardware cache line size builtins

2019-09-03 Thread JF Bastien via Phabricator via cfe-commits
jfb requested changes to this revision. jfb added a comment. This revision now requires changes to proceed. Sorry for the delayed response, I was on vacation. Thanks for tackling it! I don't think this is the approach I would take. From my dev meeting lightning talk I would instead: 1. Add to

[PATCH] D67023: Diagnose use of ATOMIC_VAR_INIT

2019-09-01 Thread JF Bastien via Phabricator via cfe-commits
jfb added a comment. I refer you to http://wg21.link/p0883 I don’t think this diagnostic should be added to clang until p0883 is fully implemented, even just for C. Otherwise we leave users with no portable way to do the right thing without diagnostic. CHANGES SINCE LAST ACTION

[PATCH] D64146: [Clang Interpreter] Initial patch for the constexpr interpreter

2019-09-12 Thread JF Bastien via Phabricator via cfe-commits
jfb accepted this revision. jfb added a comment. Sounds like this is ready to land again! Thanks for fixing everything. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64146/new/ https://reviews.llvm.org/D64146

[PATCH] D67399: [ARM] Follow AACPS standard for volatile bitfields

2019-09-13 Thread JF Bastien via Phabricator via cfe-commits
jfb added a comment. In D67399#1669038 , @dnsampaio wrote: > Indeed our main concern is regarding the access widths of loads. As mentioned > by @rjmccall, most volatile bitfields are used to perform memory mapped I/O, > and some hardware only support

[PATCH] D67399: [ARM] Follow AACPS standard for volatile bitfields

2019-09-10 Thread JF Bastien via Phabricator via cfe-commits
jfb added inline comments. Comment at: clang/test/CodeGen/aapcs-bitfield.c:541 // BE-NEXT:[[TMP0:%.*]] = getelementptr inbounds [[STRUCT_ST9:%.*]], %struct.st9* [[M:%.*]], i32 0, i32 0 +// BE-NEXT:[[BF_LOAD:%.*]] = load volatile i8, i8* [[TMP0]], align 4 // BE-NEXT:

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

2019-09-04 Thread JF Bastien via Phabricator via cfe-commits
jfb added inline comments. Comment at: lib/Sema/SemaExpr.cpp:11067 + + // Do not diagnose 2 ^ 64, but allow special case (2 ^ 64) - 1. + if (SubLHS && SubRHS && (LeftSideValue != 2 || RightSideValue != 64)) xbolva00 wrote: > xbolva00 wrote: > > aaron.ballman

[PATCH] D55895: NFC: simplify Darwin environment handling

2019-07-30 Thread JF Bastien via Phabricator via cfe-commits
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] D63082: [Diagnostics] Added support for -Wint-in-bool-context

2019-07-30 Thread JF Bastien via Phabricator via cfe-commits
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

2019-07-30 Thread JF Bastien via Phabricator via cfe-commits
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

2019-07-30 Thread JF Bastien via Phabricator via cfe-commits
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

2019-07-30 Thread JF Bastien via Phabricator via cfe-commits
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

2019-07-30 Thread JF Bastien via Phabricator via cfe-commits
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] D64742: Allow using -ftrivial-auto-var-init=zero in C mode without extra flags

2019-07-30 Thread JF Bastien via Phabricator via cfe-commits
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] D65249: [NFC] use C++11 in AlignOf.h, remove AlignedCharArray

2019-07-30 Thread JF Bastien via Phabricator via cfe-commits
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] D65458: [NFC] Remove LLVM_ALIGNAS

2019-07-30 Thread JF Bastien via Phabricator via cfe-commits
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

2019-07-30 Thread JF Bastien via Phabricator via cfe-commits
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

2019-07-30 Thread JF Bastien via Phabricator via cfe-commits
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] D65706: [docs] Better documentation for -Weverything

2019-08-04 Thread JF Bastien via Phabricator via cfe-commits
jfb updated this revision to Diff 213278. jfb marked 5 inline comments as done. jfb added a comment. - Address Aaron's comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65706/new/ https://reviews.llvm.org/D65706 Files:

[PATCH] D65706: [docs] Better documentation for -Weverything

2019-08-04 Thread JF Bastien via Phabricator via cfe-commits
jfb added inline comments. Comment at: clang/docs/UsersManual.rst:999-1000 +diagnostics contradict each other, users of :option:`-Weverything` therefore +often disable many diagnostics such as :option:`-Wno-c++98-compat` +:option:`-Wno-c++-compat`. +

[PATCH] D65846: Improve error message from FrontendAction

2019-08-06 Thread JF Bastien via Phabricator via cfe-commits
jfb planned changes to this revision. jfb added a comment. There's a failure in `clang/test/Index/pch-from-libclang.c`. @thakis disabled that test for all but Darwin in r352809, and with my change I now get: Internal compiler error: LFS error for

[PATCH] D65846: Improve error message from FrontendAction

2019-08-06 Thread JF Bastien via Phabricator via cfe-commits
jfb created this revision. Herald added subscribers: cfe-commits, dexonsmith. Herald added a project: clang. jfb planned changes to this revision. jfb added a comment. There's a failure in `clang/test/Index/pch-from-libclang.c`. @thakis disabled that test for all but Darwin in r352809, and with

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

2019-08-07 Thread JF Bastien via Phabricator via cfe-commits
jfb updated this revision to Diff 213980. jfb marked 3 inline comments as done. jfb added a comment. - Don't report_fatal_error in code that can be called from LLDB Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65545/new/

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

2019-08-07 Thread JF Bastien via Phabricator via cfe-commits
jfb added a comment. Updated. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65545/new/ https://reviews.llvm.org/D65545 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D65846: Improve error message from FrontendAction

2019-08-07 Thread JF Bastien via Phabricator via cfe-commits
jfb added a comment. My current guess is that this part of the test: c-index-test -write-pch %t.h.pch %s -fmodules -fmodules-cache-path=%t.mcp -Xclang -triple -Xclang x86_64-apple-darwin Is expected to generate the `unknown type name` error, but when than happens it ignores

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

2019-07-31 Thread JF Bastien via Phabricator via cfe-commits
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] D65545: Handle some fs::remove failures

2019-08-01 Thread JF Bastien via Phabricator via cfe-commits
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

2019-08-01 Thread JF Bastien via Phabricator via cfe-commits
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] D65249: [NFC] use C++11 in AlignOf.h

2019-07-26 Thread JF Bastien via Phabricator via cfe-commits
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] D64146: [ConstExprPreter][WIP] Initial patch for the constexpr interpreter

2019-07-29 Thread JF Bastien via Phabricator via cfe-commits
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] D65706: [docs] Better documentation for -Weverything

2019-08-05 Thread JF Bastien via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL367889: [docs] document -Weveything more betterer (authored by jfb, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D65706: [docs] Better documentation for -Weverything

2019-08-05 Thread JF Bastien via Phabricator via cfe-commits
jfb marked an inline comment as done. jfb added inline comments. Comment at: clang/docs/UsersManual.rst:999-1000 +diagnostics contradict each other, users of :option:`-Weverything` therefore +often disable many diagnostics such as :option:`-Wno-c++98-compat`

[PATCH] D65986: Allow setting the VFS to 'real' mode instead of default 'physical'

2019-08-08 Thread JF Bastien via Phabricator via cfe-commits
jfb updated this revision to Diff 214296. jfb marked an inline comment as done. jfb added a comment. - Address more comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65986/new/ https://reviews.llvm.org/D65986 Files:

[PATCH] D65986: Allow setting the VFS to 'real' mode instead of default 'physical'

2019-08-08 Thread JF Bastien via Phabricator via cfe-commits
jfb updated this revision to Diff 214292. jfb added a comment. - Undo whitespaces change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65986/new/ https://reviews.llvm.org/D65986 Files: clang/include/clang/Driver/Driver.h

[PATCH] D65986: Allow setting the VFS to 'real' mode instead of default 'physical'

2019-08-08 Thread JF Bastien via Phabricator via cfe-commits
jfb created this revision. jfb added reviewers: JDevlieghere, bruno. Herald added subscribers: cfe-commits, dexonsmith, jkorous. Herald added a project: clang. The motivation for 'physical' mode in D58169 was pretty sensible, but it has one unfortunate

[PATCH] D65986: Allow setting the VFS to 'real' mode instead of default 'physical'

2019-08-08 Thread JF Bastien via Phabricator via cfe-commits
jfb marked an inline comment as done. jfb added inline comments. Comment at: clang/test/Driver/vfsmode.py:4 + +# UNSUPPORTED: system-windows + I'm not sure what the best way to test this on Windows would be, and without a machine handy I can't really test this

[PATCH] D65986: Allow setting the VFS to 'real' mode instead of default 'physical'

2019-08-08 Thread JF Bastien via Phabricator via cfe-commits
jfb updated this revision to Diff 214291. jfb marked 6 inline comments as done. jfb added a comment. - Address comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65986/new/ https://reviews.llvm.org/D65986 Files:

[PATCH] D65986: Allow setting the VFS to 'real' mode instead of default 'physical'

2019-08-08 Thread JF Bastien via Phabricator via cfe-commits
jfb added inline comments. Comment at: clang/lib/Driver/Driver.cpp:172 + case VFSMode::Unknown: +if (!this->VFS) { + LLVM_FALLTHROUGH; JDevlieghere wrote: > Is this really necessary? the `Driver` ctor takes a `VFS` parameter which some tools set

[PATCH] D65986: Allow setting the VFS to 'real' mode instead of default 'physical'

2019-08-08 Thread JF Bastien via Phabricator via cfe-commits
jfb marked an inline comment as done. jfb added inline comments. Comment at: clang/lib/Driver/Driver.cpp:172 + case VFSMode::Unknown: +if (!this->VFS) { + LLVM_FALLTHROUGH; JDevlieghere wrote: > jfb wrote: > > JDevlieghere wrote: > > > Is this really

[PATCH] D65986: Allow setting the VFS to 'real' mode instead of default 'physical'

2019-08-08 Thread JF Bastien via Phabricator via cfe-commits
jfb updated this revision to Diff 214298. jfb marked an inline comment as done. jfb added a comment. - Address more comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65986/new/ https://reviews.llvm.org/D65986 Files:

[PATCH] D65986: Allow setting the VFS to 'real' mode instead of default 'physical'

2019-08-09 Thread JF Bastien via Phabricator via cfe-commits
jfb marked 2 inline comments as done. jfb added inline comments. Comment at: clang/test/Driver/vfsmode.py:4 + +# UNSUPPORTED: system-windows + labath wrote: > jfb wrote: > > I'm not sure what the best way to test this on Windows would be, and > > without a

[PATCH] D65986: Allow setting the VFS to 'real' mode instead of default 'physical'

2019-08-09 Thread JF Bastien via Phabricator via cfe-commits
jfb marked an inline comment as done. jfb added a comment. In D65986#1622447 , @sammccall wrote: > Tagging D62271 and @Bigcheese as this was > the change that switched the default in Driver. (My motivation for D58169 >

[PATCH] D65986: Allow setting the VFS to 'real' mode instead of default 'physical'

2019-08-09 Thread JF Bastien via Phabricator via cfe-commits
jfb marked 4 inline comments as done. jfb added inline comments. Comment at: clang/include/clang/Driver/Options.td:2010 HelpText<"Overlay the virtual filesystem described by file over the real file system">; +def ivfsmode : Joined<["-"], "ivfsmode=">, Group,

[PATCH] D65986: Allow setting the VFS to 'real' mode instead of default 'physical'

2019-08-09 Thread JF Bastien via Phabricator via cfe-commits
jfb updated this revision to Diff 214397. jfb added a comment. - Document asserts. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65986/new/ https://reviews.llvm.org/D65986 Files: clang/include/clang/Driver/Driver.h

[PATCH] D65986: Allow setting the VFS to 'real' mode instead of default 'physical'

2019-08-09 Thread JF Bastien via Phabricator via cfe-commits
jfb marked 2 inline comments as done. jfb added inline comments. Comment at: clang/test/Driver/vfsmode.py:47 +# exceed PATH_MAX. +assert os.path.exists(file_relative_path) +assert not os.path.exists(os.path.join(absolute, filename)) bruno wrote: > If you want to

[PATCH] D65986: Allow setting the VFS to 'real' mode instead of default 'physical'

2019-08-09 Thread JF Bastien via Phabricator via cfe-commits
jfb marked an inline comment as done. jfb added a comment. In D65986#1623283 , @Bigcheese wrote: > This fix works, but we could also use openat to get around max path length > issues. Windows also has an API that can be used similarly. Given the type

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

2019-08-05 Thread JF Bastien via Phabricator via cfe-commits
jfb updated this revision to Diff 213497. jfb marked 8 inline comments as done. jfb added a comment. - Return llvm::Error from ASTUnit::Save - Update per comments. - Address Volodymyr's comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

2019-08-05 Thread JF Bastien via Phabricator via cfe-commits
jfb added inline comments. Comment at: clang/lib/Frontend/CompilerInstance.cpp:1444-1445 // Remove the file. - llvm::sys::fs::remove(File->path()); + if ((EC = llvm::sys::fs::remove(File->path( +break; vsapsai wrote: > jfb wrote: >

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

2019-08-05 Thread JF Bastien via Phabricator via cfe-commits
jfb marked 2 inline comments as done. jfb added inline comments. Comment at: clang/lib/Frontend/PrecompiledPreamble.cpp:107-111 TemporaryFiles::~TemporaryFiles() { llvm::MutexGuard Guard(Mutex); for (const auto : Files) -llvm::sys::fs::remove(File.getKey()); +if

[PATCH] D66038: [Support] heavyweight_hardware_concurrency uses affinity when counting cores fails, and never returns 0

2019-08-09 Thread JF Bastien via Phabricator via cfe-commits
jfb added a comment. The name of this patch is wrong? Or rather, it's 2 in 1? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66038/new/ https://reviews.llvm.org/D66038 ___ cfe-commits mailing list

[PATCH] D65986: Allow setting the VFS to 'real' mode instead of default 'physical'

2019-08-09 Thread JF Bastien via Phabricator via cfe-commits
jfb updated this revision to Diff 214448. jfb marked an inline comment as done. jfb added a comment. - Rename option. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65986/new/ https://reviews.llvm.org/D65986 Files:

[PATCH] D65986: Allow setting the VFS to 'real' mode instead of default 'physical'

2019-08-09 Thread JF Bastien via Phabricator via cfe-commits
jfb marked an inline comment as done. jfb added inline comments. Comment at: clang/include/clang/Driver/Options.td:2012 + HelpText<"Use the virtual file system in 'real' mode, or 'physical' mode. In 'real' mode the working directory is linked to the process' working directory.

[PATCH] D65986: Allow setting the VFS to 'real' mode instead of default 'physical'

2019-08-09 Thread JF Bastien via Phabricator via cfe-commits
jfb updated this revision to Diff 214477. jfb added a comment. - Also update function name Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65986/new/ https://reviews.llvm.org/D65986 Files: clang/include/clang/Driver/Driver.h

[PATCH] D65986: Allow setting the VFS to 'real' mode instead of default 'physical'

2019-08-09 Thread JF Bastien via Phabricator via cfe-commits
jfb abandoned this revision. jfb added a comment. Let's do the better fix that Michael suggested. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65986/new/ https://reviews.llvm.org/D65986 ___

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

2019-07-23 Thread JF Bastien via Phabricator via cfe-commits
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] D64666: [Sema] Enable -Wimplicit-float-conversion for integral to floating point precision loss

2019-07-23 Thread JF Bastien via Phabricator via cfe-commits
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] D64742: Allow using -ftrivial-auto-var-init=zero in C mode without extra flags

2019-07-15 Thread JF Bastien via Phabricator via cfe-commits
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] D64666: [Sema] Enable -Wimplicit-float-conversion for integral to floating point precision loss

2019-07-24 Thread JF Bastien via Phabricator via cfe-commits
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

2019-07-24 Thread JF Bastien via Phabricator via cfe-commits
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] D65254: Allow prefetching from non-zero address spaces

2019-07-24 Thread JF Bastien via Phabricator via cfe-commits
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

2019-07-24 Thread JF Bastien via Phabricator via cfe-commits
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] D65249: [NFC] use C++11 in AlignOf.h

2019-07-24 Thread JF Bastien via Phabricator via cfe-commits
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] D65254: Allow prefetching from non-zero address spaces

2019-07-25 Thread JF Bastien via Phabricator via cfe-commits
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

<    1   2   3   4   5   6   >