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

2019-05-14 Thread Michael Wyman via Phabricator via cfe-commits
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

2019-05-14 Thread Jonathan Camilleri via Phabricator via cfe-commits
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 https

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

2019-05-14 Thread Mads Ravn via Phabricator via cfe-commits
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] D36357: Added a better diagnostic when using the delete operator with lambdas

2019-05-14 Thread Nicolas Lesser via Phabricator via cfe-commits
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

2019-05-14 Thread Roman Lebedev via Phabricator via cfe-commits
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) Repo

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

2019-05-14 Thread Nicolas Lesser via Phabricator via cfe-commits
Rakete updated this revision to Diff 199380. Rakete added a comment. - Addressed review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D36357/new/ https://reviews.llvm.org/D36357 Files: clang/include/clang/Basic/DiagnosticParseKin

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

2019-05-14 Thread Karl-Johan Karlsson via Phabricator via cfe-commits
Ka-Ka updated this revision to Diff 199385. Ka-Ka marked an inline comment as done. Ka-Ka added a comment. Added a new C++ testcase. Removed the REQUIRES: avr-registered-target in the avr-builtins.c testcase. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61845/new

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

2019-05-14 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang/docs/ClangPlugins.rst:58 ExamplePragmaHandler() : PragmaHandler("example_pragma") { } -void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer, +void HandlePragma(Preprocessor &PP, PragmaIntroducer Intro

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

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

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

2019-05-14 Thread Hans Wennborg via cfe-commits
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 = &a; int **use2 = &use1; static int a = 0; --

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

2019-05-14 Thread Gabor Marton via Phabricator via cfe-commits
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] D57858: [analyzer] Add a new frontend flag to display all checker options

2019-05-14 Thread Daniel Krupp via Phabricator via cfe-commits
dkrupp added a comment. In D57858#1500635 , @NoQ wrote: > In D57858#146 , @dkrupp wrote: > > > Some alpha checkers are considerably more mature than others and are quite > > usable. In our experience, there are

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

2019-05-14 Thread Guillaume Chatelet via Phabricator via cfe-commits
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 >

r360667 - Add a new language mode for C2x; enable [[attribute]] support by default in C2x.

2019-05-14 Thread Aaron Ballman via cfe-commits
Author: aaronballman Date: Tue May 14 05:09:55 2019 New Revision: 360667 URL: http://llvm.org/viewvc/llvm-project?rev=360667&view=rev Log: Add a new language mode for C2x; enable [[attribute]] support by default in C2x. Modified: cfe/trunk/include/clang/Basic/LangOptions.def cfe/trunk/inc

[PATCH] D61370: Add a C2x mode and allow attributes in it

2019-05-14 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman closed this revision. aaron.ballman marked an inline comment as done. aaron.ballman added a comment. I've commit in r360667, thanks! Comment at: lib/Frontend/CompilerInvocation.cpp:2593-2596 + // Enable [[]] attributes in C++11 and C2x by default. + Opts.DoubleS

[PATCH] D61788: Make getObjCEncodingForTypeImpl() take a bitmask instead of 8 bools

2019-05-14 Thread Nico Weber via Phabricator via cfe-commits
thakis updated this revision to Diff 199408. thakis added a comment. minor tweaks for landing CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61788/new/ https://reviews.llvm.org/D61788 Files: clang/include/clang/AST/ASTContext.h clang/lib/AST/ASTContext.cpp Index: clang/lib/AST/ASTC

r360668 - Make getObjCEncodingForTypeImpl() take a bitmask instead of 8 bools

2019-05-14 Thread Nico Weber via cfe-commits
Author: nico Date: Tue May 14 05:32:37 2019 New Revision: 360668 URL: http://llvm.org/viewvc/llvm-project?rev=360668&view=rev Log: Make getObjCEncodingForTypeImpl() take a bitmask instead of 8 bools Slightly easier to read, uses slightly less stack space, and makes it impossible to mix up the ord

[PATCH] D61788: Make getObjCEncodingForTypeImpl() take a bitmask instead of 8 bools

2019-05-14 Thread Phabricator via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rL360668: Make getObjCEncodingForTypeImpl() take a bitmask instead of 8 bools (authored by nico, committed by ). Herald adde

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

2019-05-14 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61874/new/ https://reviews.llvm.org/D61874

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

2019-05-14 Thread Teresa Johnson via Phabricator via cfe-commits
tejohnson added a comment. In D61634#1501066 , @gchatelet wrote: > 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 wit

