[PATCH] D90282: [clang-tidy] Support IgnoredRegexp configuration to selectively suppress identifier naming checks

2020-11-25 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D90282#2415189 , @smhc wrote: > There's a build failure from this merge, looks like a typo: > > diff --git > a/clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst >

[PATCH] D91975: [clang-tidy] cppcoreguidelines Narrowing Conversions Check: detect narrowing conversions involving typedefs

2020-11-24 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 aside from some small nits, thank you for the fix! Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions.cpp:346

[PATCH] D91898: [attributes] Add a facility for defining and enforcing a Trusted Computing Base.

2020-11-24 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D91898#2413004 , @NoQ wrote: >> if the TCB-based functions are a small subset of the code then this >> attribute is better > > Yes, that's exactly the case. We want most functions to be untrusted by > default but also

[PATCH] D90188: Add support for attribute 'using_if_exists'

2020-11-23 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:442 +// Make the selection of the recovery decl deterministic. +(RealRes)->getLocation().getRawEncoding() < IIDecl->getLocation().getRawEncoding())

[PATCH] D91898: [attributes] Add a facility for defining and enforcing a Trusted Computing Base.

2020-11-23 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added reviewers: aaronpuchert, delesley. aaron.ballman added a comment. This feels an awful lot like a set of attributes we already have -- can capability attributes be used for this instead? https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#basic-concepts-capabilities The

[PATCH] D91055: [clang-tidy] Introduce misc No Integer To Pointer Cast check

2020-11-23 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D91055#2410447 , @lebedev.ri wrote: > Ping. > If there's no pushback within next 24h i will commit this and wait for > post-commit review (if any). > Thanks. FWIW, that's not the way we usually do things, so please

[PATCH] D90282: [clang-tidy] Add IgnoreShortNames config to identifier naming checks

2020-11-23 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, but please wait for @njames93 in case they have additional feedback. Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h:64 +

[PATCH] D91915: [clang-tidy] Fix RenamerClangTidy checks trying to emit a fix that isnt a valid identifier

2020-11-23 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91915/new/ https://reviews.llvm.org/D91915 ___ cfe-commits mailing list

