[PATCH] D65127: Even more warnings utilizing gsl::Owner/gsl::Pointer annotations

gribozavr added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:6581 +if (!Callee->getIdentifier()) { + auto OO = Callee->getOverloadedOperator(); + return OO == OverloadedOperatorKind::OO_Subscript || xazax.hun wrote: > gribozavr wrote: >

[PATCH] D65065: [clang-tidy] Possibility of displaying duplicate warnings

gribozavr added a comment. So the problem you're trying to solve is that the output is non-deterministic, is that correct? > However, it is random which checker name is included in the warning. If that is indeed the problem, then I think the solution should be making the output deterministic -

[PATCH] D65065: [clang-tidy] Possibility of displaying duplicate warnings

gribozavr added a comment. Producing the message two times is worse user experience than producing one. Most users don't care which checker produced the message. However, the output should be deterministic. Therefore, a better fix would be making deduplication deterministic, instead of printing

[PATCH] D65065: [clang-tidy] Possibility of displaying duplicate warnings

gribozavr added a comment. > This suggestion would result another strange behavior: if the user disables > cert-err09-cpp because he or she doesn't want to see its reports, the other > one (cert-err61-cpp) will still report the issue. So he or she has to disable > both (or as many aliases it ha

[PATCH] D65065: [clang-tidy] Possibility of displaying duplicate warnings

gribozavr added a comment. `-Weverything` is not recommended for anything except compiler testing, and similarly with clang-tidy, enabling all checkers is not a good idea. I don't think improving this particular point will make enabling all checks more usable. Repository: rCTE Clang Tools Ex

[PATCH] D65704: DirectoryWatcher::create: Adding better error handling.

gribozavr added inline comments. Comment at: cfe/trunk/include/clang/DirectoryWatcher/DirectoryWatcher.h:102 - /// Returns nullptr if \param Path doesn't exist or isn't a directory. - /// Returns nullptr if OS kernel API told us we can't start watching. In such - /// case it

[PATCH] D64256: Teach some warnings to respect gsl::Pointer and gsl::Owner attributes

gribozavr added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:7077 +// someContainer.add(std::move(localOWner)); +// return p; +if (!IsTempGslOwner && pathOnlyInitializesGslPointer(Path) && xazax.hun wrote: > gribozavr wrote: > > Why is

[PATCH] D65704: DirectoryWatcher::create: Adding better error handling.

gribozavr added inline comments. Comment at: cfe/trunk/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:287 /*waitForInitialSync=*/true); + if (!DW) return; plotfi wrote: > gribozavr wrote: > > plotfi wrote: > > > gribozavr wrote: > > > > Why? This i

[PATCH] D65829: [clang][DirectoryWatcher][NFC] Swapping asserts for llvm fatal_error in ::create.

gribozavr added inline comments. Comment at: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:323 + /*waitForInitialSync=*/false); + // llvm::Expected will throw an error if DW is an Error. + if (!DW) Call `llvm::cantFail` and there will be no

[PATCH] D65829: [clang][DirectoryWatcher][NFC] Swapping asserts for llvm fatal_error in ::create.

gribozavr added inline comments. Comment at: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:323 + /*waitForInitialSync=*/false); + // llvm::Expected will throw an error if DW is an Error. + if (!DW) plotfi wrote: > gribozavr wrote: > > Call

[PATCH] D65829: [clang][DirectoryWatcher][NFC] Swapping asserts for llvm fatal_error in ::create.

gribozavr added inline comments. Comment at: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:323 + /*waitForInitialSync=*/false); + // llvm::Expected will throw an error if DW is an Error. + if (!DW) plotfi wrote: > gribozavr wrote: > > plotf

[PATCH] D65829: [clang][DirectoryWatcher][NFC] Swapping asserts for llvm fatal_error in ::create.