r360674 - [Driver][Windows] Add dependent lib argument for profile instr generate

2019-05-14 Thread Russell Gallop via cfe-commits
Author: russell_gallop Date: Tue May 14 07:01:40 2019 New Revision: 360674 URL: http://llvm.org/viewvc/llvm-project?rev=360674&view=rev Log: [Driver][Windows] Add dependent lib argument for profile instr generate This is needed so lld-link can find clang_rt.profile when self hosting on Windows wi

[PATCH] D61742: [Driver][Windows] Add dependent lib argument for profile instr generate

2019-05-14 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC360674: [Driver][Windows] Add dependent lib argument for profile instr generate (authored by russell_gallop, committed by ). Changed prior to commit: https://reviews.llvm.org/D61742?vs=199016&id=199436#

r360681 - [Sema] CodeSynthesisContext - add missing variable initialization to constructor. NFCI.

2019-05-14 Thread Simon Pilgrim via cfe-commits
Author: rksimon Date: Tue May 14 07:58:47 2019 New Revision: 360681 URL: http://llvm.org/viewvc/llvm-project?rev=360681&view=rev Log: [Sema] CodeSynthesisContext - add missing variable initialization to constructor. NFCI. SavedInNonInstantiationSFINAEContext isn't used outside of specific contex

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

2019-05-14 Thread via cfe-commits
There's no practical difference. I view it as a matter of documentation of intent, see my commit log comment for r360603. Arguably we could eliminate UNSUPPORTED and move all the expressions into REQUIRES (appropriately negated), but I'm not sure that's a net win for readability. --paulr From:

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

2019-05-14 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov 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] D61386: [clang-tidy] Add support writing a check as a Transformer rewrite rule.

2019-05-14 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. @aaron.ballman, would be a good reviewer for this? I'm happy to look at the transformer bits, but I'm definitely the wrong one to asses it from the `clang-tidy` perspective. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

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

2019-05-14 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked an inline comment as done. ymandel added a comment. Agreed on all the comments -- just one question: Comment at: clang/lib/Tooling/Refactoring/Transformer.cpp:250 + assert(!CommonKind.isNone() && "Cases must have compatible matchers."); + return DynTypedMatcher

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

2019-05-14 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl accepted this revision. aprantl added a comment. Alright, thanks for taking the time to walk me through the thought process! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61438/new/ https://reviews.llvm.org/D61438 ___

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

2019-05-14 Thread Chris Bieneman via Phabricator via cfe-commits
beanz created this revision. beanz added reviewers: tstellar, winksaville. Herald added a subscriber: mgorny. Herald added a project: clang. This patch adds a libClang_shared library on *nix systems which exports the entire C++ API. In order to support this on Windows we should really refactor l

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

2019-05-14 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 199465. ymandel edited the summary of this revision. ymandel added a comment. Response to comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61335/new/ https://reviews.llvm.org/D61335 Files: clang/inclu

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

2019-05-14 Thread Chris Bieneman via Phabricator via cfe-commits
beanz added a comment. @winksaville that was my bad. I left off the new files in the commit because I forgot `git-diff` doesn't grab untracked files... I've uploaded the patch as a D61909 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

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

2019-05-14 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked 9 inline comments as done and an inline comment as not done. ymandel added a comment. Thanks for the review. PTAL. Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:196 +/// Joins multiple rules into a single rule that applies the first rule in +/

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

2019-05-14 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. "complex" in this context is the C99 `_Complex`, which is supported in C++ as an extension, using the same syntax as C. You can just declare a variable with type `_Complex float`. If you need to manipulate the real and imaginary parts of a variable, you can use the gc

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

2019-05-14 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. The implementation LG, thanks! Just a few NITs and comments about the public interface and the comments Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:196 +/// Joins multiple rules into a single rule that applies the first rule

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

2019-05-14 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. Thank you guys for the review! In D61438#1501457 , @aprantl wrote: > Alright, thanks for taking the time to walk me through the thought process! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

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