[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

2020-11-23 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:616-619 + if (CRD->isStruct() && !isHungarianNotationOptionEnabled("TreatStructAsClass", +

[PATCH] D91918: Remove the IgnoreImplicitCastsAndParentheses traversal kind

2020-11-23 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 aside from a documentation request, thank you for this cleanup! Comment at: clang/docs/ReleaseNotes.rst:237 +- The TK_IgnoreImplicitCastsAndParentheses

[PATCH] D91916: Remove automatic traversal from forEach matcher

2020-11-23 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/D91916/new/ https://reviews.llvm.org/D91916

[PATCH] D91917: Update mode used in traverse() examples

2020-11-23 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/D91917/new/ https://reviews.llvm.org/D91917

[PATCH] D89743: Support Attr in DynTypedNode and ASTMatchers.

2020-11-23 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman edited subscribers, added: ymandel; removed: aaron.ballman. aaron.ballman added a comment. In D89743#2409115 , @sammccall wrote: > In D89743#2409001 , @sammccall wrote: > >> We didn't talk about

[PATCH] D91828: [Sema/Attribute] Ignore noderef attribute in unevaluated context

2020-11-23 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D91828#2411210 , @thejh wrote: > In D91828#2411207 , @aaron.ballman > wrote: > >> In D91828#2411203 , @thejh wrote: >> >>> @aaron.ballman

[PATCH] D91828: [Sema/Attribute] Ignore noderef attribute in unevaluated context

2020-11-23 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D91828#2411203 , @thejh wrote: > @aaron.ballman Can you land it for me? I don't have commit access. Happy to do so -- are you okay with "Jann Horn " for author attribution? Repository: rG LLVM Github Monorepo

[PATCH] D91828: [Sema/Attribute] Ignore noderef attribute in unevaluated context

2020-11-23 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91828/new/ https://reviews.llvm.org/D91828 ___ cfe-commits mailing list

[PATCH] D89743: Support Attr in DynTypedNode and ASTMatchers.

2020-11-20 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D89743#2406017 , @sammccall wrote: > In D89743#2363201 , @aaron.ballman > wrote: > >> In D89743#2360258 , @sammccall >> wrote: >> >>>

[PATCH] D91565: Guard init_priority attribute within libc++

2020-11-20 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM, thank you for the fixes! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91565/new/ https://reviews.llvm.org/D91565 ___

[PATCH] D91565: Guard init_priority attribute within libc++

2020-11-20 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/test/SemaCXX/init-priority-attr.cpp:26 Two foo __attribute__((init_priority(101))) ( 5, 6 ); +#if defined(__MVS__) + #if defined(SYSTEM) Rather than using the preprocessor, you can assign a prefix to be

[PATCH] D91828: [Sema/Attribute] Ignore noderef attribute in unevaluated context

2020-11-20 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/test/Frontend/noderef.c:71 + x = sizeof(s->a + (s->b)); // ok + // Nested struct access Can you add tests for the weird situations where the expression actually is evaluated? e.g., ``` struct S {

[PATCH] D91872: [libTooling] Update Transformer's `node` combinator to include the trailing semicolon for decls.

2020-11-20 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D91872#2408321 , @ymandel wrote: > In D91872#2408278 , @aaron.ballman > wrote: > >> Drive-by question from the peanut gallery, sorry if this is an ignorant one >> -- not all

[PATCH] D91872: [libTooling] Update Transformer's `node` combinator to include the trailing semicolon for decls.

2020-11-20 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Drive-by question from the peanut gallery, sorry if this is an ignorant one -- not all declarations have a trailing semicolon; is that handled properly? e.g., `int x;` has a trailing semicolon but `int x, y;` only has a trailing semicolon for one of the two

[PATCH] D89651: [clang-tidy] Add bugprone-suspicious-memory-comparison check

2020-11-20 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, thank you for the new check! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89651/new/ https://reviews.llvm.org/D89651

[PATCH] D91035: [NFC, Refactor] Convert FunctionDefinitionKind from DeclSpech.h to a scoped enum

2020-11-20 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/include/clang/Sema/DeclSpec.h:1756 /// a function. -enum FunctionDefinitionKind { - FDK_Declaration, - FDK_Definition, - FDK_Defaulted, - FDK_Deleted +enum class FunctionDefinitionKind : unsigned char { + Declaration,

[PATCH] D91009: [clang-tidy] Include std::basic_string_view in readability-redundant-string-init.

2020-11-20 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/D91009/new/ https://reviews.llvm.org/D91009

[PATCH] D91047: Add a call super attribute plugin example

2020-11-20 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman closed this revision. aaron.ballman added a comment. In D91047#2407091 , @psionic12 wrote: > @aaron.ballman That would be nice if your could help, and `Yafei Liu > ` is okay. Thank you for the new example, I've commit on your behalf in

[PATCH] D91639: Add documentation illustrating use of IgnoreUnlessSpelledInSource

2020-11-20 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, thank you for the new documentation! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91639/new/

[PATCH] D91789: [clang-tidy] find/fix unneeded trailing semicolons in macros

2020-11-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman requested changes to this revision. aaron.ballman added a comment. This revision now requires changes to proceed. The request in D90180 was to not add a check for each kind of problematic semicolon situation but to instead make a single check that

[PATCH] D91639: Add documentation illustrating use of IgnoreUnlessSpelledInSource

2020-11-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/docs/LibASTMatchersReference.html:91-94 +This mode is hard to use correctly and +requires more development iteration because it means +that the user must write AST Matchers to explicitly traverse or ignore nodes +which are

[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

2020-11-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:237 +std::string Val = HNOption.General.lookup(Opt.first); +if (Val.empty()) { + HNOption.General.insert({Opt.first, Opt.second.str()});

[PATCH] D75229: [clang-tidy] Add signal-in-multithreaded-program check

2020-11-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/SignalInMultithreadedProgramCheck.cpp:57 +const MatchFinder::MatchResult ) { + bool IsPosix = PP->isMacroDefined("_POSIX_C_SOURCE") || +

[PATCH] D90282: [clang-tidy] Add IgnoreShortNames config to identifier naming checks

2020-11-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D90282#2395956 , @smhc wrote: > I updated it to use XXShortSizeThreshold. > > But I was thinking.. perhaps a more generic approach could be > "xxIgnoreRegex", this would allow you to do things such as : > >

[PATCH] D91009: [clang-tidy] Include std::basic_string_view in readability-redundant-string-init.

2020-11-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp:25 +struct basic_string_view { + using size_type = unsigned; + nit: `using size_type = decltype(sizeof(0));`

[PATCH] D91639: Add documentation illustrating use of IgnoreUnlessSpelledInSource

2020-11-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Thank you for the documentation effort! Comment at: clang/docs/LibASTMatchersReference.html:100 + +Traverse Mode + I think this section should go above the Node Matchers section in the document so that Node Matchers stays

[PATCH] D91047: Add a call super attribute plugin example

2020-11-19 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, thank you for the new plugin example! Do you need me to commit on your behalf? If so, are you okay with `Yafei Liu ` for attribution? Repository: rG LLVM Github

[PATCH] D91485: [clang-tidy] ElseAfterReturn check wont suggest fixes if preprocessor branches are involved

2020-11-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Comment at: clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return-pp-no-crash.cpp:1 +// RUN: clang-tidy %s -checks=-*,readability-else-after-return -- -std=c++11 +

[PATCH] D91565: Guard init_priority attribute within libc++

2020-11-18 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:384 +def ExcludeTarget : TargetSpec { + let CustomCode = [{ !Target.getTriple().isOSzOS() }]; This is not a very descriptive name -- if this is only supposed to be used for

[PATCH] D91047: Add a call super attribute plugin example

2020-11-18 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/docs/ClangPlugins.rst:116 +Defining CallSuperAttr +== psionic12 wrote: > After a whole day's research of `Sphinx`, I figured out that > `ClangPlugins.rst` is the "proto-type" of >

[PATCH] D91543: [clang-tidy] Improving bugprone-sizeof-expr check.

2020-11-18 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-sizeof-expression.cpp:317 + sum = sizeof(PtrArray) / sizeof(PtrArray1[0]); + // There is no warning for 'sizeof(T*)/sizeof(Q)' case. +

[PATCH] D91015: [clang-tidy] Extend bugprone-string-constructor-check to std::string_view.

2020-11-18 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/D91015/new/ https://reviews.llvm.org/D91015

[PATCH] D91485: [clang-tidy] ElseAfterReturn check wont suggest fixes if preprocessor branches are involved

2020-11-18 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 aside from the testing request. Comment at: clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp:313 +#endif +}

[PATCH] D91592: [ASTMatchers] Fix typo for hasAnyOverloadedOperatorName

2020-11-18 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman closed this revision. aaron.ballman added a comment. Thank you for the fix, I've commit on your behalf in 871fe71f2951cb19421a2b47ddb54ed6b3c8cba2 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D91630: [Parse] Add parsing support for C++ attributes on using-declarations

2020-11-17 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D91630#2400731 , @rsmith wrote: > Do `[[deprecated]]` and `[[maybe_unused]]` now work for > //using-declarator//s? If so, a test for that would be useful. I think the answer is yes and no, respectively (at least in

[PATCH] D91495: [clang-tidy] false-positive for bugprone-redundant-branch-condition in case of passed-by-ref params

2020-11-17 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Thank you for working on this! Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp:10 +bool tryToExtinguish(bool&); +bool tryToExtinguishByVal(bool &); void tryPutFireOut(); Did you

[PATCH] D91485: [clang-tidy] ElseAfterReturn check wont suggest fixes if preprocessor branches are involved

2020-11-17 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp:118 static void removeElseAndBrackets(DiagnosticBuilder , ASTContext , - const Stmt *Else, SourceLocation ElseLoc) { +

[PATCH] D91543: [clang-tidy] Improving bugprone-sizeof-expr check.

2020-11-17 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D91543#2400051 , @balazske wrote: > This checker has multiple weaknesses. There are more cases when the warnings > should not appear (probably if the argument of `sizeof` is a template > parameter), or more than one

[PATCH] D89651: [clang-tidy] Add bugprone-suspicious-memory-comparison check

2020-11-17 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.cpp:76 + "values|the members of the object}1 " + "manually") +<< PointeeQualifiedType << (PointeeType->isRecordType() ? 1 :

[PATCH] D91630: [Parse] Add parsing support for C++ attributes on using-declarations

2020-11-17 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/test/Parser/cxx0x-attributes.cpp:134 -[[]] using ns::i; // expected-error {{an attribute list cannot appear here}} +[[]] using ns::i; [[unknown]] using namespace ns; // expected-warning {{unknown attribute 'unknown'

[PATCH] D91630: [Parse] Add parsing support for C++ attributes on using-declarations

2020-11-17 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Can you mention this change in the release notes? Also, should we document it explicitly in the Language Extensions documentation, or do you think this is too tiny of a feature to warrant that? Comment at:

[PATCH] D90982: Ignore implicit nodes in IgnoreUnlessSpelledInSource mode

2020-11-17 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, thank you! Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:164 auto *LambdaNode = dyn_cast_or_null(StmtNode); - if (LambdaNode &&

[PATCH] D90984: Update matchers to be traverse-aware

2020-11-17 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, thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90984/new/ https://reviews.llvm.org/D90984

[PATCH] D90984: Update matchers to be traverse-aware

2020-11-17 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:4083 +return false; + return InnerMatcher.matches(*Arg->IgnoreParenImpCasts(), Finder, Builder); } This probably shouldn't compile given that there's no

[PATCH] D91311: Add new 'preferred_name' attribute.

2020-11-17 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D91311#2398526 , @rsmith wrote: > In D91311#2398144 , @ldionne wrote: > >> I think that the fact we need to re declare everything shows how the >> ergonomics would be better if we

[PATCH] D90851: [clang-tidy] Extending bugprone-signal-handler with POSIX functions.

2020-11-17 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/system-header-posix-api.h:1 -//===--- signal.h - Stub header for tests ---*- C++ -*-===// +//===--- system-header-posix-api.h - Stub header for tests

[PATCH] D91543: [clang-tidy] Improving bugprone-sizeof-expr check.

2020-11-17 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 aside from a testing request. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-sizeof-expression.cpp:301 + sum += sizeof(PtrArray) /

[PATCH] D91592: [ASTMatchers] Fix typo for hasAnyOverloadedOperatorName

2020-11-17 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! Do you need me to commit on your behalf? If so, is `Keishi Hattori ` the correct attribution you'd like me to use? Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D90982: Ignore implicit nodes in IgnoreUnlessSpelledInSource mode

2020-11-16 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Thank you for this, I think it's mostly looking good! Comment at: clang/include/clang/AST/ASTNodeTraverser.h:88 void Visit(const Decl *D) { +if (Traversal != TK_AsIs && D->isImplicit()) + return; Similar to the

[PATCH] D90042: [clang-tidy] performance-unnecessary-copy-initialization: Check for const reference arguments that are replaced template parameter type.

2020-11-16 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, thank you for the fix! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90042/new/ https://reviews.llvm.org/D90042

[PATCH] D90984: Update matchers to be traverse-aware

2020-11-16 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3115 + + if (!Finder->isTraversalAsIs() && (*MatchIt)->isImplicit()) +return false; steveire wrote: > aaron.ballman wrote: > > If the traversal is not `AsIs`,

[PATCH] D90188: Add support for attribute 'using_if_exists'

2020-11-16 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D90188#2394361 , @erik.pilkington wrote: > Add support for C++11-style attributes on using-declarations. FWIW, it'd be a bit easier if those changes were split off into their own patch, as they're orthogonal to

[PATCH] D91047: Add a call super attribute plugin example

2020-11-16 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/docs/ClangPlugins.rst:117 +Defining CallSuperAttr +=== + psionic12 wrote: > aaron.ballman wrote: > > psionic12 wrote: > > > aaron.ballman wrote: > > > > aaron.ballman wrote: > > > > > The

[PATCH] D91311: Add new 'preferred_name' attribute.

2020-11-16 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: libcxx/include/regex:2520 +_LIBCPP_PREFERRED_NAME(wregex) +basic_regex { rsmith wrote: > Quuxplusone wrote: > > Why does this attribute go on the class template? Shouldn't it be an > > attribute on the

[PATCH] D91498: [NFC, Refactor] Convert TypeSpecifierSign from Specifiers.h to a scoped enum

2020-11-16 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman closed this revision. aaron.ballman added a comment. In D91498#2397251 , @tschuett wrote: > rebased Thanks! I've commit on your behalf in 7c6412e0ccf5e00a9f59c5805df9df6ff6720ed2

[PATCH] D91498: [NFC, Refactor] Convert TypeSpecifierSign from Specifiers.h to a scoped enum

2020-11-16 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D91498#2397187 , @aaron.ballman wrote: > LGTM! I'll land on your behalf momentarily. I lied. The patch doesn't apply cleanly for me -- can you rebase and upload a new diff? Repository: rG LLVM Github Monorepo

[PATCH] D91498: [NFC, Refactor] Convert TypeSpecifierSign from Specifiers.h to a scoped enum

2020-11-16 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! I'll land on your behalf momentarily. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91498/new/

[PATCH] D91506: [NFC, Refactor] Convert TypeSpecifiersPipe from Specifiers.h to a scoped enum (tiny)

2020-11-16 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman closed this revision. aaron.ballman added a comment. In D91506#2397181 , @tschuett wrote: > Yes. Thank you! You're welcome! I've commit on your behalf in a6ac2b32fbab9679c8f2fa97a3b1123e3a9654c8

[PATCH] D91506: [NFC, Refactor] Convert TypeSpecifiersPipe from Specifiers.h to a scoped enum (tiny)

2020-11-16 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D91506#2397158 , @tschuett wrote: > I am happy, if you could commit this for me. I am still learning. I'm happy to do so -- how would you like me to attribute the patch? Is `Thorsten ` what you'd like me to use?

[PATCH] D91506: [NFC, Refactor] Convert TypeSpecifiersPipe from Specifiers.h to a scoped enum (tiny)

2020-11-16 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! Would you like me to commit this one on your behalf or would you like to request your own commit access

[PATCH] D90282: [clang-tidy] Add IgnoreShortNames config to identifier naming checks

2020-11-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D90282#2393019 , @smhc wrote: > In D90282#2391360 , @aaron.ballman > wrote: > >> In D90282#2391005 , @njames93 wrote: >> >>> Should this

[PATCH] D91055: [clang-tidy] Introduce misc No Integer To Pointer Cast check

2020-11-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added subscribers: jeroen.dobbelaere, jdoerfert, hfinkel. aaron.ballman added a comment. In D91055#2382356 , @lebedev.ri wrote: > CC'ing @rsmith regarding the suggestion/question of also having a clang > diagnostic for this. I'm not

[PATCH] D91035: [NFC, Refactor] Convert FunctionDefinitionKind from DeclSpech.h to a scoped enum

2020-11-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/include/clang/Sema/DeclSpec.h:1762 }; +using FDK = FunctionDefinitionKind; rsmith wrote: > I don't think it's OK to have an initialism like this in the `clang` > namespace scope -- generally-speaking,

[PATCH] D91047: Add a call super attribute plugin example

2020-11-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/docs/ClangPlugins.rst:117 +Defining CallSuperAttr +=== + psionic12 wrote: > aaron.ballman wrote: > > aaron.ballman wrote: > > > The number of underlines here looks off -- can you verify it's

[PATCH] D91311: Add new 'preferred_name' attribute.

2020-11-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. The attribute parts LGTM, but I did have a question about the libc++ parts. Comment at: clang/include/clang/Basic/Attr.td:2367 +def PreferredName : InheritableAttr { + let Spellings =

[PATCH] D90984: Update matchers to be traverse-aware

2020-11-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3115 + + if (!Finder->isTraversalAsIs() && (*MatchIt)->isImplicit()) +return false; If the traversal is not `AsIs`, that doesn't mean it's

[PATCH] D91037: [clang-tidy] Fix crash in bugprone-redundant-branch-condition on ExprWithCleanups

2020-11-12 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! Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp:1092 + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning:

[PATCH] D91037: [clang-tidy] Fix crash in bugprone-redundant-branch-condition on ExprWithCleanups

2020-11-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp:1092 + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'isSet' [bugprone-redundant-branch-condition] + // CHECK-FIXES:

[PATCH] D90282: [clang-tidy] Add IgnoreShortNames config to identifier naming checks

2020-11-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D90282#2391005 , @njames93 wrote: > Should this be a NamingStyle option instead. > `{key: readability-identifier-naming.ParameterShortSizeThreshold, value: 2}` > WDYT? I think that makes a lot of sense -- I can imagine

[PATCH] D90042: [clang-tidy] performance-unnecessary-copy-initialization: Check for const reference arguments that are replaced template parameter type.

2020-11-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D90042#2390203 , @flx wrote: > Thanks for the suggestion, I had never hear of creduce! Glad to have introduced you to it -- it's a great tool! > After a bit of trial an error I seem to have found a more minimal example:

[PATCH] D91184: [clang-tidy] Merge options inplace instead of copying

2020-11-12 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/D91184/new/ https://reviews.llvm.org/D91184

[PATCH] D91311: Add new 'preferred_name' attribute.

2020-11-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. I really like this attribute, thank you for working on this! Comment at: clang/include/clang/Basic/Attr.td:2367 +def PreferredName : InheritableAttr { + let Spellings = [Clang<"preferred_name">]; + let Subjects = SubjectList<[ClassTmpl]>;

[PATCH] D91037: [clang-tidy] Fix crash in bugprone-redundant-branch-condition on ExprWithCleanups

2020-11-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp:1092 + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'isSet' [bugprone-redundant-branch-condition] + // CHECK-FIXES:

[PATCH] D91047: Add a call super attribute plugin example

2020-11-11 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. I have more review to do on this but have to run for a while and wanted to get you this feedback sooner rather than later. Comment at: clang/docs/ClangPlugins.rst:119 + +Attribute plugin to mark a virtual method as `call_super`, subclasses must

[PATCH] D91037: [clang-tidy] Fix crash in bugprone-redundant-branch-condition on ExprWithCleanups

2020-11-11 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp:1092 + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'isSet' [bugprone-redundant-branch-condition] + // CHECK-FIXES:

[PATCH] D91037: [clang-tidy] Fix crash in bugprone-redundant-branch-condition on ExprWithCleanups

2020-11-11 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D91037#2387377 , @njames93 wrote: > Taking a step back, rather than worrying about if its an `ExprWithCleanups`. > Shouldn't we just get the condition removing all implicit nodes. > > const Expr* Cond =

[PATCH] D91035: [NFC, Refactor] Convert FunctionDefinitionKind from DeclSpech.h to a scoped enum

2020-11-11 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/include/clang/Sema/DeclSpec.h:1837 /// Actually a FunctionDefinitionKind. - unsigned FunctionDefinition : 2; + FunctionDefinitionKind FunctionDefinition : 2; faisalv wrote: > aaron.ballman wrote: > >

[PATCH] D90275: [clang][IR] Add support for leaf attribute

2020-11-11 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; gulfem wrote: > aaron.ballman wrote: > > gulfem wrote: > >

[PATCH] D90892: [AIX][FE] Support constructor/destructor attribute

2020-11-10 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. I think this generally seems reasonable, but I'm far from an AIX expert so you should wait a few days in case other reviewers have feedback. Comment at:

[PATCH] D91037: [clang-tidy] Fix crash in bugprone-redundant-branch-condition on ExprWithCleanups

2020-11-10 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp:145 + const auto *LeftDRE = dyn_cast(CondOp->getLHS()->IgnoreParenImpCasts()); The old code used to assert that `CondOp` was a

[PATCH] D75229: [clang-tidy] Add signal-in-multithreaded-program check

2020-11-10 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/SignalInMultithreadedProgramCheck.cpp:41 + hasDeclaration(functionDecl(hasAnyListedName(ThreadList, +

[PATCH] D91037: [clang-tidy] Fix crash in bugprone-redundant-branch-condition on ExprWithCleanups

2020-11-10 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp:85 // remove the inner `if`. - const auto *BinOpCond = dyn_cast(InnerIf->getCond()); + const auto *BinOpCond = dyn_cast_or_null(InnerIf->getCond()); +

[PATCH] D91144: Add utility for testing if we're matching nodes AsIs

2020-11-10 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/ASTMatchers/ASTMatchersInternal.cpp:195 +bool ASTMatchFinder::isTraversalAsIs() const { + return getASTContext().getParentMapContext().getTraversalKind() == TK_AsIs; +} steveire wrote: > aaron.ballman

[PATCH] D90892: [AIX][FE] Support constructor/destructor attribute

2020-11-10 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.h:1482 + void AddGlobalDtor(llvm::Function *Dtor, int Priority = 65535, + bool IsDtorAttrFunc = false); Xiangling_L wrote: > aaron.ballman wrote: > >

[PATCH] D91037: [clang-tidy] Fix crash in bugprone-redundant-branch-condition on ExprWithCleanups

2020-11-10 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp:85 // remove the inner `if`. - const auto *BinOpCond = dyn_cast(InnerIf->getCond()); + const auto *BinOpCond = dyn_cast_or_null(InnerIf->getCond()); +

[PATCH] D90892: [AIX][FE] Support constructor/destructor attribute

2020-11-10 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.h:1482 + void AddGlobalDtor(llvm::Function *Dtor, int Priority = 65535, + bool IsDtorAttrFunc = false); Xiangling_L wrote: > aaron.ballman wrote: > > There's

[PATCH] D90851: [clang-tidy] Extending bugprone-signal-handler with POSIX functions.

2020-11-10 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h:25 public: + enum class AsyncSafeFunctionSetType { Minimal, POSIX }; + balazske wrote: > aaron.ballman wrote: > > I don't think this needs to be public?

[PATCH] D77493: [clang-tidy] Add do-not-refer-atomic-twice check

2020-11-10 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D77493#1962465 , @njames93 wrote: > In my mind this check is definitely in the realm of the static analyser, > clang-tidy just isn't designed for this. +1, I think this probably should be handled in the static analyzer

[PATCH] D90275: [clang][IR] Add support for leaf attribute

2020-11-10 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; gulfem wrote: > aaron.ballman wrote: > > gulfem wrote: > >

[PATCH] D90851: [clang-tidy] Extending bugprone-signal-handler with POSIX functions.

2020-11-10 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. One thing that's not clear to me in the patch is what the behavior should be when the user specifies that they want the POSIX set but they're compiling for a non-POSIX system like Windows. Do you have thoughts on that situation? Comment at:

[PATCH] D91037: [clang-tidy] Fix crash in bugprone-redundant-branch-condition on ExprWithCleanups

2020-11-10 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp:85 // remove the inner `if`. - const auto *BinOpCond = dyn_cast(InnerIf->getCond()); + const auto *BinOpCond = dyn_cast_or_null(InnerIf->getCond()); +

[PATCH] D91033: [clang-tidy][NFC] Tweak GlobList to iterate backwards

2020-11-10 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/GlobList.cpp:56 bool GlobList::contains(StringRef S) { - bool Contains = false; - for (const GlobListItem : Items) { + for (const GlobListItem : llvm::reverse(Items)) { if

  1   2   3   4   5   6   7   8   9   10   >