[PATCH] D159453: [Matrix] Fix test on SystemZ

kuhar abandoned this revision. kuhar added a comment. Gah, disregard Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159453/new/ https://reviews.llvm.org/D159453 ___ cfe-commits mailing list cfe-commits@li

[PATCH] D159453: [Matrix] Fix test on SystemZ

kuhar created this revision. Herald added subscribers: hanchung, tschuett. Herald added a project: All. kuhar requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. As reported by @uweigand in https://reviews.llvm.org/D158883: The newly added t

[PATCH] D146104: Use *{Map,Set}::contains (NFC)

kuhar accepted this revision. kuhar added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146104/new/ https://reviews.llvm.org/D146104 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D145987: [ADT][NFCI] Do not use non-const lvalue-refs with enumerate in llvm/

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb9db89fbcfda: [ADT][NFCI] Do not use non-const lvalue-refs with enumerate in llvm/ (authored by kuhar). Repository: rG LLVM Github Monorepo CHANG

[PATCH] D145987: [ADT][NFCI] Do not use non-const lvalue-refs with enumerate in llvm/

kuhar updated this revision to Diff 504901. kuhar marked an inline comment as done. kuhar added a comment. Update lambda signature Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145987/new/ https://reviews.llvm.org/D145987 Files: clang/tools/clan

[PATCH] D145987: [ADT][NFCI] Do not use non-const lvalue-refs with enumerate in llvm/

kuhar updated this revision to Diff 504837. kuhar added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. Also include clang/ since there's only one function to update Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145

[PATCH] D132001: [clang-format] Fix regressions in WhitespaceSensitiveMacros

kuhar added a comment. This seems to have caused a test failure: https://lab.llvm.org/buildbot/#/builders/109/builds/45138 Failed Tests (1): Clang-Unit :: Format/./FormatTests/TokenAnnotatorTest/UnderstandsLBracesInMacroDefinition Testing Time: 92.30s Skipped :38 Uns

[PATCH] D101471: [clang-tidy] Add proper emplace checks to modernize-use-emplace

kuhar accepted this revision. kuhar added a comment. This revision is now accepted and ready to land. LGTM but please get a second approval before submitting. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101471/new/ https://reviews.llvm.org/D10147

[PATCH] D101471: [clang-tidy] Add proper emplace checks to modernize-use-emplace

kuhar resigned from this revision. kuhar added inline comments. Herald added a subscriber: carlosgalvezp. Herald added a project: All. Comment at: clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp:44 + // FullNameTrimmed matches any of the given Names. + const StringRe

[PATCH] D123278: [Clang] [Docs] Add HLSLSupport page

kuhar accepted this revision. kuhar added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/docs/HLSLSupport.rst:223 +* References +* goto or labels +* Variable Length Arrays nit, for consistency with the other list items: make

[PATCH] D123278: [Clang] [Docs] Add HLSLSupport page

kuhar added inline comments. Comment at: clang/docs/HLSLSupport.rst:190 +* RTTI +* Exceptions +* Multiple inheritance How about goto and labels? Irreducible control flow? Are infinite loops valid? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D123278: [Clang] [Docs] Add HLSLSupport page

kuhar added a comment. Looks good to me overall, I just left some local comments. Please take my writing suggestions with a pinch of salt, English is my second language. Comment at: clang/docs/HLSLSupport.rst:13 +describes the high level goals of the project, the guiding princ

[PATCH] D119061: [Clang] noinline call site attribute

kuhar accepted this revision. kuhar added a comment. This revision is now accepted and ready to land. LGTM put please get at least one additional approval before submitting CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119061/new/ https://reviews.llvm.org/D119061 __

[PATCH] D119061: [Clang] noinline call site attribute

