[PATCH] D65993: [NFC][clang] Adding argument based Phase list filtering to getComplicationPhases

compnerd added inline comments. Comment at: clang/include/clang/Driver/Types.h:107 +llvm::opt::DerivedArgList , ID Id, +llvm::SmallVectorImpl ); This really makes things confusing, perhaps renaming

[PATCH] D66059: [clang-format] Expand AllowShortBlocksOnASingleLine for WebKit

owenpan created this revision. owenpan added reviewers: sammccall, MyDeveloperDay, klimek, djasper. Herald added a project: clang. Herald added a subscriber: cfe-commits. See PR40840 Repository: rC Clang https://reviews.llvm.org/D66059 Files: clang/docs/ClangFormatStyleOptions.rst

Re: r368446 - More warnings regarding gsl::Pointer and gsl::Owner attributes

You are right! I will look into this tomorrow. If you think this is urgent feel free to revert the commits. Cheers, Gabor On Sat, Aug 10, 2019, 6:41 PM Nico Weber wrote: > Hi Gabor, > > this makes clang warn on this program: > > $ cat test.cc > #include > #include > > struct S { > bool

Re: r368446 - More warnings regarding gsl::Pointer and gsl::Owner attributes

Hi Gabor, this makes clang warn on this program: $ cat test.cc #include #include struct S { bool operator==(const S ) const; }; const S (const std::vector ) { const auto = std::find(v.rbegin(), v.rend(), S()); return *it; } $ out/gn/bin/clang -c test.cc -isysroot $(xcrun

[PATCH] D66058: [NFC][clang] Move much of the argument handling code from Driver::BuildActions to Driver::handleArguments.

plotfi updated this revision to Diff 214545. plotfi added a comment. really sorry for how bad this diff looks. phab just doesnt present in a very ideal way. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66058/new/ https://reviews.llvm.org/D66058

[PATCH] D66058: [NFC][clang] Move much of the argument handling code from Driver::BuildActions to Driver::handleArguments.

plotfi created this revision. plotfi added reviewers: compnerd, aaron.ballman. Herald added a project: clang. Herald added a subscriber: cfe-commits. This change will go in on top of https://reviews.llvm.org/D65993. It simply moves the code that already exists into a new function. Repository:

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Szelethus requested changes to this revision. Szelethus added a comment. This revision now requires changes to proceed. In D66042#1624478 , @Charusso wrote: > In D66042#1624475 , @Szelethus wrote: > > > In

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

ABataev added a comment. In D65835#1624471 , @jdenny wrote: > I made the following changes, as suggested: > > - Add back restriction for OpenMP < 5.0. > - Remove `is_device_ptr` changes. > > Alexey, you said: > > > Plus, these changes are not enough to

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso added a comment. In D66042#1624475 , @Szelethus wrote: > In D66042#1624469 , @Charusso wrote: > > > In a long-term rewriting the Analyzer from scratch would be ideal. There is > > no problem with this

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Szelethus added a comment. In D66042#1624469 , @Charusso wrote: > In D66042#1624468 , @Szelethus wrote: > > > Speaking about performance impact, note where your patch does the actual > > silencing: by the time

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

jdenny updated this revision to Diff 214542. jdenny retitled this revision from "[OpenMP] Fix map/is_device_ptr with DSA on combined directive" to "[OpenMP] Permit map with DSA on combined directive". jdenny edited the summary of this revision. jdenny added a comment. I made the following

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Szelethus added a comment. In D66042#1624468 , @Szelethus wrote: > Speaking about performance impact, note where your patch does the actual > silencing: by the time control reaches that point, we created bug report > equivalence classes, constructed a

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso added a comment. In D66042#1624468 , @Szelethus wrote: > In D66042#1624462 , @Charusso wrote: > > > My solution preserve the checkers as they are and yours definitely would > > rewrite them. Checker

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Szelethus added a comment. In D66042#1624462 , @Charusso wrote: > My solution preserve the checkers as they are and yours definitely would > rewrite them. Checker writing has tons of boilerplates, I think adding more > should be avoided. Why would you