2019-05-14 Thread Steven Wu via Phabricator via cfe-commits
steven_wu added a comment. Thanks for doing this. I think module flag is a good idea. Some comments inline. Comment at: clang/test/CodeGen/svml-calls.ll:16 + +define void @sin_f64(double* nocapture %varray) { +; CHECK-LABEL: @sin_f64( Personally, I think codege

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

2019-05-14 Thread Richard Smith via cfe-commits
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

2019-05-14 Thread Shafik Yaghmour via Phabricator via cfe-commits
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 failed

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

2019-05-14 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
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 matche

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

2019-05-14 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
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/ https://reviews.llvm.o

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

2019-05-14 Thread David Blaikie via cfe-commits
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 o

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

2019-05-14 Thread Tom Stellard via Phabricator via cfe-commits
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. Comm

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

2019-05-14 Thread Torbjörn Klatt via Phabricator via cfe-commits
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 e

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

2019-05-14 Thread Wink Saville via Phabricator via cfe-commits
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 cm

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

2019-05-14 Thread Matthias Gehre via cfe-commits
Author: mgehre Date: Tue May 14 11:23:10 2019 New Revision: 360698 URL: http://llvm.org/viewvc/llvm-project?rev=360698&view=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 w

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

2019-05-14 Thread Matthias Gehre via Phabricator via cfe-commits
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. Change

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

2019-05-14 Thread Ilya Biryukov via Phabricator via cfe-commits
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: clang/include/clang/Lex/P

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

2019-05-14 Thread Ilya Biryukov via Phabricator via cfe-commits
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): 1

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

2019-05-14 Thread Ilya Biryukov via Phabricator via cfe-commits
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: clang/include/clang/Lex

r360699 - Restore test files accidentally deleted in r354839

2019-05-14 Thread Reid Kleckner via cfe-commits
Author: rnk Date: Tue May 14 11:51:07 2019 New Revision: 360699 URL: http://llvm.org/viewvc/llvm-project?rev=360699&view=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 su

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

2019-05-14 Thread David Blaikie via cfe-commits
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 there

r360701 - Update ASTMerge FileCheck test expectations

2019-05-14 Thread Reid Kleckner via cfe-commits
Author: rnk Date: Tue May 14 12:02:39 2019 New Revision: 360701 URL: http://llvm.org/viewvc/llvm-project?rev=360701&view=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,

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

2019-05-14 Thread Don Hinton via Phabricator via cfe-commits
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

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

2019-05-14 Thread MyDeveloperDay via Phabricator via cfe-commits
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 ___

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

2019-05-14 Thread Eric Christopher via cfe-commits
Author: echristo Date: Tue May 14 12:40:42 2019 New Revision: 360703 URL: http://llvm.org/viewvc/llvm-project?rev=360703&view=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 suc

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

2019-05-14 Thread Eric Christopher via cfe-commits
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 Sendingcfe

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

2019-05-14 Thread Jordan Rupprecht via Phabricator via cfe-commits
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 `-gz=zlib-g

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

2019-05-14 Thread Dávid Bolvanský via Phabricator via cfe-commits
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

2019-05-14 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
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] D61827: [clang-tidy] modernize-loop-convert: impl const cast iter

2019-05-14 Thread Torbjörn Klatt via Phabricator via cfe-commits
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 clang-tools-extra/docs/clang-tidy

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

2019-05-14 Thread Reid Kleckner via cfe-commits
Author: rnk Date: Tue May 14 13:01:03 2019 New Revision: 360705 URL: http://llvm.org/viewvc/llvm-project?rev=360705&view=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: http

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

2019-05-14 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
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 l

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

2019-05-14 Thread Anton Afanasyev via Phabricator via cfe-commits
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] D61817: [analyzer] Add a prunable note for skipping virtual base initializers in subclasses.

2019-05-14 Thread Csaba Dabis via Phabricator via cfe-commits
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] D36357: Added a better diagnostic when using the delete operator with lambdas

2019-05-14 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
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 expens

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

2019-05-14 Thread Teresa Johnson via Phabricator via cfe-commits
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 t

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

2019-05-14 Thread Teresa Johnson via Phabricator via cfe-commits
tejohnson marked an inline comment as done. tejohnson added inline comments. Comment at: llvm/lib/LTO/LTOBackend.cpp:221 +static TargetLibraryInfoImpl *createTLII(Module &Mod, TargetMachine *TM) { + TargetLibraryInfoImpl *TLII = tejohnson wrote: > steven_wu wr

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

2019-05-14 Thread Kristóf Umann via Phabricator via cfe-commits
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] D36357: Added a better diagnostic when using the delete operator with lambdas

2019-05-14 Thread Nicolas Lesser via Phabricator via cfe-commits
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

2019-05-14 Thread via cfe-commits
> -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 th

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

2019-05-14 Thread Chris Bieneman via Phabricator via cfe-commits
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: clang/cmake/modules/AddCl

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

2019-05-14 Thread Steven Wu via Phabricator via cfe-commits
steven_wu added inline comments. Comment at: llvm/lib/LTO/LTOBackend.cpp:221 +static TargetLibraryInfoImpl *createTLII(Module &Mod, TargetMachine *TM) { + TargetLibraryInfoImpl *TLII = tejohnson wrote: > tejohnson wrote: > > steven_wu wrote: > > > Should this

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

2019-05-14 Thread Chris Bieneman via Phabricator via cfe-commits
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 Monor

r360707 - [NewPM] Port HWASan and Kernel HWASan

2019-05-14 Thread Leonard Chan via cfe-commits
Author: leonardchan Date: Tue May 14 14:17:21 2019 New Revision: 360707 URL: http://llvm.org/viewvc/llvm-project?rev=360707&view=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] D61709: [NewPM] Port HWASan and Kernel HWASan

2019-05-14 Thread Leonard Chan via Phabricator via cfe-commits
leonardchan added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:179 - bool runOnFunction(Function &F) override; - bool doInitialization(Module &M) override; + bool instrumentFunction(Function &F); + void initializeWithModule(Module

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

2019-05-14 Thread Eli Friedman via Phabricator via cfe-commits
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 m

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

2019-05-14 Thread Leonard Chan via Phabricator via cfe-commits
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: https://reviews.llvm.org/D61709?vs=198889&i

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

2019-05-14 Thread Artem Dergachev via Phabricator via cfe-commits
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 https://reviews.llvm.

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

2019-05-14 Thread Eli Friedman via Phabricator via cfe-commits
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. Th

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

2019-05-14 Thread Eric Fiselier via Phabricator via cfe-commits
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.

2019-05-14 Thread Eric Fiselier via Phabricator via cfe-commits
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 https://reviews.llvm.org/D61

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

2019-05-14 Thread Billy Robert O'Neal III via Phabricator via cfe-commits
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 need

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

2019-05-14 Thread Eli Friedman via Phabricator via cfe-commits
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 separ

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

2019-05-14 Thread Stephane Moore via Phabricator via cfe-commits
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( +

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

2019-05-14 Thread Hal Finkel via Phabricator via cfe-commits
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] D61366: [libcxx] [test] Don't assert that moved-from containers with non-POCMA allocators are empty.

2019-05-14 Thread Arthur O'Dwyer via Phabricator via cfe-commits
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] D61364: [libcxx] [test] Fix incorrect allocator::construct assertions in associative container tests

2019-05-14 Thread Louis Dionne via Phabricator via cfe-commits
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 explicitl

r360720 - Fix bots by adding target triple to test.

2019-05-14 Thread Leonard Chan via cfe-commits
Author: leonardchan Date: Tue May 14 15:37:34 2019 New Revision: 360720 URL: http://llvm.org/viewvc/llvm-project?rev=360720&view=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: http://l

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

2019-05-14 Thread Billy Robert O'Neal III via Phabricator via cfe-commits
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 libc+

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

2019-05-14 Thread Billy Robert O'Neal III via Phabricator via cfe-commits
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/ https://reviews.llvm.org/D61366

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

2019-05-14 Thread Eric Fiselier via Phabricator via cfe-commits
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 _

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

2019-05-14 Thread Teresa Johnson via Phabricator via cfe-commits
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 opinion

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

2019-05-14 Thread Billy Robert O'Neal III via Phabricator via cfe-commits
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 th

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

2019-05-14 Thread Eric Fiselier via Phabricator via cfe-commits
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 libc

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

2019-05-14 Thread Nicolas Lesser via Phabricator via cfe-commits
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/ https://reviews.llvm.o

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

2019-05-14 Thread Billy Robert O'Neal III via Phabricator via cfe-commits
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] D36357: Added a better diagnostic when using the delete operator with lambdas

2019-05-14 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In D36357#1501999 , @Rakete wrote: > In D36357#1501961 , @rsmith wrote: > > > In D36357#1500949 , @Rakete > > wrote: > > > > > How should I d

[PATCH] D60974: Clang IFSO driver action.

2019-05-14 Thread Saleem Abdulrasool via Phabricator via cfe-commits
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 `static

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

2019-05-14 Thread Artem Dergachev via Phabricator via cfe-commits
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.

2019-05-14 Thread Artem Dergachev via Phabricator via cfe-commits
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 clang/include/clang/StaticA

  1   2   >