[PATCH] D49511: [Sema/Attribute] Check for noderef attribute

aaron.ballman added inline comments. Comment at: include/clang/Basic/AttrDocs.td:3489 + +``noderef`` is currently only supported for C style pointers and arrays and not usable for +references or Objective-C pointers. I would drop the "C style" and just say it's

[PATCH] D48100: Append new attributes to the end of an AttributeList.

aaron.ballman accepted this revision. aaron.ballman added a comment. I am still okay with this, and agree that the ordering issues are a separate thing to tackle. Repository: rC Clang https://reviews.llvm.org/D48100 ___ cfe-commits mailing list

[PATCH] D50110: Handle shared release attributes correctly

aaron.ballman added inline comments. Comment at: test/SemaCXX/warn-thread-safety-analysis.cpp:4081-4085 + void unlockExclusive() EXCLUSIVE_UNLOCK_FUNCTION(mu_) { +mu_.Unlock(); + } + + void unlockShared() SHARED_UNLOCK_FUNCTION(mu_) { Nothing calls either

[PATCH] D49910: [clang-tidy] Recognize [[clang::reinitializes]] attribute in bugprone-use-after-move

aaron.ballman added a reviewer: rsmith. aaron.ballman added a comment. Are you going to propose adding this attribute to libc++, or is this expected to only work with UDTs? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49910 ___ c

[PATCH] D50110: Handle shared release attributes correctly

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: test/SemaCXX/warn-thread-safety-analysis.cpp:4081-4085 + void unlockExclusive() EXCLUSIVE_UNLOCK_FUNCTION(mu_) { +mu_.Unlock(); + } + +

[PATCH] D50110: Handle shared release attributes correctly

aaron.ballman added a comment. In https://reviews.llvm.org/D50110#1187771, @aaronpuchert wrote: > Thanks for the review! Could you commit for me again? I can, but it might also be a good idea for you to obtain your own commit privileges at this point (https://llvm.org/docs/DeveloperPolicy.htm

[PATCH] D49910: [clang-tidy] Recognize [[clang::reinitializes]] attribute in bugprone-use-after-move

aaron.ballman added a comment. In https://reviews.llvm.org/D49910#1187492, @mboehme wrote: > In https://reviews.llvm.org/D49910#1187455, @aaron.ballman wrote: > > > Are you going to propose adding this attribute to libc++, or is this > > expected to only work with UDTs? > > > I don't have any ex

[PATCH] D50110: Handle shared release attributes correctly

aaron.ballman closed this revision. aaron.ballman added a comment. In https://reviews.llvm.org/D50110#1187828, @aaronpuchert wrote: > For now I think I'm done fixing things in the thread safety analysis, but if > I see myself contributing more in the longer term, I will definitely try to > obta

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

aaron.ballman added a comment. I think this is close. If @alexfh doesn't chime in on the open question in the next few days, I would say the check is ready to go in and we can address the open question in follow-up commits. Comment at: clang-tidy/readability/MagicNumbersCheck

[PATCH] D45444: [clang-tidy] implement new check for const-correctness

aaron.ballman added a comment. I'm sorry for the delay in reviewing this; I'm not certain how it fell off my radar for so long! Comment at: clang-tidy/cppcoreguidelines/ConstCheck.cpp:32 + * + * Handle = either a pointer or reference + * Value = everything else (Type variable

[PATCH] D45444: [clang-tidy] implement new check for const-correctness

aaron.ballman added a comment. The functionality is looking good, aside from a few small nits remaining. However, I'm wondering how this should integrate with other const-correctness efforts like `readability-non-const-parameter`? Also, I'm wondering how this check performs over a large code ba

[PATCH] D42561: [PR36008] Avoid -Wsign-compare warning for enum constants in typeof expressions

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, aside from a minor commenting nit. Comment at: lib/Sema/SemaChecking.cpp:8960 +// Avoid warning about comparison of integers with different signs when

[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers

aaron.ballman added a comment. It sounds to me like the check is working as designed and that the user code was already broken, but it happened to work at runtime because the stars aligned properly for the user. Am I misunderstanding something? Repository: rC Clang https://reviews.llvm.org/

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

aaron.ballman requested changes to this revision. aaron.ballman added a comment. This revision now requires changes to proceed. A few comments were not applied, and I'd like a more descriptive name for the check (especially if we plan to generalize this). Comment at: clang-tid

[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/ModernizeTidyModule.cpp:50 +CheckFactories.registerCheck( +"modernize-avoid-functional"); CheckFactories.registerCheck( alexfh wrote: > aaron.ballman wrote: > > I'm not keen on

[PATCH] D42887: [Driver] Add option to manually control discarding value names in LLVM IR.

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

[PATCH] D43120: [clang-tidy] New checker for exceptions that are created but not thrown

aaron.ballman added a comment. One concern I have is with RAII objects with "exception" in the name. You may already properly handle this, but I'd like to see a test case like: struct ExceptionRAII { ExceptionRAII() {} ~ExceptionRAII() {} }; void foo() { ExceptionRAII E; //

[PATCH] D43120: [clang-tidy] New checker for exceptions that are created but not thrown

aaron.ballman added inline comments. Comment at: clang-tidy/misc/ThrowKeywordMissingCheck.cpp:24 + + auto CtorInitializerList = + cxxConstructorDecl(hasAnyConstructorInitializer(anything())); Eugene.Zelenko wrote: > Please don't use auto where type could no

[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:26 +.bind("base")), + anyOf(isClass(), ast_matchers::isStruct()), + ast_matchers::isDe

[PATCH] D43095: Make attribute-target on a Definition-after-use update the LLVM attributes

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. This looks reasonable to me, but you should wait a few days to commit in case someone more into CodeGen has comments. Your choice on updating the code you factored into a functio

[PATCH] D41553: Support parsing double square-bracket attributes in ObjC

aaron.ballman updated this revision to Diff 133791. aaron.ballman added a comment. Added the remaining ObjC, NS, and CF attributes. I think that's the last of the Apple-related attributes (I plan to continue to work on the other attributes as well, but in separate patches). @rjmccall, do you see

[PATCH] D41553: Support parsing double square-bracket attributes in ObjC

aaron.ballman closed this revision. aaron.ballman added a comment. Thanks for the detailed review! I've commit in r324890. https://reviews.llvm.org/D41553 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/

[PATCH] D43120: [clang-tidy] New checker for exceptions that are created but not thrown

aaron.ballman added a comment. In https://reviews.llvm.org/D43120#1005100, @Szelethus wrote: > I also came up with this problem: > >RegularException funcReturningExceptioniTest(int i) { > return RegularException(); >} > >void returnedValueTest() { > funcReturningException

[PATCH] D43120: [clang-tidy] New checker for exceptions that are created but not thrown

aaron.ballman requested changes to this revision. aaron.ballman added a comment. This revision now requires changes to proceed. I apologize for not noticing this important detail earlier -- I think this check should live under `bugprone` rather than `misc`. Other than where it lives (and the rel

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

aaron.ballman added inline comments. Comment at: clang-tidy/utils/Matchers.h:16 +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Analysis/CFG.h" tbourvon wrote: > aaron.ballman wrote: > > This will require linking in the clangAnalysis library as

[PATCH] D41880: Adding nocf_check attribute for cf-protection fine tuning

aaron.ballman added a reviewer: aaron.ballman. aaron.ballman added inline comments. Comment at: include/clang/Basic/Attr.td:2089 +def AnyX86NoCfCheck : InheritableAttr, TargetSpecificAttr{ + let Spellings = [GCC<"nocf_check">]; + let Documentation = [AnyX86NoCfCheckDocs]; -

[PATCH] D43120: [clang-tidy] New checker for exceptions that are created but not thrown

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

[PATCH] D41880: Adding nocf_check attribute for cf-protection fine tuning

aaron.ballman added a comment. One thing I notice is that GCC trunk requires passing the `-fcf-protection` flag in order to enable the attribute. Do we wish to do the same? Comment at: include/clang/Basic/Attr.td:2089 +def AnyX86NoCfCheck : InheritableAttr, TargetSpecificAttr{

[PATCH] D43321: Improve documentation for attribute artificial

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

[PATCH] D41880: Adding nocf_check attribute for cf-protection fine tuning

aaron.ballman added inline comments. Comment at: lib/Sema/SemaDeclAttr.cpp:1990 -bool Sema::CheckNoReturnAttr(const AttributeList &Attrs) { - if (!checkAttributeNumArgs(*this, Attrs, 0)) { -Attrs.setInvalid(); +static void handleNoCfCheckAttr(Sema &S, Decl *D, const Attrib

[PATCH] D41880: Adding nocf_check attribute for cf-protection fine tuning

aaron.ballman added inline comments. Comment at: lib/Sema/SemaDeclAttr.cpp:1990 -bool Sema::CheckNoReturnAttr(const AttributeList &Attrs) { - if (!checkAttributeNumArgs(*this, Attrs, 0)) { -Attrs.setInvalid(); +static void handleNoCfCheckAttr(Sema &S, Decl *D, const Attrib

[PATCH] D43248: [Attr] Fix printing of parameter indices in attributes

aaron.ballman added inline comments. Comment at: include/clang/Basic/Attr.td:182 + // it would always be false. + string DisallowImplicitThisParamName = disallowImplicitThisParamName; +} Is there much benefit to forcing the attribute author to pick a name for t

[PATCH] D43248: [Attr] Fix printing of parameter indices in attributes

aaron.ballman added inline comments. Comment at: lib/Sema/SemaDeclAttr.cpp:355-357 } + else if (DisallowImplicitThisParam) +*DisallowImplicitThisParam = false; jdenny wrote: > aaron.ballman wrote: > > Formatting is off here -- the `else if` should go up a

[PATCH] D43359: Clean up 'target' attribute diagnostics

aaron.ballman added inline comments. Comment at: test/Sema/attr-target-ast.c:1 +// RUN: %clang_cc1 -triple x86_64-linux-gnu -ast-dump %s | FileCheck %s + Can you drop the svn props from this file? Comment at: test/Sema/attr-target.c:9-10 int _

[PATCH] D41880: Adding nocf_check attribute for cf-protection fine tuning

aaron.ballman added inline comments. Comment at: test/Sema/attr-nocf_check.c:18-20 + FuncPointerWithNoCfCheck fNoCfCheck = f; // no-warning + (*fNoCfCheck)(); // no-warning + f = fNoCfCheck;// no-warning oren_ben_s

[PATCH] D43248: [Attr] Fix printing of parameter indices in attributes

aaron.ballman added inline comments. Comment at: include/clang/Basic/Attr.td:182 + // it would always be false. + string DisallowImplicitThisParamName = disallowImplicitThisParamName; +} jdenny wrote: > jdenny wrote: > > aaron.ballman wrote: > > > Is there much

[PATCH] D41880: Adding nocf_check attribute for cf-protection fine tuning

aaron.ballman added inline comments. Comment at: test/Sema/attr-nocf_check.c:18-20 + FuncPointerWithNoCfCheck fNoCfCheck = f; // no-warning + (*fNoCfCheck)(); // no-warning + f = fNoCfCheck;// no-warning aaron.ball

[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:33 + this); + Finder->addMatcher(callExpr(callee(functionDecl(hasName("std::mem_fun" + .bind("mem_fun_call"), mass

[PATCH] D43359: Clean up 'target' attribute diagnostics

aaron.ballman added inline comments. Comment at: test/Sema/attr-target.c:9-10 int __attribute__((target("fpmath=387"))) walrus() { return 4; } //expected-warning {{ignoring unsupported 'fpmath=' in the target attribute string}} +// expected-warning@+1 {{'target' attribute igno

[PATCH] D43359: Clean up 'target' attribute diagnostics

aaron.ballman accepted this revision. aaron.ballman added a comment. Aside from a minor wording nit, LGTM! Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2448-2449 def warn_unsupported_target_attribute -: Warning<"ignoring %select{unsupported|duplicate}0" -

[PATCH] D43392: [clang-tidy] Add Fuchsia checker for visibility attributes

aaron.ballman added inline comments. Comment at: clang-tidy/fuchsia/AddVisibilityCheck.cpp:32 + if (Name.empty()) return; + Finder->addMatcher(functionDecl(allOf(hasName(Name), isDefinition(), +unless(hasVisibilityAttr( --

[PATCH] D43248: [Attr] Fix printing of parameter indices in attributes

aaron.ballman added inline comments. Comment at: include/clang/Basic/Attr.td:182 + // it would always be false. + string DisallowImplicitThisParamName = disallowImplicitThisParamName; +} jdenny wrote: > aaron.ballman wrote: > > jdenny wrote: > > > jdenny wrote:

[PATCH] D43248: [Attr] Fix printing of parameter indices in attributes

aaron.ballman added inline comments. Comment at: include/clang/Basic/Attr.td:182 + // it would always be false. + string DisallowImplicitThisParamName = disallowImplicitThisParamName; +} jdenny wrote: > aaron.ballman wrote: > > jdenny wrote: > > > aaron.ballman

[PATCH] D43392: [clang-tidy] Add Fuchsia checker for visibility attributes

aaron.ballman added inline comments. Comment at: clang-tidy/fuchsia/AddVisibilityCheck.cpp:36-39 + StringRef(Options.get("Names", "")).split(NamesRefs, ","); + for (const auto &Name : NamesRefs) { +if (!Name.empty()) Names.push_back(Name); + } Rather than

[PATCH] D43392: [clang-tidy] Add Fuchsia checker for visibility attributes

aaron.ballman added inline comments. Comment at: clang-tidy/fuchsia/AddVisibilityCheck.cpp:68 + diag(MatchedDecl->getLocStart(), + "visibility attribute not set for specified function") + << MatchedDecl jakehehrlich wrote: > aaron.ballman

[PATCH] D42840: [docs] Fix duplicate arguments for JoinedAndSeparate

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. This LGTM, but you should wait a few days in case Richard has thoughts or concerns. Repository: rC Clang https://reviews.llvm.org/D42840 __

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

aaron.ballman added a comment. To be clear -- my concerns were mostly that it seems messy for this sort of thing to require three different diagnostic entries and somewhat convoluted logic to select them -- if we can find a way to generalize this, that'd be better. Repository: rC Clang htt

[PATCH] D43500: [clang-tidy]: modernize-use-default-member-init: Remove trailing comma and colon.

aaron.ballman added a comment. > It seems, when they are apply directly from clang-tidy, the RefactoringTool > engine is smart enough to remove trailing tokens. However, when fixes are > exported, clang-apply-replacements cannot do the same trick and will lead > trailing commas and colon Is th

[PATCH] D43392: [clang-tidy] Add Fuchsia checker for visibility attributes

aaron.ballman added inline comments. Comment at: clang-tidy/fuchsia/AddVisibilityCheck.cpp:23-25 + // if (const auto *D = Node.getDefinition()) { + // return D->getExplicitVisibility(NamedDecl::VisibilityForType) == V; + // } Can remove these comments. ===

[PATCH] D43538: [clang-tidy] Update run-clang-tidy.py with config arg

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Aside from a nit with the help text, this LGTM. Comment at: clang-tidy/tool/run-clang-tidy.py:188-189 + '

[PATCH] D43581: [clang-tidy/google] Fix the Objective-C global variable declaration check 🔧

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM with a few additional test cases. It'd be nice if the style guide linked actually described what a "good" prefix is rather than make the reader guess. Co

[PATCH] D43581: [clang-tidy/google] Fix the Objective-C global variable declaration check 🔧

aaron.ballman added a comment. In https://reviews.llvm.org/D43581#1014903, @stephanemoore wrote: > In https://reviews.llvm.org/D43581#1014664, @aaron.ballman wrote: > > > LGTM with a few additional test cases. > > > > It'd be nice if the style guide linked actually described what a "good" > > pr

[PATCH] D43500: [clang-tidy]: modernize-use-default-member-init: Remove trailing comma and colon.

aaron.ballman added a comment. In https://reviews.llvm.org/D43500#1015208, @jdemeule wrote: > In https://reviews.llvm.org/D43500#1013558, @malcolm.parsons wrote: > > > In https://reviews.llvm.org/D43500#1013497, @aaron.ballman wrote: > > > > > Is there a way to make clang-apply-replacements smart

[PATCH] D41880: Adding nocf_check attribute for cf-protection fine tuning

aaron.ballman added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2691 +def warn_nocf_check_attribute_ignored : + Warning<"nocf_check attribute ignored. Use -fcf-prtection flag to enable it">, + InGroup; Diagnostics are not complete s

[PATCH] D43581: [clang-tidy/google] Fix the Objective-C global variable declaration check 🔧

aaron.ballman added inline comments. Comment at: clang-tidy/google/GlobalVariableDeclarationCheck.cpp:92 + "an appropriate prefix (see " + "http://google.github.io/styleguide/objcguide#constants).") << Decl->getName() << generateFixItHint(Decl, true); ---

[PATCH] D43581: [clang-tidy/google] Improve the Objective-C global variable declaration check 🔧

aaron.ballman added inline comments. Comment at: clang-tidy/google/GlobalVariableDeclarationCheck.cpp:92 + "an appropriate prefix (see " + "http://google.github.io/styleguide/objcguide#constants).") << Decl->getName() << generateFixItHint(Decl, true); ---

[PATCH] D41880: Adding nocf_check attribute for cf-protection fine tuning

aaron.ballman added inline comments. Comment at: test/Sema/attr-nocf_check.c:1 +// RUN: %clang_cc1 -verify -fcf-protection=branch -target-feature +ibt -fsyntax-only %s + You likely need to specify an explicit triple here, or some of the bots fail this test beca

[PATCH] D43581: [clang-tidy/google] Improve the Objective-C global variable declaration check 🔧

aaron.ballman accepted this revision. aaron.ballman added a comment. This LGTM! Do you need someone to commit on your behalf? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43581 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D43747: [Attr] Fix pointer_with_type_tag assert fail for variadic

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, thank you for the fix! https://reviews.llvm.org/D43747 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.l

[PATCH] D43748: [Attr] Fix paren, comma, and omitted arg printing in some cases

aaron.ballman added a comment. In general, I think the idea is reasonable, but in practice I'm worried about the attributes with custom parsing. Sometimes the parsing requires those extra bits and I'm not certain of how best to address that. Comment at: test/Misc/ast-print-ob

[PATCH] D43747: [Attr] Fix pointer_with_type_tag assert fail for variadic

aaron.ballman added a comment. Why is this dependent on https://reviews.llvm.org/D43248? https://reviews.llvm.org/D43747 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43747: [Attr] Fix pointer_with_type_tag assert fail for variadic

aaron.ballman added a comment. In https://reviews.llvm.org/D43747#1018814, @jdenny wrote: > Hi Aaron. Thanks for accepting. I do not have commit privileges. Would > you please commit this (and any other patches you accept) for me? I'm happy to do so. Just to double-check though, there's nothin

[PATCH] D43749: [Attr] Fix alloc_size's diags to report arg idx not value

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Aside from a minor testcase nit, this LGTM. Why is this dependent on https://reviews.llvm.org/D43248? Comment at: test/Sema/alloc-size.c:3 -void *fail1(int a

[PATCH] D43749: [Attr] Fix alloc_size's diags to report arg idx not value

aaron.ballman added a comment. In https://reviews.llvm.org/D43749#1018825, @jdenny wrote: > In https://reviews.llvm.org/D43749#1018818, @aaron.ballman wrote: > > > Aside from a minor testcase nit, this LGTM. Why is this dependent on > > https://reviews.llvm.org/D43248? > > > The dependency goes

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

aaron.ballman created this revision. aaron.ballman added a reviewer: rsmith. Calling convention attributes notionally appertain to the function type -- they modify the mangling of the function, change the behavior of assignment operations, etc. However, they're not currently allowed to be writte

[PATCH] D43747: [Attr] Fix pointer_with_type_tag assert fail for variadic

aaron.ballman closed this revision. aaron.ballman added a comment. Committed in r326057. If you're not already on IRC, I would recommend joining the #llvm channel on OFTC so that you can watch for build break notifications from the build bots. https://reviews.llvm.org/D43747 ___

[PATCH] D43749: [Attr] Fix alloc_size's diags to report arg idx not value

aaron.ballman added a comment. Committed (with whitespace fix for the test case) in r326058, thanks for the patch! https://reviews.llvm.org/D43749 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinf

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

aaron.ballman added a comment. (Review is incomplete, but here are some initial comments.) Comment at: include/clang/AST/Attr.h:210-212 + unsigned Idx; + bool HasThis; + bool IsValid; I think it might be best to mash these together using bit-fields: ``` unsi

[PATCH] D43749: [Attr] Fix alloc_size's diags to report arg idx not value

aaron.ballman added a comment. In https://reviews.llvm.org/D43749#1019359, @jdenny wrote: > In https://reviews.llvm.org/D43749#1018846, @aaron.ballman wrote: > > > Committed (with whitespace fix for the test case) in r326058, thanks for > > the patch! > > > Sure. Thanks for committing. > > By t

[PATCH] D43500: [clang-tidy]: modernize-use-default-member-init: Remove trailing comma and colon.

aaron.ballman added inline comments. Comment at: unittests/clang-tidy/ClangTidyTest.h:145 + + // Options. + if (Options.FormatStyle) { Extraneous whitespace. Comment at: unittests/clang-tidy/ClangTidyTest.h:147 + if (Options.FormatStyle) {

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

aaron.ballman added a comment. This fix is missing test coverage, can you add a C++11 and C++14 test to demonstrate the behavior differences? Comment at: clang-tidy/modernize/MakeSharedCheck.cpp:30 +bool MakeSharedCheck::isVersionSupported(const clang::LangOptions &LangOpts)

[PATCH] D41880: Adding nocf_check attribute for cf-protection fine tuning

aaron.ballman added inline comments. Comment at: include/clang/CodeGen/CGFunctionInfo.h:519 + /// Whether this function has nocf_check attribute. + unsigned NoCfCheck : 1; + oren_ben_simhon wrote: > aaron.ballman wrote: > > This is unfortunate -- it bumps the b

[PATCH] D33470: [clang-tidy] Add misc-default-numerics

aaron.ballman added inline comments. Comment at: clang-tidy/misc/DefaultNumericsCheck.cpp:37 +void DefaultNumericsCheck::check(const MatchFinder::MatchResult &Result) { + + const auto *MatchedDecl = Result.Nodes.getNodeAs("call"); Can remove the spurious newline

[PATCH] D33547: Updated getting started guide for visual studio + cmake

aaron.ballman created this revision. It took me a while to track this down and I figured I'd save someone else the time. By default, CMake uses the 32-bit toolchain on Windows, even if generating a 64-bit solution. Given the size of Clang's code base, this can lead to quite a few link errors wi

[PATCH] D33563: Track whether a unary operation can overflow

aaron.ballman created this revision. We do not explicitly model integer promotions on unary operators even though those promotions happen in practice. This patch tracks the most important piece of information about the promotion on the AST node: whether the operator can overflow or not. It then

[PATCH] D33470: [clang-tidy] Add misc-default-numerics

aaron.ballman added a comment. Once you fix the typo in the check, can you run it over some large C++ code bases to see if it finds any results? Comment at: clang-tidy/misc/DefaultNumericsCheck.cpp:30 + ofClass(classTemplateSpecializationDecl( + h

[PATCH] D33547: Updated getting started guide for visual studio + cmake

aaron.ballman closed this revision. aaron.ballman added a comment. Commit in r303913 https://reviews.llvm.org/D33547 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33531: Clang-tidy readability: avoid const value return

aaron.ballman added a comment. I'm not opposed to this check, but I'm not keen on having a check that directly contradicts the output from another check. The `cert-dcl21-cpp` check will diagnose user's code when a postfix operator ++ or -- is *not* const-qualified. Would it make sense to silenc

[PATCH] D33531: Clang-tidy readability: avoid const value return

aaron.ballman added a comment. In https://reviews.llvm.org/D33531#767059, @alexfh wrote: > > Would it make sense to silence this diagnostic in the presence of also > > checking for cert-dcl21-cpp for such operators? > > Currently there's no mechanism in clang-tidy to express dependencies or > c

[PATCH] D33531: Clang-tidy readability: avoid const value return

aaron.ballman added a comment. In https://reviews.llvm.org/D33531#767640, @alexfh wrote: > In https://reviews.llvm.org/D33531#767628, @aaron.ballman wrote: > > > In https://reviews.llvm.org/D33531#767059, @alexfh wrote: > > > > > > Would it make sense to silence this diagnostic in the presence of

[PATCH] D33398: Mangle __unaligned in Itanium ABI

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Can you run clang-format over both the test files? Aside from that, looks good to me, but you should wait for @rsmith or @majnemer to sign off before committing. https://review

[PATCH] D33563: Track whether a unary operation can overflow

aaron.ballman marked an inline comment as done. aaron.ballman added inline comments. Comment at: lib/Sema/SemaDeclCXX.cpp:9 +VK_LValue, OK_Ordinary, Loc, true); // Construct the loop that copies all elements of this array. ahatanak wr

[PATCH] D33563: Track whether a unary operation can overflow

aaron.ballman updated this revision to Diff 100703. aaron.ballman added a comment. Addressing review comments. https://reviews.llvm.org/D33563 Files: include/clang/AST/Expr.h lib/AST/ASTDumper.cpp lib/AST/ASTImporter.cpp lib/AST/ExprConstant.cpp lib/CodeGen/CGExprScalar.cpp lib/Code

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

aaron.ballman added a comment. In https://reviews.llvm.org/D3#768332, @jyu2 wrote: > Okay this CFG version of this change. In this change I am basic using same > algorithm with -Winfinite-recursion. > > In addition to my original implementation, I add handler type checking which > basic u

[PATCH] D33537: [clang-tidy] Exception Escape Checker

aaron.ballman added a comment. In https://reviews.llvm.org/D33537#765445, @baloghadamsoftware wrote: > In https://reviews.llvm.org/D33537#764834, @Prazek wrote: > > > How is that compared to https://reviews.llvm.org/D19201 and the clang patch > > mentioned in this patch? > > > Actually, this che

[PATCH] D33735: [DebugInfo] Add ThisOrSelf attribute for emission of FlagObjectPointer.

aaron.ballman added a comment. Can you help me to understand what problem is being solved with this new attribute? Under what circumstances would the first argument be an `ImplicitParamDecl` but not an implicit this or self? https://reviews.llvm.org/D33735 __

[PATCH] D33788: Return a canonical path from getClangResourcePath()

aaron.ballman created this revision. on POSIX systems, CIndexer::getClangResourcesPath() uses dladdr() to get the path of the shared object. It seems that on some systems (in our case, OS X 10.6.8), dladdr() does not return a canonicalized path. We're getting a path like PATH/TO/CLANG/build/bin

[PATCH] D33735: [DebugInfo] Add ThisOrSelf attribute for emission of FlagObjectPointer.

aaron.ballman added a comment. In https://reviews.llvm.org/D33735#770296, @ABataev wrote: > In https://reviews.llvm.org/D33735#770288, @aaron.ballman wrote: > > > Can you help me to understand what problem is being solved with this new > > attribute? Under what circumstances would the first argu

[PATCH] D33788: Return a canonical path from getClangResourcePath()

aaron.ballman added a comment. In https://reviews.llvm.org/D33788#771070, @bruno wrote: > Hi Aaron, > > Nice catch! Any chance you can add a testcase to this? There's already a test case that covers this: Index/pch-from-libclang.c -- it was failing on OS X 10.6 for us. If you have a better tes

[PATCH] D33537: [clang-tidy] Exception Escape Checker

aaron.ballman added a comment. In https://reviews.llvm.org/D33537#771159, @baloghadamsoftware wrote: > In https://reviews.llvm.org/D33537#770264, @aaron.ballman wrote: > > > I think we should try to get as much of this functionality in > > https://reviews.llvm.org/D3 as possible; there is a

[PATCH] D33788: Return a canonical path from getClangResourcePath()

aaron.ballman added a comment. In https://reviews.llvm.org/D33788#771504, @akyrtzi wrote: > Getting the real path is notoriously slow (at least it's horrible in OSX, not > sure about linux). Since this is about dropping the '/../' part, could we do > some simple canonicalization of removing the

[PATCH] D33788: Return a canonical path from getClangResourcePath()

aaron.ballman added a comment. In https://reviews.llvm.org/D33788#771671, @bruno wrote: > > I'm unaware of any other API that canonicalizes the path, which is what > > users of this API are going to expect. > > You can also call `sys::path::remove_dots(Path, /*remove_dot_dot=*/true);` Yes, but

[PATCH] D33826: [clang-tidy] avoid pointer cast to more strict alignment check

aaron.ballman added a comment. Thank you for working on this. How does this check compare with the -Wcast-align diagnostic in the Clang frontend? I believe that warning already covers these cases, so I'm wondering what extra value is added with this check? Repository: rL LLVM https://review

[PATCH] D33825: [clang-tidy] signal handler must be plain old function check

aaron.ballman added a comment. This check is an interesting one. The rules around what is signal safe are changing for C++17 to be a bit more lenient than what the rules are for C++14. CERT's rule is written against C++14, and so the current behavior matches the rule wording. However, the *inte

[PATCH] D33735: [DebugInfo] Add ThisOrSelf attribute for emission of FlagObjectPointer.

aaron.ballman added inline comments. Comment at: include/clang/AST/Decl.h:901 +/// member functions. +unsigned ImplicitParamKind : 3; }; It's a bit strange to me that the non-parameter declaration bits now have a field for implicit parameter informati

[PATCH] D33735: [DebugInfo] Add ThisOrSelf attribute for emission of FlagObjectPointer.

aaron.ballman added inline comments. Comment at: include/clang/AST/Decl.h:901 +/// member functions. +unsigned ImplicitParamKind : 3; }; ABataev wrote: > aaron.ballman wrote: > > It's a bit strange to me that the non-parameter declaration bits now have

[PATCH] D34002: [clang-tidy] When" -fno-exceptions is used", this warning is better to be suppressed.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Aside from one minor nit, LGTM Comment at: clang-tidy/misc/NoexceptMoveConstructorCheck.cpp:23 // provide any benefit to other languages, despite being benig

[PATCH] D33735: [DebugInfo] Add ThisOrSelf attribute for emission of FlagObjectPointer.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Aside from one minor nit, LGTM! Comment at: include/clang/AST/Decl.h:901 +/// member functions. +unsigned ImplicitParamKind : 3; };

[PATCH] D33135: [ASTMatchers] Add support for floatLiterals

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: include/clang/ASTMatchers/Dynamic/VariantValue.h:335 unsigned Unsigned; +double Double; bool Boolean; Lekenstey

[PATCH] D33094: [ASTMatchers] Add clang-query support for equals matcher

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

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