[PATCH] D65993: [NFC][clang] Adding argument based Phase list filtering to getComplicationPhases

plotfi updated this revision to Diff 214541. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65993/new/ https://reviews.llvm.org/D65993 Files: clang/include/clang/Driver/Types.h clang/lib/Driver/Driver.cpp clang/lib/Driver/Types.cpp Index:

[PATCH] D65993: [NFC][clang] Adding argument based Phase list filtering to getComplicationPhases

plotfi marked 5 inline comments as done. plotfi added inline comments. Comment at: clang/lib/Driver/Types.cpp:11 +#include "clang/Driver/Options.h" +#include "clang/Driver/DriverDiagnostic.h" #include "clang/Driver/Types.h" aaron.ballman wrote: > Can you sort

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso added a comment. In D66042#1624432 , @Szelethus wrote: > In D66042#1624365 , @Charusso wrote: > > > In D66042#1624320 , @Szelethus > > wrote: > > > > > I think it

[PATCH] D64672: [X86] Prevent passing vectors of __int128 as in llvm IR

craig.topper updated this revision to Diff 214540. craig.topper added a comment. More test check lines. Test the compatibility flag. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64672/new/ https://reviews.llvm.org/D64672 Files:

[PATCH] D64672: [X86] Prevent passing vectors of __int128 as in llvm IR

craig.topper updated this revision to Diff 214539. craig.topper edited the summary of this revision. craig.topper added a comment. Herald added a project: clang. Add test cases. Change the type to vXi64 instead of vXf64. Add abi compatibility flag. Restrict to linux and freebsd. Repository:

Re: r368501 - Fix a false positive warning when initializing members with gsl::Owners.

Up to you :) On Fri, Aug 9, 2019 at 8:54 PM Gábor Horváth wrote: > It does! Do you want me to commit that test as well? > > On Fri, 9 Aug 2019 at 17:50, Nico Weber wrote: > >> This fixes `+ X(std::unique_ptr up) : pointee(up.get()), >> pointer(std::move(up)) {}` as well, right? >> >> On Fri,

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Szelethus added a comment. In D66042#1624365 , @Charusso wrote: > In D66042#1624320 , @Szelethus wrote: > > > I think it would make a lot more sense to create a separate (and hidden!) > > coreModeling package that

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

MaskRay added inline comments. Comment at: test/Sema/parentheses.c:148 + + (void)(x | b ? 1 : 2); // expected-warning {{operator '?:' has lower precedence than '|'}} expected-note 2{{place parentheses}} + (void)(x & b ? 1 : 2); // expected-warning {{operator '?:' has lower

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

xbolva00 marked an inline comment as done. xbolva00 added inline comments. Comment at: include/clang/Basic/DiagnosticGroups.td:508 def Varargs : DiagGroup<"varargs">; +def XorUsedAsPow : DiagGroup<"xor-used-as-pow">; aaron.ballman wrote: > xbolva00 wrote: > >

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

xbolva00 updated this revision to Diff 214534. xbolva00 added a comment. Addressed review notes CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D65993: [NFC][clang] Adding argument based Phase list filtering to getComplicationPhases

aaron.ballman added inline comments. Comment at: clang/lib/Driver/Driver.cpp:3326 +types::getCompilationPhases(*this, Args, InputType, PL); +if (!PL.size()) continue; `PL.empty()` instead Comment at:

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

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

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

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

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] D66045: Improve detection of same value in comparisons

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] D55125: [clang-tidy] Fix a false positive in misc-redundant-expression check

aaron.ballman added inline comments. Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:621 +static bool isSameToken(const Token , const Token , +const SourceManager ) { `isSameRawIdentifierToken()` so that the name isn't too

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

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] D65919: [clang-tidy] Add llvm-prefer-register-over-unsigned check and apply it to LLVM

