[PATCH] D43867: Rename a few checks from misc- to bugprone-.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D43867#1021985, @alexfh wrote: > In https://reviews.llvm.org/D43867#1021956, @aaron.ballman wrote: > > > When we do this sort of move, do we want to ke

[PATCH] D43868: Rename more checks from misc- to bugprone-.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43868 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D43870: [clang-tidy] Another batch of checks to rename from misc- to bugprone-.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM with a couple of drive-by nits that could be fixed if you wanted to. Comment at: docs/clang-tidy/checks/bugprone-swapped-arguments.rst:6 + + +Finds potenti

[PATCH] D43766: [clang-tidy][modernize-make-unique] Checks c++14 flag before using std::make_unique

aaron.ballman added inline comments. Comment at: clang-tidy/modernize/MakeUniqueCheck.cpp:21 +: MakeSmartPtrCheck(Name, Context, "std::make_unique"), + MinimumLanguageVersion(Options.get("MakeUniqueLanguageVersion", + getDefaultMin

[PATCH] D43902: [clang-format] Don't detect C++11 attribute specifiers as ObjC

aaron.ballman added inline comments. Comment at: lib/Format/TokenAnnotator.cpp:346 + bool parseCpp11AttributeSpecifier(FormatToken *Tok) { +if (!Style.isCpp()) return false; +if (!Tok || !Tok->startsSequence(tok::l_square, tok::l_square)) C can use this

[PATCH] D26350: Keep invalid Switch in the AST

aaron.ballman added inline comments. Comment at: lib/Sema/SemaStmt.cpp:823 + CondExpr->isValueDependent() || + isa(CondExpr); + unsigned CondWidth = rsmith wrote: > It's fragile to assume that the

[PATCH] D42730: [clang-tidy]] Add check for use of types/classes/functions from header which are deprecated and removed in C++17

aaron.ballman added inline comments. Comment at: clang-tidy/modernize/DeprecatedFunctionalCheck.cpp:48-54 + } else if (const auto *const Call = + Result.Nodes.getNodeAs("ptr_fun_call")) { +diag(Call->getLocStart(), Message) << "'std::ptr_fun'"; + } else if (

[PATCH] D43766: [clang-tidy][modernize-make-unique] Checks c++14 flag before using std::make_unique

aaron.ballman added inline comments. Comment at: clang-tidy/modernize/MakeUniqueCheck.cpp:21 +: MakeSmartPtrCheck(Name, Context, "std::make_unique"), + MinimumLanguageVersion(Options.get("MakeUniqueLanguageVersion", + getDefaultMin

[PATCH] D43847: [clang-tidy] Add check: replace string::find(...) == 0 with absl::StartsWith

aaron.ballman added a comment. In https://reviews.llvm.org/D43847#1023452, @hokein wrote: > In https://reviews.llvm.org/D43847#1021967, @aaron.ballman wrote: > > > I need a bit more context because I'm unfamiliar with `absl`. What is this > > module's intended use? > > > As `absl` has been open-

[PATCH] D43745: Fix cppcoreguidelines-pro-bounds-pointer-arithmetic not working for functions with auto return specifier.

aaron.ballman added a comment. Some minor formatting nits, but otherwise LGTM. You should run the patch through clang-format: https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting Comment at: clang-tidy/cppcoreguidelines/ProBoundsPointerArithmeticCheck.cp

[PATCH] D43248: [Attr] Fix parameter indexing for attributes

aaron.ballman added inline comments. Comment at: include/clang/AST/Attr.h:206 + + void cmpable(const ParamIdx &I) const { +assert(isValid() && I.isValid() && The name here can be improved. How about `checkInvariants()`? Might as well make this inline while

[PATCH] D43248: [Attr] Fix parameter indexing for attributes

aaron.ballman added a comment. In https://reviews.llvm.org/D43248#1023720, @jdenny wrote: > Hi Aaron. It occurs to me now that this patch has grown rather large and, in > some places, a little subtle. Would it help the review if I were to break it > up into a patch series that introduces Para

[PATCH] D43248: [Attr] Fix parameter indexing for attributes

aaron.ballman added inline comments. Comment at: include/clang/AST/Attr.h:206 + + void cmpable(const ParamIdx &I) const { +assert(isValid() && I.isValid() && jdenny wrote: > aaron.ballman wrote: > > The name here can be improved. How about `checkInvariants()

[PATCH] D43248: [Attr] Fix parameter indexing for attributes

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Aside from two minor nits, this LGTM. Thank you for working on it! Comment at: test/Sema/attr-ownership.cpp:1 +// RUN: %clang_cc1 %s -verify +

[PATCH] D43248: [Attr] Fix parameter indexing for attributes

aaron.ballman added inline comments. Comment at: test/Sema/attr-ownership.cpp:1 +// RUN: %clang_cc1 %s -verify + jdenny wrote: > aaron.ballman wrote: > > Please pass `-fsyntax-only` as well. > Sure. Would you like me to change test/Sema/attr-ownership.c while we

[PATCH] D43248: [Attr] Fix parameter indexing for attributes

aaron.ballman added a comment. In https://reviews.llvm.org/D43248#1025495, @jdenny wrote: > Aaron, thanks for the review. I've applied your suggestions and am ready to > commit. You're correct that we have a lot of freedom with the commit message, but the important piece is in describing wha

[PATCH] D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test

aaron.ballman added a comment. In https://reviews.llvm.org/D40737#1025777, @lebedev.ri wrote: > In https://reviews.llvm.org/D40737#1024120, @JonasToth wrote: > > > After long inactivity (sorry!) i had a chance to look at it again: > > > > switch(i) { > > case 0:; > > case 1:; > > case 2:;

[PATCH] D43847: [clang-tidy] Add check: replace string::find(...) == 0 with absl::StartsWith

aaron.ballman added inline comments. Comment at: clang-tidy/absl/StringFindStartswithCheck.cpp:17-19 + auto stringClassMatcher = anyOf(cxxRecordDecl(hasName("string")), + cxxRecordDecl(hasName("__versa_string")), +

[PATCH] D44032: Remove -i command line option, add -imultilib

aaron.ballman added a comment. testcase Repository: rC Clang https://reviews.llvm.org/D44032 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listi

[PATCH] D44032: Remove -i command line option, add -imultilib

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! https://reviews.llvm.org/D44032 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman

[PATCH] D43750: Allow writing calling convention attributes on function types

aaron.ballman added a comment. Ping https://reviews.llvm.org/D43750 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42730: [clang-tidy]] Add check for use of types/classes/functions from header which are deprecated and removed in C++17

aaron.ballman added inline comments. Comment at: clang-tidy/modernize/DeprecatedFunctionalCheck.cpp:48-54 + } else if (const auto *const Call = + Result.Nodes.getNodeAs("ptr_fun_call")) { +diag(Call->getLocStart(), Message) << "'std::ptr_fun'"; + } else if (

[PATCH] D25820: [Sema][Objective-C] Formatting warnings should see through Objective-C message sends

aaron.ballman added a comment. In https://reviews.llvm.org/D25820#1027216, @benhamilton wrote: > Actually, looking more deeply, it's possible this is a bug in the Apple SDK: > > @interface NSBundle > - (NSString *)localizedStringForKey:(NSString *)key value:(nullable > NSString *)value table

[PATCH] D44093: [BUILTINS] structure pretty printer

aaron.ballman added reviewers: echristo, rsmith. aaron.ballman added a comment. Adding some reviewers. One thing that's missing from this patch are test cases that demonstrate both the semantic checking (builtin accepts proper args, rejects improper ones, etc) and output. Co

[PATCH] D44137: [clang-tidy] Fix one corner case in make-unique check.

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44137 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/c

[PATCH] D44093: [BUILTINS] structure pretty printer

aaron.ballman added inline comments. Comment at: lib/CodeGen/CGBuiltin.cpp:1208 + +assert(RT && "The first argument must be a record type"); + paulsemel wrote: > aaron.ballman wrote: > > I don't see anything enforcing this constraint, so users are likely to h

[PATCH] D44143: Create properly seeded random generator check

aaron.ballman added a comment. Thank you for working on this check! Do you think it might be possible to also add a check for `cert-msc32-c` that handles the C side of things, and use a common check to implement both? C won't ever have the C++'isms, but C++ can certainly abuse `std::srand()` so

[PATCH] D44093: [BUILTINS] structure pretty printer

aaron.ballman added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5022-5023 +def err_dump_struct_invalid_argument_type : Error< + "invalid argument of type %0; expected %1">; + Can you look to see if we have an existing diagnostic tha

[PATCH] D42730: [clang-tidy]] Add check for use of types/classes/functions from header which are deprecated and removed in C++17

aaron.ballman added inline comments. Comment at: clang-tidy/modernize/DeprecatedFunctionalCheck.cpp:48-54 + } else if (const auto *const Call = + Result.Nodes.getNodeAs("ptr_fun_call")) { +diag(Call->getLocStart(), Message) << "'std::ptr_fun'"; + } else if (

[PATCH] D42624: [clang-tidy] Add a utility Matcher to match the next statement within a statement sequence

aaron.ballman added a comment. In https://reviews.llvm.org/D42624#1031073, @sylvestre.ledru wrote: > @aaron.ballman Hello, Do you think it is ready to land? Thanks @alexfh was asking to make this matcher local to the check rather than add it to Matchers.h, so I think this patch should be aband

[PATCH] D44231: [clang-tidy] Check for sizeof that call functions

aaron.ballman added a comment. In https://reviews.llvm.org/D44231#1031380, @pfultz2 wrote: > > Can you elaborate a bit more about this? > > This catches problems when calling `sizeof(f())` when `f` returns an > integer(or enum). This is because, most likely, the integer represents the > type to

[PATCH] D44231: [clang-tidy] Check for sizeof that call functions

aaron.ballman added a comment. In https://reviews.llvm.org/D44231#1031503, @pfultz2 wrote: > > Can you point to some real world code that would benefit from this check? > > Yes, here: > > https://github.com/ROCmSoftwarePlatform/MIOpen/blob/master/src/convolution.cpp#L184 > > It is erroneously cal

[PATCH] D44093: [BUILTINS] structure pretty printer

aaron.ballman added inline comments. Comment at: lib/CodeGen/CGBuiltin.cpp:1231 + Types[getContext().VoidPtrTy] = "%p"; + Types[getContext().FloatTy] = "%f"; + Types[getContext().DoubleTy] = "%f"; paulsemel wrote: > aaron.ballman wrote: > > paulsem

[PATCH] D44231: [clang-tidy] Check for sizeof that call functions

aaron.ballman added a comment. In https://reviews.llvm.org/D44231#1032011, @pfultz2 wrote: > > If the return type of foo() is changed, then the use for s1 will be > > automatically updated > > But usually you write it as: > > using foo_return = uint16_t; > foo_return foo(); > ... > size_

[PATCH] D44231: [clang-tidy] Check for sizeof that call functions

aaron.ballman added a comment. In https://reviews.llvm.org/D44231#1032782, @pfultz2 wrote: > > Again, that only works for C++ and not C. > > Typedef has always worked in C. This is true. I think what I'm struggling to say is: I don't think this is a common pattern in either C or C++. It's als

[PATCH] D37014: [clang-tidy] Add a checker to remove useless intermediate variables before return statements with comparisons

aaron.ballman added inline comments. Comment at: clang-tidy/readability/UnnecessaryIntermediateVarCheck.cpp:27-30 + auto File = Context->getCurrentFile(); + auto Style = format::getStyle(*Context->getOptionsForFile(File).FormatStyle, +File, "none

[PATCH] D44346: [clang-tidy] Add Fuchsia checker for temporary objects

aaron.ballman added inline comments. Comment at: clang-tidy/fuchsia/FuchsiaTidyModule.cpp:44 +CheckFactories.registerCheck( +"fuchsia-zx-temporary-objects"); } Do we want a zircon module instead? I'm wondering about people who enable modules by do

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. I think this check LGTM. https://reviews.llvm.org/D41655 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.

[PATCH] D43248: [Attr] Fix parameter indexing for attributes

aaron.ballman added a comment. Can you regenerate the patch using the same paths as https://reviews.llvm.org/D43248?id=136811? When I try to do a diff between what was accepted & committed and the current patch, Phabricator gets confused because the paths are too different from one another. h

[PATCH] D44143: [clang-tidy] Create properly seeded random generator check

aaron.ballman added inline comments. Comment at: clang-tidy/cert/CERTTidyModule.cpp:40 // DCL -CheckFactories.registerCheck( -"cert-dcl21-cpp"); +CheckFactories.registerCheck("cert-dcl21-cpp"); CheckFactories.registerCheck("cert-dcl50-cpp");

[PATCH] D43248: [Attr] Fix parameter indexing for attributes

aaron.ballman added a comment. It seems like there are some other changes than just the serialize and deserialize that I'm not opposed to, but am wondering why they're needed. It seems some functions are now `getFoo()` calls and it seems like some declarations moved around. Are those intended a

[PATCH] D43248: [Attr] Fix parameter indexing for attributes

aaron.ballman added a comment. In https://reviews.llvm.org/D43248#1035477, @jdenny wrote: > In https://reviews.llvm.org/D43248#1035466, @aaron.ballman wrote: > > > It seems like there are some other changes than just the serialize and > > deserialize that I'm not opposed to, but am wondering why

[PATCH] D43248: [Attr] Fix parameter indexing for attributes

aaron.ballman added inline comments. Comment at: cfe/trunk/test/Frontend/ast-attr.cpp:5 +// RUN: %clang -emit-ast -o %t.ast %S/../Sema/attr-print.cpp +// RUN: %clang_cc1 %t.ast -ast-print | FileCheck %S/../Sema/attr-print.cpp Just to verify my understanding, this

[PATCH] D43248: [Attr] Fix parameter indexing for attributes

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM with the test comments fixed up. Comment at: cfe/trunk/test/Frontend/ast-attr.cpp:5 +// RUN: %clang -emit-ast -o %t.ast %S/../Sema/attr-print.cpp +// RUN:

[PATCH] D16008: [clang-tidy] Add calling virtual functions in constructors/destructors check.

aaron.ballman added a comment. In https://reviews.llvm.org/D16008#1035683, @alexfh wrote: > If the CSA checker is still in alpha, I'd proceed with this check instead of > investing time in polishing the CSA implementation. Do you know why the CSA checker is still in alpha? As best I can tell,

[PATCH] D44346: [clang-tidy] Add Zircon module to clang-tidy

aaron.ballman added inline comments. Comment at: clang-tidy/tool/ClangTidyMain.cpp:527 -// This anchor is used to force the linker to link the GoogleModule. +// This anchor is used to force the linker to link the FuchsiaModule. extern volatile int FuchsiaModuleAnchorSource; --

[PATCH] D43248: [Attr] Fix parameter indexing for attributes

aaron.ballman added a comment. In https://reviews.llvm.org/D43248#1036409, @jdenny wrote: > I'd prefer to move it than to expect people to obey such a comment. Let's > see what Aaron says. I have a slight preference for moving the tests now that I know they're causing problems, unless that t

[PATCH] D44346: [clang-tidy] Add Zircon module to clang-tidy

aaron.ballman added a comment. I dreamed up some test cases that may or may not make sense. I think they boil down to a few things: - Does inheriting from a prohibited type still diagnose when the derived type is used to construct a temporary? - Should it still be prohibited if the temporary ty

[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods

aaron.ballman added inline comments. Comment at: clang-tidy/bugprone/ParentVirtualCallCheck.cpp:24-25 + const CXXRecordDecl *ThisClass) { + assert(Parent); + assert(ThisClass); + if (Parent->getCanonicalDecl() == ThisClass->getCanonicalDecl()) ---

[PATCH] D16008: [clang-tidy] Add calling virtual functions in constructors/destructors check.

aaron.ballman added a comment. In https://reviews.llvm.org/D16008#1035948, @malcolm.parsons wrote: > In https://reviews.llvm.org/D16008#1035789, @aaron.ballman wrote: > > > Do you know why the CSA checker is still in alpha? > > > It isn't - https://reviews.llvm.org/D26768 moved it to optin. > > I

[PATCH] D44346: [clang-tidy] Add Zircon module to clang-tidy

aaron.ballman added inline comments. Comment at: test/clang-tidy/zircon-temporary-objects.cpp:86 +Ty make_ty() { return Ty(); } +// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: creating a temporary object of type 'Bar' is prohibited +// CHECK-MESSAGES: :[[@LINE-2]]:23: warning: cre

[PATCH] D44439: [Sema] Pop function scope when instantiating a func with skipped body

aaron.ballman requested changes to this revision. aaron.ballman added a comment. This revision now requires changes to proceed. Test case for this change? Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:3944 + // context seems wrong. Investigate more. + ActOnFini

[PATCH] D44439: [Sema] Pop function scope when instantiating a func with skipped body

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, thank you for the explanation about the tests. Repository: rC Clang https://reviews.llvm.org/D44439 ___ cfe-commits mailin

[PATCH] D44439: [Sema] Pop function scope when instantiating a func with skipped body

aaron.ballman added a comment. In https://reviews.llvm.org/D44439#1036868, @ilya-biryukov wrote: > @aaron.ballman , here's a test-case I came up with. I'll fix the other issue > (invalid error, that forces the test to fail) and submit it for review along > with the fix for the error. Does that

[PATCH] D43162: [Parser] (C++) Make -Wextra-semi slightly more useful

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a testing nit. Comment at: test/SemaCXX/extra-semi.cpp:14 +void F(){} +; // expected-warning {{extra ';' outside of a function is}} + --

[PATCH] D44480: [Sema] Don't skip function bodies with 'auto' without trailing return type

aaron.ballman added a reviewer: rsmith. aaron.ballman added a comment. Adding Richard as a reviewer, because this seems somewhat clever to me (so there may be a better change to make). Comment at: lib/Sema/SemaDecl.cpp:12611 +// false on C++14's auto return type without tr

[PATCH] D44346: [clang-tidy] Add Zircon module to clang-tidy

aaron.ballman added inline comments. Comment at: clang-tidy/zircon/TemporaryObjectsCheck.cpp:51 + "creating a temporary object of type %0 is prohibited") +<< D->getConstructor()->getParent()->getQualifiedNameAsString(); +} aaron.ballman wrote: > Y

[PATCH] D44346: [clang-tidy] Add Zircon module to clang-tidy

aaron.ballman added inline comments. Comment at: clang-tidy/zircon/TemporaryObjectsCheck.cpp:51 + "creating a temporary object of type %0 is prohibited") +<< D->getConstructor()->getParent()->getQualifiedNameAsString(); +} aaron.ballman wrote: > a

[PATCH] D43766: [clang-tidy][modernize-make-unique] Checks c++14 flag before using std::make_unique

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! https://reviews.llvm.org/D43766 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44346: [clang-tidy] Add Zircon module to clang-tidy

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM with some minor documentation nits. Comment at: docs/clang-tidy/checks/zircon-temporary-objects.rst:7 +Warns on construction of specific temporary objects

[PATCH] D44231: [clang-tidy] Check for sizeof that call functions

aaron.ballman added a comment. In https://reviews.llvm.org/D44231#1039888, @hokein wrote: > As this patch can catch some mistakes, I'm fine with checking it in. I agree > that it is reasonable to write normal code like `sizeof(func_call())` (not > false positive), maybe set the option to `false

[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods

aaron.ballman added inline comments. Comment at: test/clang-tidy/bugprone-parent-virtual-call.cpp:105 + +// Test virtual method is diagnosted although not overridden in parent. +class BI : public A { Typo: diagnosted -> diagnosed Comment at: te

[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods

aaron.ballman added inline comments. Comment at: test/clang-tidy/bugprone-parent-virtual-call.cpp:113 + int virt_1() override { return A::virt_1(); } + // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified function name A::virt_1 refers to a function not from a direct base cl

[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods

aaron.ballman added inline comments. Comment at: clang-tidy/bugprone/ParentVirtualCallCheck.cpp:128 + diag(Member->getQualifierLoc().getSourceRange().getBegin(), + "'%0' is a grand-parent's method, not parent's. Did you mean %1?") + << CalleeName << ParentsStr

[PATCH] D44093: [BUILTINS] structure pretty printer

aaron.ballman added inline comments. Comment at: lib/CodeGen/CGBuiltin.cpp:935 +static llvm::Value *dumpRecord(CodeGenFunction &CGF, QualType RType, + Value*& RecordPtr, CharUnits Align, + Value *Func, int Lvl) Form

[PATCH] D44559: [Sema] Wrong width of result of mul operation

aaron.ballman added a comment. In https://reviews.llvm.org/D44559#1040398, @craig.topper wrote: > gcc also warns for this > > short foo(char a) { > > return a * a; > > } > > Despite the fact that the char would be promoted to int, the upper bits are > just sign bits and the multiply result sti

[PATCH] D44559: [Sema] Wrong width of result of mul operation

aaron.ballman added a comment. In https://reviews.llvm.org/D44559#1040435, @craig.topper wrote: > Fair point, what is the default signedness of char? It's decided by target architecture. ARM and PPC often use unsigned, x86 often uses signed. > FWIW, all these warn in gcc. So they seem to be

[PATCH] D44589: [Sema] Make deprecation fix-it replace all multi-parameter ObjC method slots.

aaron.ballman added a reviewer: rsmith. aaron.ballman added inline comments. Comment at: clang/include/clang/Sema/DelayedDiagnostic.h:198-199 +assert(Kind == Availability && "Not an availability diagnostic."); +return MultiSourceLocation(ArrayRef( +AvailabilityDat

[PATCH] D44589: [Sema] Make deprecation fix-it replace all multi-parameter ObjC method slots.

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7207-7217 +} else { + FixIts.push_back(FixItHint::CreateInsertion( + SelectorLocs[I], SelectorSlotNames[I])); +} + } +} else { +

[PATCH] D44231: [clang-tidy] Check for sizeof that call functions

aaron.ballman added inline comments. Comment at: test/clang-tidy/misc-sizeof-expression.cpp:17 +enum E { E_VALUE = 0 }; + Can you add a C++11 test case using `enum class` -- I am worried that the `isInteger()` matcher will return false for that type. Also, I

[PATCH] D44155: Fix support for try_acquire_capability

aaron.ballman added a comment. Ping. https://reviews.llvm.org/D44155 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44155: Fix support for try_acquire_capability

aaron.ballman updated this revision to Diff 138900. aaron.ballman added a comment. Now: with more context! https://reviews.llvm.org/D44155 Files: lib/Analysis/ThreadSafety.cpp test/SemaCXX/warn-thread-safety-analysis.cpp Index: test/SemaCXX/warn-thread-safety-analysis.cpp

[PATCH] D44155: Fix support for try_acquire_capability

aaron.ballman added a comment. In https://reviews.llvm.org/D44155#1041584, @lebedev.ri wrote: > Please upload patches with full context (`-U9`) :) Ugh, yes. TortoiseSVN has no option for this, so I perpetually forget. https://reviews.llvm.org/D44155 ___

[PATCH] D44093: [BUILTINS] structure pretty printer

aaron.ballman added inline comments. Comment at: lib/Sema/SemaChecking.cpp:1122 +!PtrArgType->getPointeeType()->isRecordType()) { + this->Diag(PtrArg->getLocStart(), + diag::err_typecheck_convert_incompatible) Drop all instances of `t

[PATCH] D47157: Warning for framework headers using double quote includes

aaron.ballman added a comment. In https://reviews.llvm.org/D47157#1115445, @bruno wrote: > > Consistency would be nice, but at the same time, I don't see a good metric > > for when we'd know it's time to switch it to being on by default. I'm > > worried that it'll remain off by default forever

[PATCH] D45686: [Driver] Clean up tmp files when deleting Compilation objects

aaron.ballman accepted this revision. aaron.ballman added a comment. In https://reviews.llvm.org/D45686#1115836, @dstenb wrote: > Any more comments or concerns, or can I land this? None from me; you're good to land it. Any further comments can be handled post-commit. https://reviews.llvm.org

[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning

aaron.ballman added inline comments. Comment at: include/clang/AST/Decl.h:2212-2213 + bool isCpuDispatchMultiVersion() const; + bool isCpuSpecificMultiVersion() const; + Pedantic nit: CPU instead of Cpu? Comment at: include/clang/Basic/Attr

[PATCH] D47157: Warning for framework headers using double quote includes

aaron.ballman added inline comments. Comment at: include/clang/Basic/DiagnosticLexKinds.td:713 +def warn_quoted_include_in_framework_header : Warning< + "double-quoted include \"%0\" in framework header, expected system style include" + >, InGroup, DefaultIgnore;

[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning

aaron.ballman added inline comments. Comment at: include/clang/Basic/Attr.td:851 + let Spellings = [GCC<"cpu_specific">]; + let Args = [VariadicIdentifierArgument<"Cpus">]; + let Subjects = SubjectList<[Function]>; erichkeane wrote: > aaron.ballman wrote: > >

[PATCH] D46602: [clang-tidy] Store checks profiling info as YAML files

aaron.ballman added inline comments. Comment at: clang-tidy/ClangTidyProfiling.cpp:72 + if (EC) { +llvm::errs() << "Error opening output file'" << Storage->StoreFilename + << "': " << EC.message() << "\n"; Missing a whitespace before the quot

[PATCH] D45835: Add new driver mode for dumping compiler options

aaron.ballman closed this revision. aaron.ballman marked 4 inline comments as done. aaron.ballman added a comment. Committed in r333653. Comment at: test/Frontend/compiler-options-dump.cpp:3 +// RUN: %clang_cc1 -compiler-options-dump -std=c++17 %s -o - | FileCheck %s --check-p

[PATCH] D47157: Warning for framework headers using double quote includes

aaron.ballman added inline comments. Comment at: test/Modules/double-quotes.m:27-29 +// CHECK: double-quoted include "A0.h" in framework header, expected angle-bracketed include instead +// CHECK: double-quoted include "B.h" in framework header, expected angle-bracketed includ

[PATCH] D47157: Warning for framework headers using double quote includes

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D47157 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/

[PATCH] D47201: [CUDA] Implement nv_weak attribute for functions

aaron.ballman added a comment. In https://reviews.llvm.org/D47201#1119249, @tra wrote: > IIUIC, nv_weak is a synonym for weak (why, oh why did they need > it?) > You may need to hunt down and change few other places that deal with the > weak attribute. > E.g.: > https://github.com/llvm-proje

[PATCH] D47201: [CUDA] Implement nv_weak attribute for functions

aaron.ballman added a comment. In https://reviews.llvm.org/D47201#1119947, @Hahnfeld wrote: > In https://reviews.llvm.org/D47201#1119254, @aaron.ballman wrote: > > > In https://reviews.llvm.org/D47201#1119249, @tra wrote: > > > > > IIUIC, nv_weak is a synonym for weak (why, oh why did they need

[PATCH] D46602: [clang-tidy] Store checks profiling info as YAML files

aaron.ballman added inline comments. Comment at: clang-tidy/ClangTidy.h:230-232 +/// \param StoreCheckProfile If provided, and EnableCheckProfile is true, +/// the profile will not be outputted to the stderr, but instead will be stored +/// as JSON file in the specified directory

[PATCH] D45679: [clang-tidy] Add ExprMutationAnalyzer, that analyzes whether an expression is mutated within a statement.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Aside from a minor commenting nit, this LGTM. Comment at: clang-tidy/utils/ExprMutationAnalyzer.cpp:78-81 + // TODO: also handle these cases: + // - `Exp` is

[PATCH] D46805: If some platforms do not support an attribute, we should exclude the platform

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a nit with one of the tests. Once you've updated the patch and verified that check-clang passes all tests, I can commit for you next week when I'm back from meeti

[PATCH] D46602: [clang-tidy] Store checks profiling info as JSON files

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Comment at: clang-tidy/ClangTidyProfiling.cpp:47 +void ClangTidyProfiling::printAsJSON(llvm::raw_ostream &OS) { + OS << "{\n"; lebedev.ri wrote: > aaron.ballman wrote: > > I'm not kee

[PATCH] D47804: [CUDA] Replace 'nv_weak' attributes in CUDA headers with 'weak'.

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM as well, for what little it's worth. :-) https://reviews.llvm.org/D47804 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

aaron.ballman added subscribers: hans, joerg. aaron.ballman added a comment. In https://reviews.llvm.org/D47290#1115352, @jfb wrote: > Hopefully this makes sense? I'm not sure I summarize my context very well. > I'm happy to talk about it in-person next week too. I appreciate the in-person con

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

aaron.ballman added a comment. In https://reviews.llvm.org/D47290#1124866, @hans wrote: > If we really want to special-case NSInteger, and given that you're targeting > a specific wide-spread pattern maybe that's the right thing to do, I think we > should make -Wformat accept (move the warning

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

aaron.ballman added a comment. In https://reviews.llvm.org/D47290#1124956, @hans wrote: > In https://reviews.llvm.org/D47290#1124933, @aaron.ballman wrote: > > > In https://reviews.llvm.org/D47290#1124866, @hans wrote: > > > > > If we really want to special-case NSInteger, and given that you're

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

aaron.ballman added a comment. In https://reviews.llvm.org/D47290#1124991, @hans wrote: > In https://reviews.llvm.org/D47290#1124964, @aaron.ballman wrote: > > > In https://reviews.llvm.org/D47290#1124956, @hans wrote: > > > > > In https://reviews.llvm.org/D47290#1124933, @aaron.ballman wrote: >

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

aaron.ballman added a comment. In https://reviews.llvm.org/D47290#1125028, @aaron.ballman wrote: > Okay, that's fair, but the vendor-specific type for my Windows example is > spelled `DWORD`. I'm really worried that this special case will become a > precedent and we'll wind up with -Wformat bei

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

aaron.ballman added a comment. In https://reviews.llvm.org/D47290#1127190, @jfb wrote: > In https://reviews.llvm.org/D47290#1126443, @aaron.ballman wrote: > > > In https://reviews.llvm.org/D47290#1125028, @aaron.ballman wrote: > > > > > Okay, that's fair, but the vendor-specific type for my Windo

[PATCH] D47435: Add __builtin_signbit semantic checking

aaron.ballman added a comment. Ping Repository: rC Clang https://reviews.llvm.org/D47435 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47435: Add __builtin_signbit semantic checking

aaron.ballman updated this revision to Diff 150967. aaron.ballman marked 5 inline comments as done. aaron.ballman added a comment. Updating based on review feedback. https://reviews.llvm.org/D47435 Files: lib/Sema/SemaChecking.cpp test/Sema/builtins.c Index: test/Sema/builtins.c =

[PATCH] D47435: Add __builtin_signbit semantic checking

aaron.ballman added inline comments. Comment at: lib/Sema/SemaChecking.cpp:925 + case Builtin::BI__builtin_signbitl: if (SemaBuiltinFPClassification(TheCall, 1)) return ExprError(); lebedev.ri wrote: > The name of the function is unfortunate given th

[PATCH] D45679: [clang-tidy] Add ExprMutationAnalyzer, that analyzes whether an expression is mutated within a statement.

aaron.ballman closed this revision. aaron.ballman added a comment. I've commit in r334604 with one minor change -- I added an include for to the unit test so that `std::isspace()` would compile properly on all platforms. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45679

<    1   2   3   4   5   6   7   8   9   10   >