gribozavr added inline comments. Comment at: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:323 + /*waitForInitialSync=*/false); + // llvm::Expected will throw an error if DW is an Error. + if (!DW) plotfi wrote: > gribozavr wrote: > > plotf

[PATCH] D65853: Use ASSERT_THAT_ERROR instead of logAllUnhandledErrors/exit

gribozavr created this revision. gribozavr added reviewers: plotfi, jkorous, compnerd. Herald added subscribers: cfe-commits, dexonsmith, mgorny. Herald added a project: clang. ASSERT_THAT_ERROR looks like the intended helper for use in tests. Repository: rG LLVM Github Monorepo https://revie

[PATCH] D65940: [clang-format] fix crash involving invalid preprocessor line

gribozavr accepted this revision. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: unittests/Format/FormatTest.cpp:3090 Style); + // Don't crash if there is nothing following #elif. + verifyFormat("#if 1\n" --

[PATCH] D65944: [clang] Update `ignoringElidableConstructorCall` matcher to ignore `ExprWithCleanups`.

gribozavr accepted this revision. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:6538 /// -/// In C++17 copy elidable constructors are no longer being -/// generated in the AST as it is

[PATCH] D65944: [clang] Update `ignoringElidableConstructorCall` matcher to ignore `ExprWithCleanups`.

gribozavr accepted this revision. gribozavr added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:6556 /// -/// ``varDecl(hasInitializer(any( -/// ignoringElidableConstructorCall(callExpr()), -/// exprWithCleanups(ignoringElidableConstructo

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 🔍

gribozavr accepted this revision. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:820 + llvm::DenseMap> + CompatibleAliases; `unordered_set`? Repository: rG LLVM Gith

[PATCH] D65853: Use ASSERT_THAT_ERROR instead of logAllUnhandledErrors/exit

gribozavr added a comment. In D65853#1619801 , @plotfi wrote: > I tested this out. Seems to work fine, and print the Expected's Error. I am > fine with it if @compnerd and @lhames and @jkorous are cool with it. @compnerd @lhames @jkorous ping? Reposit

[PATCH] D65963: [clang][NFC] Move matcher ignoringElidableConstructorCall's tests to appropriate file.

gribozavr accepted this revision. gribozavr added a comment. This revision is now accepted and ready to land. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65963/new/ https://reviews.llvm.org/D65963 __

[PATCH] D65853: Use ASSERT_THAT_ERROR instead of logAllUnhandledErrors/exit