aaron.ballman marked an inline comment as done. aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp:28-29 CheckFactories.registerCheck("llvm-include-order"); +CheckFactories.registerCheck( +

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

Charusso added a comment. In D66042#1624320 , @Szelethus wrote: > I think it would make a lot more sense to create a separate (and hidden!) > coreModeling package that would do all the modeling, and regard core as a > highly recommended, but not

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

xbolva00 added a comment. I will split enum constants handling to other patch in near future. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63082/new/ https://reviews.llvm.org/D63082 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

xbolva00 updated this revision to Diff 214533. xbolva00 added a comment. Added UDL tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D66054: [CrossTU] Fix problem with CrossTU AST load limit and progress messages.

balazske updated this revision to Diff 214532. balazske added a comment. - [CrossTU] Fix problem with CrossTU AST load limit and progress messages. - Reformat. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66054/new/

[PATCH] D66056: Add 2 tests which cover the hover result for auto

puremourning created this revision. puremourning added a reviewer: sammccall. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, ilya-biryukov. Herald added a project: clang. Clangd supports reporting the deduced type when hovering on 'auto', but does not report it when hovering

[PATCH] D65577: [ASTImporter] Import default expression of param before creating the param.

balazske updated this revision to Diff 214531. balazske added a comment. Rename of variables and reformat. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65577/new/ https://reviews.llvm.org/D65577 Files: clang/include/clang/AST/ASTImporter.h

[PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

Lekensteyn added a comment. Thanks for picking this up again. I've left some nitpicks below in a quick review. The "strict" parameter is not precisely defined, if that is fixed I think this would be ready for merge. Comment at: clang/test/Driver/debug-prefix-map.c:8 +//

[PATCH] D55326: [Driver] Fix incorrect GNU triplet for PowerPC on SUSE Linux

aaronpuchert added a comment. Herald added subscribers: shchenz, wuzish. Sorry for warming this up again, but it would be nice to get rid of this patch in openSUSE. In D55326#1342412 , @nemanjai wrote: > - Will this change not affect 64-bit PPC SUSE?

[PATCH] D55125: [clang-tidy] Fix a false positive in misc-redundant-expression check

dkrupp updated this revision to Diff 214529. dkrupp added a comment. @aaron.ballman 's comments are fixed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55125/new/ https://reviews.llvm.org/D55125 Files: clang-tidy/misc/RedundantExpressionCheck.cpp

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

xbolva00 added a comment. Good improvement, thanks 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] D66042: [analyzer] Analysis: "Disable" core checkers

Szelethus added a comment. There is a discussion to be had about the core package. So @NoQ suggested that we could hide the entire thing just as we do with apiModeling, but I argued that the users wouldn't be exposed to the checker descriptions. However, it makes little sense to caution our

[PATCH] D65149: [Format] Add test demonstrating PR42722

MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:5184 + "c++;\n" + "d++\n" + " });\n" modocache wrote: > This is a passing test that demonstrates that the indentation of

[PATCH] D66054: [CrossTU] Fix problem with CrossTU AST load limit and progress messages.

martong accepted this revision. martong added a reviewer: gamesh411. martong added a comment. This revision is now accepted and ready to land. LGTM! Thank you. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66054/new/

[PATCH] D66054: [CrossTU] Fix problem with CrossTU AST load limit and progress messages.

balazske created this revision. Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp. Herald added a reviewer: martong. Herald added a project: clang. Number of loaded ASTs is to be incremented only if the AST was really loaded but not if it was returned from cache. At the same

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

xbolva00 updated this revision to Diff 214525. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaExpr.cpp

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

xbolva00 added a comment. Hello, I fixed your notes + added test cases. Only remaining question is what to do in "2 ^ 64" (65, 66, .. etc) case. I can emit "result of 2 ^ 64" is "XYZ", but this is rather poor diagnostic. What should we suggest for "2 ^ 64"? (@jfb The "2 ^ 64 - 1" case to be

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

xbolva00 updated this revision to Diff 214524. xbolva00 added a comment. svn added forgotten test file.. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 Files: include/clang/Basic/DiagnosticGroups.td

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

xbolva00 updated this revision to Diff 214523. xbolva00 added a comment. Addressed review notes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63423/new/ https://reviews.llvm.org/D63423 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D65978: [clang] Fixed x86 cpuid NSC signature

This revision was automatically updated to reflect the committed changes. Closed by commit rL368510: [clang] Fixed x86 cpuid NSC signature (authored by teemperor, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D65978: [clang] Fixed x86 cpuid NSC signature

teemperor accepted this revision. teemperor added a comment. LGTM. Thanks for the patch! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65978/new/ https://reviews.llvm.org/D65978 ___ cfe-commits mailing list

r368510 - [clang] Fixed x86 cpuid NSC signature

Author: teemperor Date: Sat Aug 10 03:14:01 2019 New Revision: 368510 URL: http://llvm.org/viewvc/llvm-project?rev=368510=rev Log: [clang] Fixed x86 cpuid NSC signature Summary: The signature "Geode by NSC" for NSC vendor is wrong. In lib/Headers/cpuid.h, signature_NSC_edx and signature_NSC_ecx

[PATCH] D65978: [clang] Fixed x86 cpuid NSC signature

Jacotot added a comment. In D65978#1624220 , @teemperor wrote: > @Jacotot I assume you don't have commit access to merge yourself? I don't : this is my very first contribution to the project Repository: rC Clang CHANGES SINCE LAST ACTION

[PATCH] D65978: [clang] Fixed x86 cpuid NSC signature

teemperor added a comment. @Jacotot I assume you don't have commit access to merge yourself? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65978/new/ https://reviews.llvm.org/D65978 ___ cfe-commits mailing list

[PATCH] D65577: [ASTImporter] Import default expression of param before creating the param.

martong added inline comments. Comment at: clang/test/Analysis/Inputs/ctu-other.cpp:135 + +struct DefParmContext { + static const int I; balazske wrote: > martong wrote: > > shafik wrote: > > > martong wrote: > > > > Perhaps we could write `Default` instead of

r368508 - [modulemap] Add AArch64SVEACLETypes.def

Author: kristina Date: Sat Aug 10 01:21:14 2019 New Revision: 368508 URL: http://llvm.org/viewvc/llvm-project?rev=368508=rev Log: [modulemap] Add AArch64SVEACLETypes.def Update modulemap with a new textual header. Modified: cfe/trunk/include/clang/module.modulemap Modified:

[PATCH] D66049: Fixes Bug 41729 and improves strlcat and strlcpy modeling

dkrupp created this revision. dkrupp added reviewers: NoQ, Szelethus, gamesh411. Herald added subscribers: cfe-commits, rnkovacs. Herald added a project: clang. Fixes Bug 41729 (https://bugs.llvm.org/show_bug.cgi?id=41729) and the following errors: -Fixes false positive reports of strlcat

[PATCH] D65925: [clang-format] Add SpaceInEmptyBlock option for WebKit

This revision was automatically updated to reflect the committed changes. Closed by commit rL368507: [clang-format] Add SpaceInEmptyBlock option for WebKit (authored by owenpan, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

r368507 - [clang-format] Add SpaceInEmptyBlock option for WebKit

Author: owenpan Date: Sat Aug 10 00:51:21 2019 New Revision: 368507 URL: http://llvm.org/viewvc/llvm-project?rev=368507=rev Log: [clang-format] Add SpaceInEmptyBlock option for WebKit See PR40840 Differential Revision: https://reviews.llvm.org/D65925 Modified:

[PATCH] D65925: [clang-format] Add SpaceInEmptyBlock option for WebKit

owenpan added a comment. In D65925#1623965 , @thakis wrote: > https://webkit.org/code-style-guidelines/ has one instance of `{ }` with a > space and one of `{}` without a space, and as far as I can tell no text that > says which instance is correct. Are

[PATCH] D65925: [clang-format] Add SpaceInEmptyBlock option for WebKit

owenpan updated this revision to Diff 214509. owenpan added a comment. Changed "may" to "will" and updated the diff. (Should we do the same for the SpaceInEmptyParentheses option?) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65925/new/

[PATCH] D66040: [Sema] Implement DR2386 for C++17 structured binding

Jeroen added a comment. In the reproduction of https://bugs.llvm.org/show_bug.cgi?id=33236 there is explicit mentioning of `const T`. It would be nice if the test cases for this fix would also have coverage for that. Comment at: clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp:15