[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

ymandel added inline comments. Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:196 +/// Joins multiple rules into a single rule that applies the first rule in +/// `Rules` whose pattern matches a given node. All of the rules must use the +/// same kind of

[PATCH] D61335: [LibTooling] Add support to Transformer for composing rules as an ordered choice.

ymandel updated this revision to Diff 199476. ymandel marked 18 inline comments as done. ymandel added a comment. Herald added a subscriber: jfb. responded to comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61335/new/

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov updated this revision to Diff 199486. ilya-biryukov added a comment. - Remove the now redundant 'public:' spec. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59885/new/ https://reviews.llvm.org/D59885 Files:

r360699 - Restore test files accidentally deleted in r354839

Author: rnk Date: Tue May 14 11:51:07 2019 New Revision: 360699 URL: http://llvm.org/viewvc/llvm-project?rev=360699=rev Log: Restore test files accidentally deleted in r354839 I think there must be a bug in git-llvm causing parent directories to be deleted when the diff deletes files in a

[PATCH] D61281: [clang-format] Fixed self assignment

xbolva00 updated this revision to Diff 199494. xbolva00 added a comment. removed line CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61281/new/ https://reviews.llvm.org/D61281 Files: lib/Format/FormatTokenLexer.cpp Index: lib/Format/FormatTokenLexer.cpp

[PATCH] D61790: [C++20] add consteval specifier

rsmith added a comment. Thanks for working on this! Comments on the general approach: - We should only evaluate each immediate invocation once (this will become essential once we start supporting reflection -- and particularly operations that mutate the AST -- inside `consteval` functions). -

[PATCH] D36357: Added a better diagnostic when using the delete operator with lambdas

rsmith added a comment. In D36357#1500949 , @Rakete wrote: > How should I do this? Do I just skip to the next `}`, or also take into > account any additional scopes? Also does this mean that I skip and then > revert, because that seems pretty

r360703 - Temporarily revert "Change -gz and -Wa, --compress-debug-sections to use gABI compression (SHF_COMPRESSED)"

Author: echristo Date: Tue May 14 12:40:42 2019 New Revision: 360703 URL: http://llvm.org/viewvc/llvm-project?rev=360703=rev Log: Temporarily revert "Change -gz and -Wa,--compress-debug-sections to use gABI compression (SHF_COMPRESSED)" This affects users of older (pre 2.26) binutils in such a

r360701 - Update ASTMerge FileCheck test expectations

Author: rnk Date: Tue May 14 12:02:39 2019 New Revision: 360701 URL: http://llvm.org/viewvc/llvm-project?rev=360701=rev Log: Update ASTMerge FileCheck test expectations I belive many of these diagnostics changed from errors to warnings in r357394. I've simply mechanically updated the tests, but

[PATCH] D60162: [ThinLTO] Add module flags for TargetLibraryInfoImpl and use in LTO backends

tejohnson marked 2 inline comments as done. tejohnson added inline comments. Comment at: clang/test/CodeGen/svml-calls.ll:16 + +define void @sin_f64(double* nocapture %varray) { +; CHECK-LABEL: @sin_f64( steven_wu wrote: > Personally, I think codegen tests like

Re: r359960 - Reduce amount of work ODR hashing does.

On Tue, May 7, 2019 at 7:40 PM Richard Trieu wrote: > > > From: David Blaikie > Date: Mon, May 6, 2019 at 4:39 PM > To: Richard Trieu > Cc: cfe-commits > >> On Mon, May 6, 2019 at 4:24 PM Richard Trieu wrote: >> > >> > There was no cycle for this crash. >> >> Oh, yeah, didn't mean to imply

[PATCH] D61689: Change -gz and -Wa,--compress-debug-sections to use gABI compression (SHF_COMPRESSED)

rupprecht added a comment. So, while I think this is an //entirely// reasonable assumption in most cases, it's also not one that provides any kind of workaround for the few cases where it's not universally true. - As mentioned in the patch, this effectively changes the default from

[PATCH] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

torbjoernk updated this revision to Diff 199496. torbjoernk added a comment. minor rewording CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61827/new/ https://reviews.llvm.org/D61827 Files: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

rsmith added a comment. In D59885#1501774 , @ilya-biryukov wrote: > The suggested approach looks promising. The difference seems to be within the > noise levels on my machine: :) Awesome! > I guess it would make more sense to do the following before

r360705 - Fix ASTMerge/namespace/test.cpp after r360701

Author: rnk Date: Tue May 14 13:01:03 2019 New Revision: 360705 URL: http://llvm.org/viewvc/llvm-project?rev=360705=rev Log: Fix ASTMerge/namespace/test.cpp after r360701 Modified: cfe/trunk/test/ASTMerge/namespace/test.cpp Modified: cfe/trunk/test/ASTMerge/namespace/test.cpp URL:

[PATCH] D61914: [Support][Test] Time profiler: add regression test

anton-afanasyev created this revision. anton-afanasyev added a reviewer: thakis. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. Add output to `llvm::errs()` when `-ftime-trace` option is enabled, add regression test checking this option works as expected.

[PATCH] D61909: Add Clang shared library with C++ exports

winksaville added a comment. You mention that you're using OBJECT libraries so objects aren't built mutliples times and in my current tests the number of steps increased by only 3, it went from 4353 to 4356, when using this patch, which is great! What I see in my testing of the patch is that

[PATCH] D61281: [clang-format] Fixed self assignment

MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. please simply remove line 249 Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61281/new/ https://reviews.llvm.org/D61281

Re: r360637 - PR41817: Fix regression in r359260 that caused the MS compatibility

On Tue, 14 May 2019, 03:09 Hans Wennborg via cfe-commits, < cfe-commits@lists.llvm.org> wrote: > Actually, we didn't notice r359260 in Chromium, however this one > (r360637) caused Clang to start asserting during our builds > (https://bugs.chromium.org/p/chromium/issues/detail?id=962840), here's

[PATCH] D44100: [ASTImporter] Reorder fields after structure import is finished

shafik added a comment. Done with first round. Comment at: lib/AST/ASTImporter.cpp:1643 +if (!ImportedOrErr) { + // For RecordDecls, failed import of a field will break the layout of the + // structure. Handle it as an error. What cases are

[PATCH] D61909: Add Clang shared library with C++ exports

tstellar added a comment. Thanks for working on this, I have wanted something like this for a while. It would also be nice to have a CLANG_LINK_CLANG_DYLIB option like we have for llvm, but this can be a follow on patch, and I would be happy to help with this if needed.

[PATCH] D61874: [clang-tidy] Fix invalid fixit for readability-static-accessed-through-instance (bug 40544)

This revision was automatically updated to reflect the committed changes. Closed by commit rL360698: [clang-tidy] Fix invalid fixit for readability-static-accessed-through-instance… (authored by mgehre, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits.

[clang-tools-extra] r360698 - [clang-tidy] Fix invalid fixit for readability-static-accessed-through-instance (bug 40544)

Author: mgehre Date: Tue May 14 11:23:10 2019 New Revision: 360698 URL: http://llvm.org/viewvc/llvm-project?rev=360698=rev Log: [clang-tidy] Fix invalid fixit for readability-static-accessed-through-instance (bug 40544) Summary: Fixed https://bugs.llvm.org/show_bug.cgi?id=40544 Before, we would

[PATCH] D61817: [analyzer] Add a prunable note for skipping virtual base initializers in subclasses.

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. What about "the most derived class" or "a superclass" instead of "the superclass"? (https://en.cppreference.com/w/cpp/language/derived_class) May the sentence is a little bit too long. It

[PATCH] D60162: [ThinLTO] Add module flags for TargetLibraryInfoImpl and use in LTO backends

tejohnson marked an inline comment as done. tejohnson added inline comments. Comment at: llvm/lib/LTO/LTOBackend.cpp:221 +static TargetLibraryInfoImpl *createTLII(Module , TargetMachine *TM) { + TargetLibraryInfoImpl *TLII = tejohnson wrote: > steven_wu

Re: r360452 - Replace 'REQUIRES: nozlib' with '!zlib' because we don't need two ways

Fair enough - since they're already there I don't feel super serious about converging on one (though I probably wouldn't've been in favor of adding a second spelling at the point it was proposed). On Tue, May 14, 2019 at 8:03 AM wrote: > > There's no practical difference. I view it as a matter

[PATCH] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

torbjoernk updated this revision to Diff 199483. torbjoernk edited the summary of this revision. torbjoernk added a comment. Herald added a subscriber: arphaman. Addressed Roman Lebedev's comment regarding reference to `NOLINT` in the docs. As I don't have commit rights, I'd be grateful someone

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov added a comment. The suggested approach looks promising. The difference seems to be within the noise levels on my machine: Before the change (baseline upstream revision): Time (mean ± σ): 159.1 ms ± 8.7 ms[User: 138.3 ms, System: 20.6 ms] Range (min … max):

[PATCH] D59885: [Lex] Allow to consume tokens while preprocessing

ilya-biryukov updated this revision to Diff 199485. ilya-biryukov added a comment. - Use a flag inside clang::Token instead Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59885/new/ https://reviews.llvm.org/D59885 Files:

[PATCH] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

hintonda added a comment. I'll be happy to commit for you, but will give it till tomorrow just to make sure no one else has any additional comments. Thanks again! Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst:262 +not been used on code with

Re: r360403 - Change -gz and -Wa, --compress-debug-sections to use gABI compression (SHF_COMPRESSED)

Hi Ray, I've temporarily reverted this here: echristo@jhereg ~/s/llvm-project> git llvm push Pushing 1 commit: fda79815a33 Temporarily revert "Change -gz and -Wa,--compress-debug-sections to use gABI compression (SHF_COMPRESSED)" Sendingcfe/trunk/docs/ReleaseNotes.rst Sending

[PATCH] D61861: DeleteNullPointerCheck now deletes until the end brace of the condition

J-Camilleri added a comment. In D61861#1500868 , @madsravn wrote: > Looks good to me :) Thanks for the review and suggestion, who should I contact now in order to push the patch? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D61861: DeleteNullPointerCheck now deletes until the end brace of the condition

madsravn added a comment. In D61861#1500900 , @J-Camilleri wrote: > In D61861#1500868 , @madsravn wrote: > > > Looks good to me :) > > > Thanks for the review and suggestion, who should I contact now in order to >

[PATCH] D61350: [clang-tidy] New check calling out uses of +new in Objective-C code

mwyman updated this revision to Diff 199375. mwyman added a comment. Bah, previous changes not caught in Git commit; switching back and forth between Git/Mercurial makes for some mix-ups, I guess. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61350/new/ https://reviews.llvm.org/D61350

[PATCH] D61861: DeleteNullPointerCheck now deletes until the end brace of the condition

madsravn accepted this revision. madsravn added a comment. This revision is now accepted and ready to land. Looks good to me :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61861/new/ https://reviews.llvm.org/D61861

[PATCH] D36357: Added a better diagnostic when using the delete operator with lambdas

Rakete marked 3 inline comments as done. Rakete added a comment. > we could perform a tentative parse and skip to the } of the lambda. How should I do this? Do I just skip to the next `}`, or also take into account any additional scopes? Also does this mean that I skip and then revert,

[PATCH] D61851: [clang-tidy] New option for misc-throw-by-value-catch-by-reference

lebedev.ri added inline comments. Comment at: docs/ReleaseNotes.rst:182-184 +- New options `WarnOnLargeObject` and `MaxSize` for check + :doc:`misc-throw-by-value-catch-by-reference + ` check to warn for check :doc:`..`<..> check (repeated 'check' word)

[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] D61841: [clangd] Respect WarningsAsErrors configuration for clang-tidy

nridge updated this revision to Diff 199552. nridge marked 2 inline comments as done. nridge added a comment. Address review comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61841/new/ https://reviews.llvm.org/D61841 Files:

[PATCH] D61804: Support building shared and static libraries simultaneously

winksaville added a comment. I did some quick testing. I used cmake and ninja to build `llvm` and enabled `clang;lld;compiler-rt` subprojects: $ cd build-beanz-clang-shlib-2-add-debug-BUILD-LINK_DYNLIB-ON $ cmake ../llvm -G Ninja '-DLLVM_ENABLE_PROJECTS=clang;lld;compiler-rt'

[PATCH] D36357: Added a better diagnostic when using the delete operator with lambdas

Rakete marked 2 inline comments as done. Rakete added a comment. In D36357#1501961 , @rsmith wrote: > In D36357#1500949 , @Rakete > wrote: > > > How should I do this? Do I just skip to the next `}`, or

RE: r360452 - Replace 'REQUIRES: nozlib' with '!zlib' because we don't need two ways

> -Original Message- > From: David Blaikie [mailto:dblai...@gmail.com] > Sent: Tuesday, May 14, 2019 1:47 PM > To: Robinson, Paul > Cc: cfe-commits > Subject: Re: r360452 - Replace 'REQUIRES: nozlib' with '!zlib' because we > don't need two ways > > Fair enough - since they're already

[PATCH] D61709: [NewPM] Port HWASan and Kernel HWASan

leonardchan added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:179 - bool runOnFunction(Function ) override; - bool doInitialization(Module ) override; + bool instrumentFunction(Function ); + void initializeWithModule(Module );

[PATCH] D61634: [clang/llvm] Allow efficient implementation of libc's memory functions in C/C++

efriedma added a comment. > I have a related patch that turns -fno-builtin* options into module flags Do you have any opinion on representing -fno-builtin using a module flag vs. a function attribute in IR? It seems generally more flexible and easier to reason about a function attribute from

r360707 - [NewPM] Port HWASan and Kernel HWASan

Author: leonardchan Date: Tue May 14 14:17:21 2019 New Revision: 360707 URL: http://llvm.org/viewvc/llvm-project?rev=360707=rev Log: [NewPM] Port HWASan and Kernel HWASan Port hardware assisted address sanitizer to new PM following the same guidelines as msan and tsan. Changes: - Separate

[PATCH] D61366: [libcxx] [test] Don't assert that moved-from containers with non-POCMA allocators are empty.

BillyONeal added a comment. In D61366#1502073 , @EricWF wrote: > I'm not sure I agree with your design decision, but this patch LGTM. I wouldn't object to a standards change to make this the case; though it is suboptimal to destroy all the elements

[PATCH] D61364: [libcxx] [test] Fix incorrect allocator::construct assertions in associative container tests

ldionne added a comment. > When the insert(P&&) is called, it delegates to emplace, which only gets > Cpp17EmplaceConstructible from the supplied parameters, not > Cpp17EmplaceConstructible from add_const_t of the parameters. Thus, libcxx's > test is asserting a condition the standard

[PATCH] D61366: [libcxx] [test] Don't assert that moved-from containers with non-POCMA allocators are empty.

Quuxplusone added a comment. >> Are you not allowed to move the containers elements in this case? > > Correct. The allocator is not POCMA and not equal, so it's functionally the > same as doing `assign(make_move_iterator(begin()), > make_move_iterator(end()))`. In case the clarification helps

[PATCH] D61634: [clang/llvm] Allow efficient implementation of libc's memory functions in C/C++

tejohnson added a comment. In D61634#1502138 , @hfinkel wrote: > In D61634#1502043 , @efriedma wrote: > > > > I have a related patch that turns -fno-builtin* options into module flags > > > > Do you have any

[PATCH] D61364: [libcxx] [test] Fix incorrect allocator::construct assertions in associative container tests

EricWF added a comment. From Billy and my last discussion, I think we came to the agreement that it's not clear exactly what the "standard behavior" is. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61364/new/ https://reviews.llvm.org/D61364

r360737 - [analyzer] MIGChecker: Add support for os_ref_retain().

Author: dergachev Date: Tue May 14 18:19:19 2019 New Revision: 360737 URL: http://llvm.org/viewvc/llvm-project?rev=360737=rev Log: [analyzer] MIGChecker: Add support for os_ref_retain(). Suppress MIG checker false positives that occur when the programmer increments the reference count before

[PATCH] D57858: [analyzer] Add a new frontend flag to display all checker options

NoQ added a comment. http://lists.llvm.org/pipermail/cfe-dev/2019-May/062298.html CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57858/new/ https://reviews.llvm.org/D57858 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D60953: [clangd] Respect clang-tidy suppression comments

nridge updated this revision to Diff 199551. nridge marked 9 inline comments as done. nridge added a comment. Address review comments, except for the derived class one, about which I have a question Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D60953: [clangd] Respect clang-tidy suppression comments

nridge added inline comments. Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:243 +// future). +class ClangdDiagnosticConsumer : public StoreDiags { +public: sammccall wrote: > Thanks, this is much cleaner. > > I think we don't need the subclass at all

[PATCH] D61909: Add Clang shared library with C++ exports

winksaville added a comment. I did some quick testing. I used cmake and ninja to build `llvm` and enabled `clang;lld;compiler-rt` subprojects: $ cd build-beanz-clang-shlib-2-add-debug-BUILD-LINK_DYNLIB-ON $ cmake ../llvm -G Ninja '-DLLVM_ENABLE_PROJECTS=clang;lld;compiler-rt'

[PATCH] D61909: Add Clang shared library with C++ exports

winksaville added a comment. Questions: - Should we only build `libclang_shared.so` if `LLVM_BUILD_LLVM_DYLIB` is ON? - Should we use link clang-9 to libclang_shared.so when `LLVM_LINK_LLVM_DYLIB` is ON? - Or maybe we should define `BUILD_CLANG_DYLIB` and `LINK_CLANG_DYLIB` or ... ?

[PATCH] D61841: [clangd] Respect WarningsAsErrors configuration for clang-tidy

nridge added inline comments. Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:346 + std::string CheckName = CTContext->getCheckName(Info.getID()); + if (!CheckName.empty() && WarningAsErrorFilter->contains(CheckName)) { +Level = DiagnosticsEngine::Error;

[PATCH] D61804: Support building shared and static libraries simultaneously

winksaville added a comment. Questions: - Should we only build `libclang_shared.so` if `LLVM_BUILD_LLVM_DYLIB` is ON? - Should we use link clang-9 to libclang_shared.so when `LLVM_LINK_LLVM_DYLIB` is ON? - Or maybe we should define `BUILD_CLANG_DYLIB` and `LINK_CLANG_DYLIB` or ... ?

[PATCH] D61804: Support building shared and static libraries simultaneously

winksaville added a comment. Sorry, the two previous comments were meant for D61909 and I've moved them. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61804/new/ https://reviews.llvm.org/D61804

[PATCH] D61364: [libcxx] [test] Fix incorrect allocator::construct assertions in associative container tests

BillyONeal added a comment. In D61364#1502172 , @ldionne wrote: > Do you know *why* the tests are failing with libc++? I see this overload for > `insert` and it seems like it should be a better match? No, I haven't investigated. I avoid looking at

[PATCH] D61366: [libcxx] [test] Don't assert that moved-from containers with non-POCMA allocators are empty.

BillyONeal added a comment. In D61366#1502170 , @Quuxplusone wrote: > you are indeed allowed to //move// the //elements// And indeed we *must* do that :). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61366/new/

[PATCH] D61814: [CFG] NFC: Remove implicit conversion from CFGTerminator to Stmt *, make it a variant class instead.

Szelethus accepted this revision. Szelethus added a comment. LGTM! Comment at: clang/include/clang/Analysis/CFG.h:514 + CFGTerminator() { assert(!isValid()); } + CFGTerminator(Stmt *S, Kind K = StmtBranch) : Data(S, K) {} Can we add something to check that

[PATCH] D61364: [libcxx] [test] Fix incorrect allocator::construct assertions in associative container tests

BillyONeal added a comment. In D61364#1502200 , @EricWF wrote: > From Billy and my last discussion, I think we came to the agreement that it's > not clear exactly what the "standard behavior" is. No, I don't think so. I think there was agreement that

[PATCH] D61364: [libcxx] [test] Fix incorrect allocator::construct assertions in associative container tests

EricWF added inline comments. Comment at: test/std/containers/map_allocator_requirement_test_templates.h:236 const ValueTp v(42, 1); -cc->expect(); +cc->expect(); It ret = c.insert(c.end(), std::move(v)); I really think the current behavior

[PATCH] D61817: [analyzer] Add a prunable note for skipping virtual base initializers in subclasses.

NoQ updated this revision to Diff 199534. NoQ marked an inline comment as done. NoQ added a comment. > What about "the most derived class" Sounds great! > It would be cool to say "by A" or something more simple and precise. We can't do that in all cases (we don't necessarily see the

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

eugenis added a comment. I think the idea is that implementing our own spinlock is not much harder than having 3 platform-specific implementations (posix, window, fuchsia). Also, scudo_standalone is doing the same ( @cryptoad, why? ). As Mitch mentioned, we should move the implementation into

r360739 - [analyzer] MIGChecker: Fix redundant semicolon.

Author: dergachev Date: Tue May 14 18:36:41 2019 New Revision: 360739 URL: http://llvm.org/viewvc/llvm-project?rev=360739=rev Log: [analyzer] MIGChecker: Fix redundant semicolon. Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MIGChecker.cpp Modified:

[PATCH] D61909: Add Clang shared library with C++ exports

beanz updated this revision to Diff 199508. beanz added a comment. Changed to lowercase 'c' to match other clang libraries. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61909/new/ https://reviews.llvm.org/D61909 Files:

[PATCH] D60162: [ThinLTO] Add module flags for TargetLibraryInfoImpl and use in LTO backends

steven_wu added inline comments. Comment at: llvm/lib/LTO/LTOBackend.cpp:221 +static TargetLibraryInfoImpl *createTLII(Module , TargetMachine *TM) { + TargetLibraryInfoImpl *TLII = tejohnson wrote: > tejohnson wrote: > > steven_wu wrote: > > > Should this be

[PATCH] D61909: Add Clang shared library with C++ exports

beanz added a comment. @winksaville whether or not PIC is required for shared libraries varies by platform. These days LLVM defaults to -fPIC, and I'm not sure we actually support running LLVM without -fPIC on systems that require shared libraries to be PIC. Repository: rG LLVM Github

[PATCH] D61350: [clang-tidy] New check calling out uses of +new in Objective-C code

stephanemoore accepted this revision. stephanemoore marked an inline comment as done. stephanemoore added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp:54 +CheckFactories.registerCheck(

r360720 - Fix bots by adding target triple to test.

Author: leonardchan Date: Tue May 14 15:37:34 2019 New Revision: 360720 URL: http://llvm.org/viewvc/llvm-project?rev=360720=rev Log: Fix bots by adding target triple to test. Modified: cfe/trunk/test/CodeGen/hwasan-new-pm.c Modified: cfe/trunk/test/CodeGen/hwasan-new-pm.c URL:

[PATCH] D36357: Added a better diagnostic when using the delete operator with lambdas

Rakete updated this revision to Diff 199528. Rakete marked an inline comment as done. Rakete added a comment. Nevermind, seems to be working fine even with. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D36357/new/

[PATCH] D61364: [libcxx] [test] Fix incorrect allocator::construct assertions in associative container tests

BillyONeal marked an inline comment as done. BillyONeal added inline comments. Comment at: test/std/containers/map_allocator_requirement_test_templates.h:236 const ValueTp v(42, 1); -cc->expect(); +cc->expect(); It ret = c.insert(c.end(), std::move(v));

[PATCH] D60974: Clang IFSO driver action.

compnerd added a comment. How about some cases for: - global variable which is `static` and `extern`'ed - global variable which is `static` defined in a function which is `static` - global variable which is `static` defined in a function which is *not* `inline` - global variable which is

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

eugenis added a comment. Ha, I think it matches LLVM perfectly :) G, of course, stands for "Galaxy". Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60593/new/ https://reviews.llvm.org/D60593 ___

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

hctim added a comment. In D60593#1502266 , @jfb wrote: > Hello I'm here to bikeshed, and this is a terrible name for an LLVM project. > Or super appropriate because LLVM itself is a terrible name? In any case, > backronym it to something else, or rename

[PATCH] D57858: [analyzer] Add a new frontend flag to display all checker options

NoQ added a comment. In D57858#1501065 , @dkrupp wrote: > These are the alpha checkers that we are testing in Ericsson: Hmm, if you don't mind i'll take this to cfe-dev, as it's an interesting topic :) CHANGES SINCE LAST ACTION

[PATCH] D61845: [builtin] Fixed definitions of builtins that rely on the int/long long type is 32/64 bits

efriedma added inline comments. Comment at: test/CodeGen/builtins.cpp:5 +// RUN: %clang_cc1 -std=c++11 -triple powerpc-pc-linux -verify %s +// RUN: %clang_cc1 -std=c++11 -triple arm-linux-gnueabi -verify %s + You don't need quite so many targets on this list.

[PATCH] D61365: [libcxx] [test] Suppress float->int narrowing warning in vector range-construction test.

EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. This LGTM after correctly wrapping it as Casey mentionse. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61365/new/ https://reviews.llvm.org/D61365

[PATCH] D61366: [libcxx] [test] Don't assert that moved-from containers with non-POCMA allocators are empty.

EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. I'm not sure I agree with your design decision, but this patch LGTM. Are you not allowed to move the containers elements in this case? CHANGES SINCE LAST ACTION

[PATCH] D61634: [clang/llvm] Allow efficient implementation of libc's memory functions in C/C++

hfinkel added a comment. In D61634#1502043 , @efriedma wrote: > > I have a related patch that turns -fno-builtin* options into module flags > > Do you have any opinion on representing -fno-builtin using a module flag vs. > a function attribute in IR? It

[PATCH] D61814: [CFG] NFC: Remove implicit conversion from CFGTerminator to Stmt *, make it a variant class instead.

NoQ updated this revision to Diff 199531. NoQ marked 2 inline comments as done. NoQ added a comment. Fxd. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61814/new/ https://reviews.llvm.org/D61814 Files: clang/include/clang/Analysis/CFG.h clang/include/clang/Analysis/ProgramPoint.h

[PATCH] D61816: [CFG] [analyzer] pr41300: Add a branch to skip virtual base initializers when they are handled by the superclass.

NoQ updated this revision to Diff 199532. NoQ added a comment. Rebase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61816/new/ https://reviews.llvm.org/D61816 Files: clang/include/clang/Analysis/AnalysisDeclContext.h clang/include/clang/Analysis/CFG.h

[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] D61925: [analyzer] MIGChecker: Add support for os_ref_retain().

NoQ created this revision. NoQ added a reviewer: dcoughlin. Herald added subscribers: cfe-commits, Charusso, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a project: clang. Suppress MIG checker false positives that occur when the

[PATCH] D61925: [analyzer] MIGChecker: Add support for os_ref_retain().

NoQ marked an inline comment as done. NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp:118-119 +static const ParmVarDecl *getOriginParam(SVal V, CheckerContext , + bool IncludeBaseRegions = false) { +

[PATCH] D61925: [analyzer] MIGChecker: Add support for os_ref_retain().

dcoughlin accepted this revision. dcoughlin added a comment. This revision is now accepted and ready to land. LGTM. Thanks! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61925/new/ https://reviews.llvm.org/D61925

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

hctim updated this revision to Diff 199541. hctim marked 10 inline comments as done. hctim added a comment. Herald added subscribers: cfe-commits, srhines. Herald added a reviewer: jfb. Herald added a project: clang. - Working copy of mutex. - >>! In D61923#1502245

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

hctim added a comment. 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 can't use `std::mutex` as we must be C-compatible (and can't make the strong

[PATCH] D61709: [NewPM] Port HWASan and Kernel HWASan

This revision was automatically updated to reflect the committed changes. leonardchan marked 2 inline comments as done. Closed by commit rC360707: [NewPM] Port HWASan and Kernel HWASan (authored by leonardchan, committed by ). Changed prior to commit:

[PATCH] D61750: [Targets] Move soft-float-abi filtering to `initFeatureMap`

efriedma added a comment. > If we agree that this is a good way forward, there also appears to be > +neonfp/-neonfp additions happening in handleTargetFeatures that should > probably be happening in initFeatureMap instead? neonfp isn't passed as a feature in the first place; there's a

[PATCH] D36357: Added a better diagnostic when using the delete operator with lambdas

rsmith added a comment. In D36357#1501999 , @Rakete wrote: > In D36357#1501961 , @rsmith wrote: > > > In D36357#1500949 , @Rakete > > wrote: > > > > > How should I

Re: r360403 - Change -gz and -Wa, --compress-debug-sections to use gABI compression (SHF_COMPRESSED)

It's fine :) The problem may be that users with older pre-2.26 binutils may have older GNU as as well. Their GNU as may not recognize --compress-debug-sections=zlib. On Wed, May 15, 2019 at 3:39 AM Eric Christopher wrote: > Hi Ray, > > I've temporarily reverted this here: > > echristo@jhereg

[PATCH] D61925: [analyzer] MIGChecker: Add support for os_ref_retain().

This revision was automatically updated to reflect the committed changes. Closed by commit rC360737: [analyzer] MIGChecker: Add support for os_ref_retain(). (authored by dergachev, committed by ). Changed prior to commit: https://reviews.llvm.org/D61925?vs=199537=199543#toc Repository: rC

r360657 - Revert r360637 "PR41817: Fix regression in r359260 that caused the MS compatibility"

Author: hans Date: Tue May 14 03:11:33 2019 New Revision: 360657 URL: http://llvm.org/viewvc/llvm-project?rev=360657=rev Log: Revert r360637 "PR41817: Fix regression in r359260 that caused the MS compatibility" > extension allowing a "static" declaration to follow an "extern" > declaration to

Re: r360637 - PR41817: Fix regression in r359260 that caused the MS compatibility

Actually, we didn't notice r359260 in Chromium, however this one (r360637) caused Clang to start asserting during our builds (https://bugs.chromium.org/p/chromium/issues/detail?id=962840), here's a reduced test case: -- extern int a; static int *use1 = int **use2 = static int a = 0; -- $

[PATCH] D61643: [PragmaHandler][NFC] Expose `#pragma` location

lebedev.ri added inline comments. Comment at: clang/docs/ClangPlugins.rst:58 ExamplePragmaHandler() : PragmaHandler("example_pragma") { } -void HandlePragma(Preprocessor , PragmaIntroducerKind Introducer, +void HandlePragma(Preprocessor , PragmaIntroducer

[PATCH] D61438: [ASTImporter] Use llvm::Expected and Error in the importer API

martong marked an inline comment as done. martong added inline comments. Comment at: clang/include/clang/AST/ASTImporter.h:203 /// context, or the import error. -llvm::Expected Import_New(TypeSourceInfo *FromTSI); -// FIXME: Remove this version. -TypeSourceInfo

[PATCH] D61634: [clang/llvm] Allow efficient implementation of libc's memory functions in C/C++

gchatelet added a subscriber: tejohnson. gchatelet added a comment. In D61634#1500453 , @efriedma wrote: > My main blocker is that I want to make sure we're moving in the right > direction: towards LLVM IR with clear semantics, towards straightforward >

  1   2   >