This revision was automatically updated to reflect the committed changes. Closed by commit rL368399: Use ASSERT_THAT_ERROR instead of logAllUnhandledErrors/exit (authored by gribozavr, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D65877: [libTooling] In Transformer, generalize `applyFirst` to admit rules with incompatible matchers.

gribozavr added inline comments. Comment at: clang/lib/Tooling/Refactoring/Transformer.cpp:85 // the implicit relationship of Type and QualType. static bool isBaseOf(ASTNodeKind A, ASTNodeKind B) { static auto TypeKind = ASTNodeKind::getFromNodeKind(); I'd f

[PATCH] D65127: Even more warnings utilizing gsl::Owner/gsl::Pointer annotations

gribozavr accepted this revision. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Sema/SemaInit.cpp:6581 +if (!Callee->getIdentifier()) { + auto OO = Callee->getOverloadedOperator(); + return OO == OverloadedOp

[PATCH] D65877: [libTooling] In Transformer, generalize `applyFirst` to admit rules with incompatible matchers.

gribozavr accepted this revision. gribozavr added a comment. This revision is now accepted and ready to land. Nice simplification, thanks! Comment at: clang/lib/Tooling/Refactoring/Transformer.cpp:127 + // in `taggedMatchers`. + std::map, 1>> + Buckets;

[PATCH] D65877: [libTooling] In Transformer, generalize `applyFirst` to admit rules with incompatible matchers.

gribozavr added a comment. In D65877#1625493 , @ymandel wrote: > I was going to add a test for `Type`/`QualType` and realized that they don't > carry any source location info. Therefore, I don't think they belong as > top-level matchers for rewrite rule

[PATCH] D65877: [libTooling] In Transformer, generalize `applyFirst` to admit rules with incompatible matchers.

gribozavr accepted this revision. gribozavr added inline comments. Comment at: clang/lib/Tooling/Refactoring/Transformer.cpp:127 +if (!hasValidKind(Cases[I].Matcher)) + return std::vector(); +Buckets[Cases[I].Matcher.getSupportedKind()].emplace_back(I, Cases[I]); ---

[PATCH] D66136: Remove no-op callbacks from TemplateInstantiationCallback

gribozavr created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. It is not clear to me that these callbacks ever did anything, they were empty even in the original implementation (r324808). Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D66

[PATCH] D66156: Removed dead code from clang/tools/libclang/CXIndexDataConsumer.{cpp,h}

gribozavr created this revision. gribozavr added a reviewer: jkorous. Herald added subscribers: cfe-commits, arphaman, dexonsmith. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D66156 Files: clang/tools/libclang/CXIndexDataConsumer.cpp clang/to

[PATCH] D61027: Fix crash on switch conditions of non-integer types in templates

gribozavr added a comment. Sorry, but this change broke ClangTidy tests: http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/16398. I reverted it. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61027/new/ https://reviews.llvm.org/D61027 _

[PATCH] D61027: Fix crash on switch conditions of non-integer types in templates

gribozavr added a comment. Like @riccibruno said, `check-clang-tools` will run them. However, before committing a patch, please run `check-all` -- you never know what your patch can affect. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61027/new/ https://reviews

[PATCH] D66156: Removed dead code from clang/tools/libclang/CXIndexDataConsumer.{cpp,h}

This revision was automatically updated to reflect the committed changes. Closed by commit rL368805: Removed dead code from clang/tools/libclang/CXIndexDataConsumer.{cpp,h} (authored by gribozavr, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prio

[PATCH] D66179: [LifetimeAnalysis] Support more STL idioms (template forward declaration and DependentNameType)

gribozavr added inline comments. Comment at: clang/lib/Sema/SemaAttr.cpp:102 + dyn_cast_or_null(Record->getDescribedTemplate())) { +if (auto *Def = Record->getDefinition()) + addGslOwnerPointerAttributeIfNotExisting(Context, Def); I wonder why

[PATCH] D66152: Fix false negatives of statement local lifetime analysis for some STL implementation

gribozavr accepted this revision. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Sema/SemaInit.cpp:6579 + if (Name.size() >= 2 && Name.front() == '_' && + (Name[1] == '_' || llvm::toUpper(Name[1]) == Name[1]))

[PATCH] D66209: Improved the doc comment for getCommentsInFile

gribozavr created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. gribozavr added a reviewer: hokein. hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. thanks! Repository: rG LLVM Github Monorepo https

[PATCH] D66209: Improved the doc comment for getCommentsInFile

This revision was automatically updated to reflect the committed changes. Closed by commit rL368827: Improved the doc comment for getCommentsInFile (authored by gribozavr, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://rev

[PATCH] D66212: Removed ToolExecutor::isSingleProcess, it is not used by anything

gribozavr created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. gribozavr added a reviewer: sammccall. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D66212 Files: clang/include/clang/Tooling/AllTUsExecution.h clang/include/clang/Toolin

[PATCH] D66212: Removed ToolExecutor::isSingleProcess, it is not used by anything

This revision was automatically updated to reflect the committed changes. Closed by commit rL368832: Removed ToolExecutor::isSingleProcess, it is not used by anything (authored by gribozavr, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to co

[PATCH] D66179: [LifetimeAnalysis] Support more STL idioms (template forward declaration and DependentNameType)

gribozavr added inline comments. Comment at: clang/lib/Sema/SemaAttr.cpp:200 CXXRecordDecl *Canonical = Record->getCanonicalDecl(); if (Canonical->hasAttr() || Canonical->hasAttr()) return; mgehre wrote: > gribozavr wrote: > > Should this code no

[PATCH] D66350: Rudimentary support for Doxygen \retval command

gribozavr accepted this revision. gribozavr added a comment. This revision is now accepted and ready to land. Thanks! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66350/new/ https://reviews.llvm.org/D66350 ___ cfe-c

[PATCH] D66270: [clang-tidy] Migrate objc-super-self to use isDerivedFrom 🚛

gribozavr added a comment. LGTM, thanks! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66270/new/ https://reviews.llvm.org/D66270 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/

[PATCH] D66269: [clang-tidy] Migrate objc-forbidden-subclassing to use isDerivedFrom 🚛

gribozavr added a comment. LGTM, thanks! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66269/new/ https://reviews.llvm.org/D66269 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/

[PATCH] D66462: Removed the 'id' AST matcher, which is superseded by '.bind()'

gribozavr created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The 'id' matcher is not even included in the AST Matchers Reference document, so I don't expect there to be a significant number of users. There's no reason to provide two ways to do the exact

[PATCH] D66462: Removed the 'id' AST matcher, which is superseded by '.bind()'

gribozavr updated this revision to Diff 216082. gribozavr added a comment. Fixed a typo. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66462/new/ https://reviews.llvm.org/D66462 Files: clang/include/clang/ASTMatchers/ASTMatchers.h clang/unitte

[PATCH] D66179: [LifetimeAnalysis] Support more STL idioms (template forward declaration and DependentNameType)

gribozavr added inline comments. Comment at: clang/lib/Sema/SemaAttr.cpp:200 CXXRecordDecl *Canonical = Record->getCanonicalDecl(); if (Canonical->hasAttr() || Canonical->hasAttr()) return; mgehre wrote: > gribozavr wrote: > > mgehre wrote: > > >

[PATCH] D66462: Removed the 'id' AST matcher, which is superseded by '.bind()'

This revision was automatically updated to reflect the committed changes. Closed by commit rL369380: Removed the 'id' AST matcher, which is superseded by '.bind()' (authored by gribozavr, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commi

[PATCH] D65182: [analyzer] Add fix-it hint support.

gribozavr added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:347 + + virtual llvm::ArrayRef getFixits() const { return Fixits; } + Why is it virtual? In fact, why is BugReporter subclassable at all? ==

[PATCH] D66473: Removed some dead code in BugReporter and related files

gribozavr created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. gribozavr added a reviewer: NoQ. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D66473 Files: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h clang/includ

[PATCH] D63288: [clang-tidy] Generalize TransformerClangTidyCheck to take a rule generator.

gribozavr accepted this revision. gribozavr added inline comments. Comment at: clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp:92 + const std::string Input = "void log(int);"; + EXPECT_EQ(Input, test::runCheckOnCode(Input)); +} Would a

[PATCH] D63763: [clang-tidy] Update documentation for Qt Creator integration.

gribozavr accepted this revision. gribozavr added a comment. This revision is now accepted and ready to land. LGTM, assuming you know what's new in QtCreator. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63763/new/ https://reviews.llvm.org/D63763

[PATCH] D63623: [clang-tidy] Add a check on expected return values of posix functions (except posix_openpt) in Android module.

gribozavr accepted this revision. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/docs/clang-tidy/checks/android-posix-return.rst:21 + int ret = posix_fadvise(...); + if (ret != 0) ... why not `if

[PATCH] D63623: [clang-tidy] new check: bugprone-posix-return

gribozavr added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/PosixReturnCheck.cpp:49 +diag(OperatorLoc, "POSIX functions (except posix_openpt) never return negative values") +<< FixItHint::CreateReplacement(OperatorLoc, Twine(">").str()); +r

[PATCH] D63784: [clang-tidy] Fix ClangTidyTest to initialize context before checks.

gribozavr accepted this revision. gribozavr added a comment. Good catch! Comment at: clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h:48 + +template class TestClangTidyAction : public ASTFrontendAction { "CheckTypes"? 'cause "Checks" below is also tech

[PATCH] D63288: [clang-tidy] Generalize TransformerClangTidyCheck to take a rule generator.

gribozavr accepted this revision. gribozavr added inline comments. Comment at: clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp:69 + +// A trivial rewrite rule generator that requires ObjectiveC code. +Optional needsObjC(const LangOptions &LangOpts, -

[PATCH] D63623: [clang-tidy] new check: bugprone-posix-return

gribozavr added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/PosixReturnCheck.cpp:49 +diag(OperatorLoc, "POSIX functions (except posix_openpt) never return negative values") +<< FixItHint::CreateReplacement(OperatorLoc, Twine(">").str()); +r

[PATCH] D63623: [clang-tidy] new check: bugprone-posix-return

gribozavr added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/PosixReturnCheck.cpp:61 + if (const auto *AlwaysTrueOp = Result.Nodes.getNodeAs("atop")) { +diag(AlwaysTrueOp->getOperatorLoc(), "%0 returns zero on success and a positive value on failure s

[PATCH] D63929: [clang-tidy] - Introduce abseil-prefixed-thread-annotations check.

gribozavr added a reviewer: gribozavr. gribozavr accepted this revision. gribozavr added a comment. This revision is now accepted and ready to land. Do you have commit access? Comment at: clang-tools-extra/clang-tidy/abseil/PrefixedThreadAnnotationsCheck.cpp:43 +FixItHint

[PATCH] D63929: [clang-tidy] - Introduce abseil-prefixed-thread-annotations check.

gribozavr added inline comments. Comment at: clang-tools-extra/clang-tidy/abseil/PrefixedThreadAnnotationsCheck.cpp:43 +FixItHint hint = FixItHint::CreateInsertion(Range.getBegin(), "ABSL_"); +Check.diag(Range.getBegin(), "usage of unprefixed thread annotation") +

[PATCH] D63892: [LibTooling] Extend `RewriteRule` with support for adding includes.

gribozavr accepted this revision. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:123 TextGenerator Explanation; +// Include paths that to add to the file affected by this

[PATCH] D63295: [clang][HeaderSearch] Shorten paths for includes in mainfile's directory

gribozavr accepted this revision. gribozavr added a comment. This revision is now accepted and ready to land. I'm not an expert in this code, but it looks reasonable. Comment at: clang-tools-extra/clangd/Headers.h:155 + /// \param IncludingFile Used to shorten the include for

[PATCH] D63623: [clang-tidy] new check: bugprone-posix-return

gribozavr accepted this revision. gribozavr added a comment. Good improvements! Comment at: clang-tools-extra/clang-tidy/bugprone/PosixReturnCheck.cpp:21 +static StringRef getFunctionSpelling(const MatchFinder::MatchResult &Result, const char *BindingStr) { + const CallExpr *

[PATCH] D63892: [LibTooling] Extend `RewriteRule` with support for adding includes.

gribozavr accepted this revision. gribozavr added a comment. LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63892/new/ https://reviews.llvm.org/D63892 ___ cfe-commits mailing list cfe-comm

[PATCH] D63893: [clang-tidy] Extend TransformerClangTidyCheck to support adding includes.

gribozavr accepted this revision. gribozavr added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp:51 + // includes. + if (Rule && std::any_of(Rule->Cases.begin(), Rule->Cases.end(), + [](const RewriteRule::C

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

gribozavr added inline comments. Comment at: clang/include/clang/Tooling/ReplacementsYaml.h:35 +: FilePath(""), Offset(0), Length(0), ReplacementText("") { + size_t lineBreakPos = ReplacementText.find('\n'); + while (lineBreakPos != std::string::npos) {

[PATCH] D63954: Add lifetime categories attributes

gribozavr added a comment. > We explicitly allow to add an annotation after the definition of the class to > allow adding annotations to external source from by the user Asking users to forward-declare anything from the standard library is a very bad idea -- in fact it is undefined behavior. h

[PATCH] D63954: Add lifetime categories attributes

gribozavr added a comment. > I agree. In a follow-up patch we will add the attributes to STL types during > parsing. Do you think it is OK to always add the attributes or should we only > do it if a flag, e.g. -wlifetime is added to the compiler invocation? Warning flags should not change the

[PATCH] D63954: Add lifetime categories attributes

gribozavr added a comment. In D63954#1565128 , @xazax.hun wrote: > In D63954#1565109 , @gribozavr wrote: > > > > I agree. In a follow-up patch we will add the attributes to STL types > > > during parsing. Do you th

[PATCH] D63623: [clang-tidy] new check: bugprone-posix-return

gribozavr accepted this revision. gribozavr added a comment. Thanks! Do you have commit access? Do you want me to commit this patch for you? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63623/new/ https://reviews.llvm.org/D63623 ___

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

gribozavr added inline comments. Comment at: clang/include/clang/Tooling/ReplacementsYaml.h:35 +: FilePath(""), Offset(0), Length(0), ReplacementText("") { + size_t lineBreakPos = ReplacementText.find('\n'); + while (lineBreakPos != std::string::npos) {

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

gribozavr accepted this revision. gribozavr added a comment. This revision is now accepted and ready to land. Thanks! Do you have commit access? Would you like me to commit this patch for you? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63482/new/ https://reviews.llvm.org/D63482 _

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

gribozavr added a comment. SVN repo is still there. However, I don't know how to commit using SVN, I commit using the git-svn integration (which still commits using the "svn" command under the hood, but as a user you will be interacting with a git repo). git clone https://github.com/llvm/llv

[PATCH] D63623: [clang-tidy] new check: bugprone-posix-return

gribozavr added a comment. Committed revision 365007. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63623/new/ https://reviews.llvm.org/D63623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm

[PATCH] D63623: [clang-tidy] new check: bugprone-posix-return

This revision was automatically updated to reflect the committed changes. Closed by commit rL365007: [clang-tidy] new check: bugprone-posix-return (authored by gribozavr, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://revi

[PATCH] D64151: Enhance abseil-faster-strsplit-delimiter to handle other non-printable characters.

gribozavr accepted this revision. gribozavr added a comment. This revision is now accepted and ready to land. Do you have commit access? Would you like me to commit this patch for you? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64151/new/ https:

[PATCH] D64151: Enhance abseil-faster-strsplit-delimiter to handle other non-printable characters.

gribozavr added a comment. Sorry, the patch does not apply cleanly to current master -- could you rebase please? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64151/new/ https://reviews.llvm.org/D64151 __

[PATCH] D64256: Teach some warnings to respect gsl::Pointer and gsl::Owner attributes

gribozavr added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:6510 LifetimeBoundCall, +LifetimePointerInit, +LifetimeTempOwner What is this name supposed to mean? Initialization of a "lifetime pointer"? What's a "lifetime pointer"? If yo

[PATCH] D64151: Enhance abseil-faster-strsplit-delimiter to handle other non-printable characters.

gribozavr marked an inline comment as done. gribozavr added inline comments. Comment at: clang-tools-extra/test/clang-tidy/abseil-faster-strsplit-delimiter.cpp:55 + + absl::StrSplit("ABC", R"(A)"); + // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit() called with a s

[PATCH] D64151: Enhance abseil-faster-strsplit-delimiter to handle other non-printable characters.

This revision was automatically updated to reflect the committed changes. Closed by commit rL365463: Enhance abseil-faster-strsplit-delimiter to handle other non-printable… (authored by gribozavr, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior

[PATCH] D64256: Teach some warnings to respect gsl::Pointer and gsl::Owner attributes

gribozavr added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:6549 +static bool +pathInitializeLifetimePointer(llvm::ArrayRef Path) { + return Path.size() > 0 && llvm::all_of(Path, [=](IndirectLocalPathEntry E) { Move closer to the point of usage? =

[PATCH] D64448: gsl::Owner/gsl::Pointer: Add implicit annotations for some std types

gribozavr added a comment. > The tests are currently here > I think due to their dependency on a standard library, they are not a good > fit for clang/test/. Where else could I put them? Make some mocks that reproduce the salient parts of different libraries, the coding patterns they use, and

[PATCH] D64256: Teach some warnings to respect gsl::Pointer and gsl::Owner attributes

gribozavr accepted this revision. gribozavr added a comment. This revision is now accepted and ready to land. I'm not an expert in SemaInit code, but this change LGTM. Comment at: clang/test/Sema/warn-lifetime-analysis-nocfg.cpp:8 + +struct OwnerWithConv; + Not

[PATCH] D64448: gsl::Owner/gsl::Pointer: Add implicit annotations for some std types

gribozavr added a comment. > Those are already there in clang/test/SemaCXX/attr-gsl-owner-pointer.cpp. I see. Sorry, but that seems insufficient to me -- different libraries use different patterns. For example, libc++ wraps everything in std in an inline namespace. I don't know how various vers

[PATCH] D63954: Add lifetime categories attributes

gribozavr added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:4164 + let Content = [{ +When annotating a class ``O`` with ``[[gsl::Owner(T)]]``, then each function +that returns cv-qualified ``T&`` or ``T*`` is assumed to return a Slightly b

[PATCH] D61485: Added an assert in `isConstantInitializer`: initializer lists must be in semantic form

This revision was automatically updated to reflect the committed changes. Closed by commit rL360406: Added an assert in `isConstantInitializer`: initializer lists must be in… (authored by gribozavr, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed pri

[PATCH] D61522: Added an assertion to constant evaluation enty points that prohibits dependent expressions

gribozavr added a comment. @ABataev @rsmith Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61522/new/ https://reviews.llvm.org/D61522 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:

[PATCH] D61522: Added an assertion to constant evaluation enty points that prohibits dependent expressions

This revision was automatically updated to reflect the committed changes. Closed by commit rL361050: Added an assertion to constant evaluation enty points that prohibits dependent… (authored by gribozavr, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Chang

[PATCH] D62125: Run ClangTidy tests in all C++ language modes

gribozavr created this revision. gribozavr added a reviewer: alexfh. Herald added subscribers: cfe-commits, lebedev.ri, jdoerfert, arphaman, kbarton, nemanjai. Herald added a reviewer: lebedev.ri. Herald added a project: clang. I inspected every test and did one of the following: - changed the t

[PATCH] D62125: Run ClangTidy tests in all C++ language modes

gribozavr added a comment. > c++98 coverage is a rather niche thing there days. I believe, someone with > genuine need for C++98 support can work on that. +1, that's exactly the reason why I skipped 98. C++98 is 21 years old now (or 16 if you count from C++03). Most projects that build with a

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

gribozavr added inline comments. Comment at: clang/include/clang/DirectoryWatcher/DirectoryWatcher.h:78 + /// The DirectoryWatcher that originated this event is no longer valid and + /// it's behavior is undefined. + /// The prime case is kernel signalling to OS-s

[PATCH] D62150: Renamed `apply` to `select` to avoid ADL conflict with `std::apply`

gribozavr created this revision. gribozavr added a reviewer: ilya-biryukov. Herald added a project: clang. Herald added a subscriber: cfe-commits. `std::apply` in C++14 and above is defined with two unrestricted arguments, and it wins overload resolution in this case. Repository: rG LLVM Githu

[PATCH] D62150: Renamed `apply` to `select` to avoid ADL conflict with `std::apply`

This revision was automatically updated to reflect the committed changes. Closed by commit rC361170: Renamed `apply` to `select` to avoid ADL conflict with `std::apply` (authored by gribozavr, committed by ). Changed prior to commit: https://reviews.llvm.org/D62150?vs=200313&id=200316#toc Repo

[PATCH] D62187: Delete default constructors, copy constructors, move constructors, copy assignment, move assignment operators on Expr, Stmt and Decl

gribozavr created this revision. gribozavr added reviewers: ilya-biryukov, rsmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D62187 Files: clang/include/clang/AST/DeclBase.h clang/include/clang/AST/Ex

[PATCH] D61386: [clang-tidy] Add support writing a check as a Transformer rewrite rule.

gribozavr accepted this revision. gribozavr added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clang-tidy/utils/TransformerTidy.h:32 +// }; +class TransformerTidy : public ClangTidyCheck { +public: I'd prefer a n

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

gribozavr added inline comments. Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:136 + +const int PollResult = poll(&PollReq, 1, TimeoutMs); +// There are inotify events waiting to be read! jkorous wrote: > gribozav

[PATCH] D61386: [clang-tidy] Add support writing a check as a Transformer rewrite rule.

gribozavr added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/TransformerTidy.h:32 +// }; +class TransformerTidy : public ClangTidyCheck { +public: ymandel wrote: > gribozavr wrote: > > I'd prefer a name like "TransformerClangTidyCheck", it will

[PATCH] D62187: Delete default constructors, copy constructors, move constructors, copy assignment, move assignment operators on Expr, Stmt and Decl

gribozavr marked 4 inline comments as done. gribozavr added inline comments. Comment at: clang/include/clang/AST/DeclBase.h:374 + Decl(const Decl&) = delete; + Decl(Decl &&) = delete; + Decl &operator=(const Decl&) = delete; ilya-biryukov wrote: > NIT: move co

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 🔍

gribozavr added a comment. > I prefer Option 2 because it is a cleaner, more understandable design for the > matchers. I agree with Aaron. Clang or Clang Tooling provide no promise of source stability. Of course we should not break things just because we can, but API coherence and maintainabil

[PATCH] D62187: Delete default constructors, copy constructors, move constructors, copy assignment, move assignment operators on Expr, Stmt and Decl

gribozavr marked 2 inline comments as done. gribozavr added inline comments. Comment at: clang/include/clang/AST/Stmt.h:1057 + Stmt() = delete; Stmt(const Stmt &) = delete; ilya-biryukov wrote: > gribozavr wrote: > > ilya-biryukov wrote: > > > NIT: Move the

[PATCH] D62187: Delete default constructors, copy constructors, move constructors, copy assignment, move assignment operators on Expr, Stmt and Decl

gribozavr updated this revision to Diff 200684. gribozavr marked 2 inline comments as done. gribozavr added a comment. Addressed review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62187/new/ https://reviews.llvm.org/D62187 Files: clan

[PATCH] D61487: [clang-tidy] Make the plugin honor NOLINT

gribozavr added inline comments. Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:457 + forwardDiagnostic(Info); + return; + } Indentation is 2 spaces. Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:472 if (Info.hasS

[PATCH] D62187: Delete default constructors, copy constructors, move constructors, copy assignment, move assignment operators on Expr, Stmt and Decl

This revision was automatically updated to reflect the committed changes. gribozavr marked an inline comment as done. Closed by commit rC361468: Delete default constructors, copy constructors, move constructors, copy… (authored by gribozavr, committed by ). Changed prior to commit: https://revi

[PATCH] D61350: [clang-tidy] New check calling out uses of +new in Objective-C code

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE361487: [clang-tidy] New check calling out uses of +new in Objective-C code (authored by gribozavr, committed by ). Changed prior to commit: https://reviews.llvm.org/D61350?vs=200824&id=200931#toc Re

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