[PATCH] D33623: Make the parser close parens for you on EOF

sbenza added a comment. Do we want this change in the parser itself? or in clang-query? clang-query could be helpful enough to add parens when it detect an `ET_ParserNoCloseParen` error, without changing the language here. https://reviews.llvm.org/D33623 _

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

sbenza added inline comments. Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:31 + isAnyPointer(), + references(type()), + templateTypeParmType(), References are never const qualified, right? Comment at:

[PATCH] D41779: [clang-tidy] Fix DanglingHandleCheck for the correct conversion operation between basic_string and basic_string_view.

sbenza created this revision. sbenza added a reviewer: hokein. Herald added subscribers: cfe-commits, xazax.hun, klimek. Fix DanglingHandleCheck to handle the final implementation of std::string and std::string_view. These use a conversion operator instead of a conversion constructor. Repositor

[PATCH] D41779: [clang-tidy] Fix DanglingHandleCheck for the correct conversion operation between basic_string and basic_string_view.

This revision was automatically updated to reflect the committed changes. Closed by commit rL322002: [clang-tidy] Fix DanglingHandleCheck for the correct conversion operation… (authored by sbenza, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm

[PATCH] D18914: [clang-tidy] new readability-redundant-inline

sbenza added a comment. In D18914#396149 , @mgehre wrote: > ... > I personally think that 1) should be used, because late one could move the > function definition to a source file (removing the inline) without having to > touch > the class declaration.

[PATCH] D54307: [clang] overload ignoringParens for Expr

sbenza added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchers.h:814 /// would match the declaration for fp. -AST_MATCHER_P(QualType, ignoringParens, - internal::Matcher, InnerMatcher) { +AST_MATCHER_P_OVERLOAD(QualType, ignoringParens, internal::Ma

[PATCH] D54407: Record the matcher type when storing a binding

sbenza added a comment. In https://reviews.llvm.org/D54407#1294934, @aaron.ballman wrote: > Adding @sbenza in case he has opinions on this approach. I think it's > reasonable, but I also know that changes to the the AST matcher internals > sometimes have unintended side effects with the various

[PATCH] D54404: Exclude matchers which can have multiple results

sbenza added a comment. In https://reviews.llvm.org/D54404#1296224, @aaron.ballman wrote: > In https://reviews.llvm.org/D54404#1295426, @steveire wrote: > > > I acknowledge and share the future-proofing concern. > > > > We could possibly use something trait-based instead and put the trait > > be

[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.

sbenza added inline comments. Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:45 + /// Evaluates this part to a string and appends it to `result`. + virtual llvm::Error eval(const ast_matchers::MatchFinder::MatchResult &Match, + std::stri

[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.

sbenza added inline comments. Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:45 + /// Evaluates this part to a string and appends it to `result`. + virtual llvm::Error eval(const ast_matchers::MatchFinder::MatchResult &Match, + std::stri

[PATCH] D86751: Add new warning for compound punctuation tokens that are split across macro expansions or split by whitespace.

sbenza accepted this revision. sbenza added inline comments. This revision is now accepted and ready to land. Comment at: clang/test/Parser/compound-token-split.cpp:30 + +[ // expected-warning-re ^}}'[' tokens introducing attribute appear in different source files}} +#define

[PATCH] D110586: Update `DynTypedNode` to support the conversion of `TypeLoc`s.

sbenza added inline comments. Comment at: clang/include/clang/AST/ASTTypeTraits.h:472 + assert(ASTNodeKind::getFromNodeKind().isBaseOf(NodeKind)); + return *static_cast(reinterpret_cast(Storage)); +} The create/get don't seem to match. We are constr

[PATCH] D110586: Update `DynTypedNode` to support the conversion of `TypeLoc`s.

sbenza added inline comments. Comment at: clang/unittests/AST/ASTTypeTraitsTest.cpp:212 + DynTypedNode Node = DynTypedNode::create(tl); + EXPECT_TRUE(Node == Node); + EXPECT_FALSE(Node < Node); I don't know what we are trying to check with this self equivalenc

[PATCH] D110586: Update `DynTypedNode` to support the conversion of `TypeLoc`s.

sbenza accepted this revision. sbenza added inline comments. Comment at: clang/unittests/AST/ASTTypeTraitsTest.cpp:234 + EXPECT_TRUE(PointerTypeLocNode == PointerTypeLocNode); + EXPECT_FALSE(PointerTypeLocNode < PointerTypeLocNode); +} Maybe also check ``` EXP

[PATCH] D29622: Add a batch query and replace tool based on AST matchers.

sbenza added inline comments. Comment at: clang-query/QueryReplace.h:35-36 + + /// \brief Replacement text. %"identifier" will be substituted by the text of + /// an identifier. + std::string ToTemplate; klimek wrote: > This doesn't seem to come up in the test

[PATCH] D29621: Add ASTMatchRefactorer and ReplaceNodeWithTemplate to RefactoringCallbacks

sbenza added inline comments. Comment at: include/clang/Tooling/RefactoringCallbacks.h:61 +MatchFinder.addMatcher(Matcher, Callback); +Callbacks.emplace_back(Callback); + } Why emplace_back instead of push_back? Comment at: lib/Tooling

[PATCH] D41998: [clang-tidy] Expand readability-redundant-smartptr-get to understand implicit converions to bool in more contexts.

sbenza created this revision. sbenza added a reviewer: hokein. Herald added subscribers: cfe-commits, xazax.hun, klimek. Expand readability-redundant-smartptr-get to understand implicit converions to bool in more contexts. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41998 F

[PATCH] D41998: [clang-tidy] Expand readability-redundant-smartptr-get to understand implicit converions to bool in more contexts.

sbenza updated this revision to Diff 129880. sbenza marked an inline comment as done. sbenza added a comment. minor fix Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41998 Files: clang-tidy/readability/RedundantSmartptrGetCheck.cpp test/clang-tidy/readability-redundant-sma

[PATCH] D41998: [clang-tidy] Expand readability-redundant-smartptr-get to understand implicit converions to bool in more contexts.

This revision was automatically updated to reflect the committed changes. Closed by commit rL322497: [clang-tidy] Expand readability-redundant-smartptr-get to understand implicit… (authored by sbenza, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews

[PATCH] D41998: [clang-tidy] Expand readability-redundant-smartptr-get to understand implicit converions to bool in more contexts.

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE322497: [clang-tidy] Expand readability-redundant-smartptr-get to understand implicit… (authored by sbenza, committed by ). Changed prior to commit: https://reviews.llvm.org/D41998?vs=129880&id=12988

[PATCH] D29621: Add ASTMatchRefactorer and ReplaceNodeWithTemplate to RefactoringCallbacks

sbenza added inline comments. Comment at: include/clang/Tooling/RefactoringCallbacks.h:61 +MatchFinder.addMatcher(Matcher, Callback); +Callbacks.emplace_back(Callback); + } jbangert wrote: > sbenza wrote: > > Why emplace_back instead of push_back? > Chan

[PATCH] D29621: Add ASTMatchRefactorer and ReplaceNodeWithTemplate to RefactoringCallbacks

sbenza added inline comments. Comment at: lib/Tooling/RefactoringCallbacks.cpp:213 +llvm::errs() << "Node " << Element.Value + << " used in replacement template not bound in Matcher \n"; +llvm_unreachable("Unbound node in replacement template.

[PATCH] D31160: [clang-tidy] Fix misc-move-const-arg for move-only types.

sbenza added inline comments. Comment at: clang-tidy/misc/MoveConstantArgumentCheck.cpp:76 if (IsConstArg || IsTriviallyCopyable) { +if (const CXXRecordDecl *R = Arg->getType()->getAsCXXRecordDecl()) { + for (const auto *Ctor : R->ctors()) { Can we ge