kuhar added inline comments. Comment at: clang/test/CodeGen/attr-noinline.cpp:11 + +void foo(int i) { + [[clang::noinline]] bar(); Could you also add a test function that is already marked as noinline at the declaration? CHANGES SINCE LAST ACTION https://re

[PATCH] D119061: [Clang] noinline call site attribute

kuhar added a comment. In D119061#3312571 , @xbolva00 wrote: >>> Do you plan to also add inline and flatten? > > You mean always_inline? Yes, after noinline. The flatten call site attribute > - theoretically why not, but it needs to be reworked in LLVM (

[PATCH] D119061: [Clang] noinline call site attribute

kuhar added a comment. Thanks for working on this, @xbolva00! I don't know this part of the codebase, so won't comment on the patch itself. Just a few questions/suggestions: 1. Do you plan to also add `inline` and `flatten`? 2. When I worked on my implementation, I remember that I also ran into

[PATCH] D118386: [Support][NFC] Fix generic `ChildrenGetterTy` of `IDFCalculatorBase`

kuhar accepted this revision. kuhar 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/D118386/new/ https://reviews.llvm.org/D118386 ___ c

[PATCH] D112926: run-clang-tidy.py analyze unique files only

kuhar accepted this revision. kuhar added a comment. This revision is now accepted and ready to land. I think this change is fine. One could argue that having multiple files is an issue with tooling/build system in the first place, and we could consider printing a warning instead of silently fi

[PATCH] D101471: [clang-tidy] Add proper emplace checks to modernize-use-emplace

kuhar added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp:28 + std::string FullNameTrimmed; + int Count = 0; + for (const auto &Character : FullName) { Can you add a comment explaining what this loops tries to do? Idea

[PATCH] D91327: [NewPM] Redesign of PreserveCFG Checker

kuhar added a comment. Just two nits from me. I think it looks fine, but I'm not familiar with the new pass manager and don't feel confident enough to approve it. Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:134 public: static cl::opt VerifyPreservedCF

[PATCH] D94827: [SimplifyCFG] If provided, preserve Dominator Tree

kuhar accepted this revision. kuhar added a comment. Choo choo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94827/new/ https://reviews.llvm.org/D94827 ___ cfe-commits mailing list cfe-commits@lists.llvm

[PATCH] D94827: [SimplifyCFG] Require and preserve dominator tree

kuhar added a comment. Wow, this is fantastic. When I first started working on the domtree updater back in 2017, SimplifyGFG seemed like one of the most difficult passes to handle, and I wasn't sure if we ever get there. Very impressive work, @lebedev.ri! Repository: rG LLVM Github Monorepo

[PATCH] D91327: [NewPM] Redesign of PreserveCFG Checker

kuhar added inline comments. Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:134 public: static cl::opt VerifyPreservedCFG; yrouban wrote: > kuhar wrote: > > not necessary anymore > there can bee a need to disabled/enable (e.g. for some test

[PATCH] D91327: [NewPM] Redesign of PreserveCFG Checker

kuhar added a comment. Looks fine to me, but I'm not confident enough to give an approval. Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:733-735 + static AnalysisKey Key; + +public: This is already public CHANGES SINCE LAST ACTION https://review

[PATCH] D91327: [NewPM] Redesign of PreserveCFG Checker

kuhar added a comment. Found some cosmetics, but I'm not familiar enough with the NPM to do a full review. Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:99 struct CFG { struct BBGuard final : public CallbackVH { BBGuard(const BasicBlock *BB) :

[PATCH] D83088: Introduce CfgTraits abstraction

kuhar added a comment. Hi Nicoali, In D83088#2227151 , @nhaehnle wrote: > ... > Take a look here for example: > https://github.com/nhaehnle/llvm-project/blob/715450fa7f968ceefaf9c3b04b47066866c97206/llvm/lib/Analysis/GenericConvergenceUtils.cpp#L499 > -

[PATCH] D84713: [DominatorTree] Simplify ChildrenGetter.

kuhar accepted this revision. kuhar added a comment. This revision is now accepted and ready to land. LGTM. One tiny nit: the function name `ChildrenGet` sounds kind of backwards to me, but it seems like the other direction is already taken. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D83087: DomTree: remove explicit use of DomTreeNodeBase::iterator

kuhar added a comment. In D83087#2139211 , @nhaehnle wrote: > In D83087#2134881 , @kuhar wrote: > > > modulo accidental formatting changes. > > > I'm not aware of any. Some line breaks changed because "const_iterato

[PATCH] D83087: DomTree: remove explicit use of DomTreeNodeBase::iterator

kuhar accepted this revision. kuhar added a comment. LGTM modulo accidental formatting changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83087/new/ https://reviews.llvm.org/D83087 ___ cfe-commits

[PATCH] D77341: [DomTree] Replace ChildrenGetter with GraphTraits over GraphDiff.

kuhar accepted this revision. kuhar added a comment. This revision is now accepted and ready to land. Looks good to me overall. I don't want to block it over the cosmetic issues like allocating the empty GD object. Comment at: llvm/include/llvm/Support/GenericDomTreeConstructi

[PATCH] D77341: [DomTree] Replace ChildrenGetter with GraphTraits over GraphDiff.

kuhar added inline comments. Comment at: llvm/include/llvm/IR/CFGDiff.h:198 +namespace { +template kuhar wrote: > What benefit does an anonymous namespace in a header have over a named one, > e.g., `detail`/`impl`? Doesn't it make it more difficult to dedupli

[PATCH] D77341: [DomTree] Replace ChildrenGetter with GraphTraits over GraphDiff.

kuhar added a comment. In D77341#1960854 , @asbirlea wrote: > Address comments. Thanks for the changes and explanations. It think with a few more tweaks this will be a good refactoring step towards phasing out BUI. Comment at: llvm/i

[PATCH] D77341: [DomTree] Replace ChildrenGetter with GraphTraits over GraphDiff.

kuhar added inline comments. Comment at: llvm/include/llvm/Support/GenericDomTreeConstruction.h:186 + for (auto &Pair : children(GDNodePair)) { +const NodePtr Succ = Pair.second; const auto SIT = NodeToInfo.find(Succ); kuhar wrote: > Could t

[PATCH] D77341: [DomTree] Replace ChildrenGetter with GraphTraits over GraphDiff.

kuhar added inline comments. Comment at: llvm/include/llvm/IR/CFGDiff.h:199 +namespace { +template struct reverse_if_helper; +template <> struct reverse_if_helper { You can use two helper functions taking `integral_constant` tags -- should be less verbose ===

[PATCH] D62619: [analyzer][IDF] Add a control dependency calculator + a new debug checker

kuhar added a comment. The non-static-analyzer bits look good to me, I added a few nits. Comment at: clang/include/clang/Analysis/Analyses/Dominators.h:230 + + virtual void releaseMemory() { +PostDomTree.releaseMemory(); If the virtual function is introduc

[PATCH] D62619: [analyzer][IDF] Add a control dependency calculator + a new debug checker

kuhar added inline comments. Comment at: clang/include/clang/Analysis/Analyses/Dominators.h:51 + CFGDominatorTreeImpl(CFG *cfg) { +buildDominatorTree(cfg); + } DomTree has a constructor that runs the builder -- why not use it directly? Co

[PATCH] D62619: [analyzer][Dominators] Add a control dependency tree builder + a new debug checker

kuhar added a comment. You can easily get CD by calculating the PostDominanceFrontier. LLVM implements a templated IDF (Iterated Dominance Frontier) construction. A native implementation for llvm ir for reference, if you need: - https://github.com/seahorn/seahorn/blob/deep-dev-5.0/include/seaho

[PATCH] D62611: [analyzer][Dominators] Add unittests

kuhar added inline comments. Comment at: clang/unittests/Analysis/CFGDominatorTree.cpp:95 + CFGPostDomTree PostDom; + PostDom.buildDominatorTree(cfg); + Why not have a constructor that takes the cfg and constructs a domtree straight away? But this should proba

[PATCH] D62611: [analyzer][Dominators] Add unittests

kuhar added inline comments. Comment at: clang/unittests/Analysis/CFGDominatorTree.cpp:37 +TEST(CFGDominatorTree, DomTree) { + const char *Code = "enum Kind {\n" + " A\n" You can use a raw string literal to make it more readable: https://en

[PATCH] D62551: [analyzer][Dominator] Add post dominator tree builder for the CFG + a debug checker

kuhar added inline comments. Comment at: clang/include/clang/Analysis/Analyses/Dominators.h:111 + assert( + (IDom && !IDom->getBlock() && *I == &(*I)->getParent()->getExit() && + IsPostDom) || kuhar wrote: > Szelethus wrote: > > kuhar wrot

[PATCH] D62551: [analyzer][Dominator] Add post dominator tree builder for the CFG + a debug checker

kuhar added inline comments. Comment at: clang/include/clang/Analysis/Analyses/Dominators.h:111 + assert( + (IDom && !IDom->getBlock() && *I == &(*I)->getParent()->getExit() && + IsPostDom) || Szelethus wrote: > kuhar wrote: > > Assertions

[PATCH] D62551: [analyzer][Dominator] Add post dominator tree builder for the CFG + a debug checker

kuhar added inline comments. Comment at: clang/include/clang/Analysis/Analyses/Dominators.h:39 /// Concrete subclass of DominatorTreeBase for Clang /// This class implements the dominators tree functionality given a Clang CFG. Seems outdated? ==

[PATCH] D62507: [Dominators] PR42041: Skip nullpointer successors

kuhar accepted this revision. kuhar added a comment. This revision is now accepted and ready to land. Thanks, I think this is a much better solution than directly hacking on DomTreeBuilder. Note that every time you ask DomTree about a `nullptr` CFG node, it understands it as a request for a vir

[PATCH] D62507: [Dominators] PR42041: Skip nullpointer successors

kuhar added inline comments. Comment at: clang/include/clang/Analysis/Analyses/Dominators.h:190 +clang::CFGBlock *N, std::integral_constant) { + auto RChildren = reverse(children(N)); + ResultTy Ret(RChildren.begin(), RChildren.end()); Shouldn't one of the

[PATCH] D51200: Introduce per-callsite inline intrinsics

kuhar added a comment. Thank you for the comments, Richard! In https://reviews.llvm.org/D51200#1223745, @rsmith wrote: > Have you considered whether the builtin should apply to `new` expressions? > (There are potentially three different top-level calls implied by a `new` > expression -- an `op

[PATCH] D51200: Introduce per-callsite inline intrinsics

kuhar added a comment. In https://reviews.llvm.org/D51200#1223752, @rsmith wrote: > +rjmccall for CodeGen changes @rsmith @rjmccall I have one high-level question about the CodeGen part that I wasn't able to figure out: is it possible to bail out of custom CodeGen in CGBuiltin and somehow sa

[PATCH] D51200: Introduce per-callsite inline intrinsics

kuhar updated this revision to Diff 162520. https://reviews.llvm.org/D51200 Files: include/clang/Basic/Builtins.def include/clang/Basic/DiagnosticSemaKinds.td include/clang/CodeGen/CGFunctionInfo.h lib/CodeGen/CGBuiltin.cpp lib/CodeGen/CGCall.cpp lib/CodeGen/CGExpr.cpp lib/CodeGen/C

[PATCH] D51200: Introduce per-callsite inline intrinsics

kuhar updated this revision to Diff 162519. kuhar added a comment. Rename missed CallInlineKind parameters to CIK https://reviews.llvm.org/D51200 Files: include/clang/Basic/Builtins.def include/clang/Basic/DiagnosticSemaKinds.td include/clang/CodeGen/CGFunctionInfo.h lib/CodeGen/CGBuilt

[PATCH] D51200: Introduce per-callsite inline intrinsics

kuhar added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8236 +def err_argument_to_inline_intrinsic_builtin_call : Error< + "argument to %0 must not be a builtin call">; +def err_argument_to_inline_intrinsic_pdotr_call : Error< Quuxplu

[PATCH] D51200: Introduce per-callsite inline intrinsics

kuhar updated this revision to Diff 162495. kuhar marked an inline comment as done. kuhar added a comment. Fix naming and add more tests. Repository: rC Clang https://reviews.llvm.org/D51200 Files: include/clang/Basic/Builtins.def include/clang/Basic/DiagnosticSemaKinds.td include/clan

[PATCH] D51200: Introduce per-callsite inline intrinsics

kuhar added a comment. One extra question: do you think this deserves a separate RFC? Repository: rC Clang https://reviews.llvm.org/D51200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-

[PATCH] D51200: Introduce per-callsite inline intrinsics

kuhar added a comment. Disclaimer: I'm a Clang newbie and I'm not sure if that's a good way to implement these intrinsics. I'm not sure about the following things: - The new enum CallInlineKind may not be in the right place - Not sure if adding the extra parameter to EmitSomething methods is the

[PATCH] D51200: Introduce per-callsite inline intrinsics

kuhar created this revision. kuhar added reviewers: rsmith, pcc, Prazek, sanjoy. kuhar added a project: clang. Herald added a subscriber: eraman. Herald added a reviewer: grosser. Traditionally, to force some inlining decisions one has to annotate function declarations with `__attribute__((always

[PATCH] D32690: [clang-tidy] modernize-use-emplace: Remove unnecessary make_tuple calls

kuhar updated this revision to Diff 99103. kuhar added a comment. The patch broke the spinx docs build and I had to revert it in r303140. I fixed the docs file problems. Repository: rL LLVM https://reviews.llvm.org/D32690 Files: clang-tidy/modernize/UseEmplaceCheck.cpp clang-tidy/moderni

[PATCH] D32690: [clang-tidy] modernize-use-emplace: Remove unnecessary make_tuple calls

This revision was automatically updated to reflect the committed changes. Closed by commit rL303139: [clang-tidy] modernize-use-emplace: Remove unnecessary make_tuple calls (authored by kuhar). Changed prior to commit: https://reviews.llvm.org/D32690?vs=98073&id=99100#toc Repository: rL LLVM

[PATCH] D32690: [clang-tidy] modernize-use-emplace: Remove unnecessary make_tuple calls

kuhar updated this revision to Diff 98073. kuhar added a comment. Herald added a subscriber: xazax.hun. I updated the patch against the current trunk. I also run the modernize-use-emplace check on llvm and verified that there are no new regressions after applying fixits. https://reviews.llvm.o

[PATCH] D32923: [clang-tidy] Use cxxStdInitializerListExpr in modernize-use-emplace

This revision was automatically updated to reflect the committed changes. Closed by commit rL302317: [clang-tidy] Use cxxStdInitializerListExpr in modernize-use-emplace (authored by kuhar). Changed prior to commit: https://reviews.llvm.org/D32923?vs=98024&id=98034#toc Repository: rL LLVM ht

[PATCH] D32923: [clang-tidy] Use cxxStdInitializerListExpr in modernize-use-emplace

kuhar created this revision. kuhar added a project: clang-tools-extra. Herald added a subscriber: xazax.hun. Use the cxxStdInitializerListExp matcher from ASTMatchers.h instead of a local one. Repository: rL LLVM https://reviews.llvm.org/D32923 Files: clang-tidy/modernize/UseEmplaceCheck.

[PATCH] D32810: Add cxxStdInitializerListExpr AST matcher

This revision was automatically updated to reflect the committed changes. Closed by commit rL302287: Add cxxStdInitializerListExpr AST matcher (authored by kuhar). Changed prior to commit: https://reviews.llvm.org/D32810?vs=97700&id=98018#toc Repository: rL LLVM https://reviews.llvm.org/D32

[PATCH] D32767: [clang-tidy] Fix PR32896: detect initializer lists in modernize-use-empalce

This revision was automatically updated to reflect the committed changes. Closed by commit rL302281: [clang-tidy] Fix PR32896: detect initializer lists in modernize-use-empalce (authored by kuhar). Changed prior to commit: https://reviews.llvm.org/D32767?vs=97704&id=98013#toc Repository: rL

[PATCH] D32767: [clang-tidy] Fix PR32896: detect initializer lists in modernize-use-empalce

kuhar updated this revision to Diff 97704. kuhar added a comment. I switched from a narrowing matcher to a "normal" node matcher. Repository: rL LLVM https://reviews.llvm.org/D32767 Files: clang-tidy/modernize/UseEmplaceCheck.cpp test/clang-tidy/modernize-use-emplace.cpp Index: test/cl

[PATCH] D32810: Add cxxStdInitializerListExpr AST matcher

kuhar updated this revision to Diff 97700. kuhar added a comment. I regenerated docs and added unit tests. Thanks for help. Repository: rL LLVM https://reviews.llvm.org/D32810 Files: docs/LibASTMatchersReference.html include/clang/ASTMatchers/ASTMatchers.h lib/ASTMatchers/Dynamic/Regi

[PATCH] D32767: [clang-tidy] Fix PR32896: detect initializer lists in modernize-use-empalce

kuhar added inline comments. Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:23 +AST_MATCHER(CXXStdInitializerListExpr, cxxStdInitializerListExpr) { + return true; Prazek wrote: > kuhar wrote: > > alexfh wrote: > > > This should be a node matcher rather t

[PATCH] D32810: Add cxxStdInitializerListExpr AST matcher

kuhar created this revision. Herald added a subscriber: klimek. This adds a new ASTMatcher for CXXStdInitializerListExprs that matches C++ initializer list expressions. The primary motivation is to use it to fix PR32896 (review here D32767

[PATCH] D32767: [clang-tidy] Fix PR32896: detect initializer lists in modernize-use-empalce

kuhar added inline comments. Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:23 +AST_MATCHER(CXXStdInitializerListExpr, cxxStdInitializerListExpr) { + return true; alexfh wrote: > This should be a node matcher rather than a narrowing matcher, and it shoul

[PATCH] D32767: [clang-tidy] Fix PR32896: detect initializer lists in modernize-use-empalce

kuhar created this revision. kuhar added a project: clang-tools-extra. Herald added a subscriber: xazax.hun. This patch fixes PR32896 . The problem was that modernize-use-emplace incorrectly removed changed push_back into emplace_back, removing explic

[PATCH] D32690: [clang-tidy] modernize-use-emplace: Remove unnecessary make_tuple calls

kuhar updated this revision to Diff 97299. kuhar edited the summary of this revision. kuhar added a comment. I went ahead and generalized the patch to support custom tuple-like types and custom make_ functions. I added a bunch of tests and updated the docs. Let me know what you think. https://

[PATCH] D32690: [clang-tidy] modernize-use-emplace: Remove unnecessary make_tuple calls

kuhar added a comment. In https://reviews.llvm.org/D32690#741952, @JonasToth wrote: > Would it make sense to add boost pairs/tuples support? I think it can be added the same way it is possible to specify smart pointers and vector-like containers now. It would require users to provide both type

[PATCH] D32690: [clang-tidy] modernize-use-emplace: Remove unnecessary make_tuple calls

kuhar updated this revision to Diff 97248. kuhar added a comment. Cosmetic changes. https://reviews.llvm.org/D32690 Files: clang-tidy/modernize/UseEmplaceCheck.cpp docs/ReleaseNotes.rst test/clang-tidy/modernize-use-emplace.cpp Index: test/clang-tidy/modernize-use-emplace.cpp ===

[PATCH] D32690: [clang-tidy] modernize-use-emplace: Remove unnecessary make_tuple calls

kuhar created this revision. kuhar added a project: clang-tools-extra. This patch makes modernize-use-emplace remove unnecessary make_tuple calls from push_back calls and turn them into emplace_back -- the same way make_pair calls are handled. Eq. std::vector> v; v.push_back(std::make_tupl

[PATCH] D32678: [clang-tidy] Fix naming convention in modernize-use-emplace

This revision was automatically updated to reflect the committed changes. Closed by commit rL301780: [clang-tidy] Fix naming convention in modernize-use-emplace (authored by kuhar). Changed prior to commit: https://reviews.llvm.org/D32678?vs=97221&id=97243#toc Repository: rL LLVM https://re

[PATCH] D32678: [clang-tidy] Fix naming convention in modernize-use-empace

kuhar created this revision. kuhar added a project: clang-tools-extra. Conform to llvm naming convention for local variables in modernize-use-emplace check. https://reviews.llvm.org/D32678 Files: clang-tidy/modernize/UseEmplaceCheck.cpp Index: clang-tidy/modernize/UseEmplaceCheck.cpp =

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

This revision was automatically updated to reflect the committed changes. Closed by commit rL301651: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls (authored by kuhar). Changed prior to commit: https://reviews.llvm.org/D32395?vs=96634&id=97113#toc Repository: rL LLVM

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

kuhar added a comment. I run clang-tidy with this version of modernize-use-emplace on the whole llvm + clang + clang tools extra tree and it emitted ~500 fixits, ~100 of them were make_pair ones. The whole codebase compiled fine and there were no new test failures with check-all. https://revi

[PATCH] D32294: [clang-tidy] run-clang-tidy.py: check if clang-apply-replacements succeeds

This revision was automatically updated to reflect the committed changes. Closed by commit rL301365: [clang-tidy] run-clang-tidy.py: check if clang-apply-replacements succeeds (authored by kuhar). Changed prior to commit: https://reviews.llvm.org/D32294?vs=96097&id=96642#toc Repository: rL L

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

kuhar marked 2 inline comments as done. kuhar added inline comments. Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:115 + const auto *MakePairCall = Result.Nodes.getNodeAs("make_pair"); + assert(InnerCtorCall || MakePairCall); Prazek wrote: > JDevlieghe

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

kuhar updated this revision to Diff 96634. kuhar added a comment. Use igoringImplicit instead of ignoringParenImpCasts. Options moved to the anonymous namespace. https://reviews.llvm.org/D32395 Files: clang-tidy/modernize/UseEmplaceCheck.cpp docs/ReleaseNotes.rst docs/clang-tidy/checks/m

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

kuhar updated this revision to Diff 96622. kuhar marked 4 inline comments as done. kuhar added a comment. Added const where possible, moved from if-else to ternary. https://reviews.llvm.org/D32395 Files: clang-tidy/modernize/UseEmplaceCheck.cpp docs/ReleaseNotes.rst docs/clang-tidy/checks

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

kuhar added a comment. Thanks for the suggestions, Jonas. Fixed. https://reviews.llvm.org/D32395 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32294: [clang-tidy] run-clang-tidy.py: check if clang-apply-replacements succeeds

kuhar requested review of this revision. kuhar added a comment. Is the current patch OK? Do you have any other comments? https://reviews.llvm.org/D32294 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/li

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

kuhar updated this revision to Diff 96384. kuhar added a comment. Don't remove make_pair calls when inserted type is different than std::pair. https://reviews.llvm.org/D32395 Files: clang-tidy/modernize/UseEmplaceCheck.cpp docs/ReleaseNotes.rst docs/clang-tidy/checks/modernize-use-emplace

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

kuhar added inline comments. Comment at: test/clang-tidy/modernize-use-emplace.cpp:284 + // CHECK-FIXES: v.emplace_back(42LL, 13); + + v.push_back(std::make_pair(0, 3)); kuhar wrote: > Prazek wrote: > > I would add here test like: > > > > class X { > > X

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

kuhar updated this revision to Diff 96332. kuhar marked an inline comment as done. kuhar added a comment. Update modernize-use-emplace rst docs. https://reviews.llvm.org/D32395 Files: clang-tidy/modernize/UseEmplaceCheck.cpp docs/ReleaseNotes.rst docs/clang-tidy/checks/modernize-use-empla

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

kuhar marked 2 inline comments as done. kuhar added inline comments. Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:93 + to(functionDecl(hasName("::std::make_pair" + + .bind("make_pair")); Prazek wrote: > JonasTot

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

kuhar updated this revision to Diff 96329. kuhar added a comment. Add test for pair constructor argument, mention changes in release notes. https://reviews.llvm.org/D32395 Files: clang-tidy/modernize/UseEmplaceCheck.cpp docs/ReleaseNotes.rst test/clang-tidy/modernize-use-emplace.cpp Inde

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

kuhar created this revision. kuhar added a project: clang-tools-extra. When there is a push_back with a call to make_pair, turn it into emplace_back and remove the unnecessary make_pair call. Eg. std::vector> v; v.push_back(std::make_pair(1, 2)); // --> v.emplace_back(1, 2); make_pair does

[PATCH] D32294: [clang-tidy] run-clang-tidy.py: check if clang-apply-replacements succeeds

kuhar updated this revision to Diff 96097. kuhar added a comment. Minor change to success detection logic. https://reviews.llvm.org/D32294 Files: clang-tidy/tool/run-clang-tidy.py Index: clang-tidy/tool/run-clang-tidy.py === ---

[PATCH] D32294: [clang-tidy] run-clang-tidy.py: check if clang-apply-replacements succeeds

kuhar updated this revision to Diff 96045. kuhar added a comment. After thinking about Piotr's comment, I think that a better way to perform the check would be te invoking clang-apply-replacements with `--version` and seeing if it fails even before running clang-tidy. This way it is possible to

[PATCH] D32313: [clang-tidy] Don't turn .push_back({1, 2}) into .emplace_back(1, 2) for pairs

kuhar created this revision. kuhar added a project: clang-tools-extra. This patch prevents modernize-use-emplace from changing push_backs of brace initialized pairs (`.push_back({1, 2})`) to `.emplace_back(1, 2)`. Pair's constructor doesn't have any interesting logic and basically performs aggre

[PATCH] D32294: [clang-tidy] run-clang-tidy.py: check if clang-apply-replacements succeeds

kuhar marked 2 inline comments as done. kuhar added inline comments. Comment at: clang-tidy/tool/run-clang-tidy.py:100 + except: +print >>sys.stderr, "Unable to run clang-apply-replacements." +sys.exit(1) kimgr wrote: > alexfh wrote: > > "Unable to run c

[PATCH] D32294: [clang-tidy] run-clang-tidy.py: check if clang-apply-replacements succeeds

kuhar updated this revision to Diff 96001. kuhar added a comment. Use python3 printing. Move exception handling out of apply_fixes. Now, the code prints the following on failure: Applying fixes ... Error applying fixes. Is clang-apply-replacements binary correctly specified? Traceback (

[PATCH] D32294: [clang-tidy] run-clang-tidy.py: check if clang-apply-replacements succeeds

kuhar created this revision. kuhar added a project: clang-tools-extra. When running run-clang-tidy.py with -fix it tries to apply found replacements at the end. If there are errors running clang-apply-replacements, the script currently crashes or displays no error at all. This patch checks for