[PATCH] D56849: [ASTMatchers][NFC] Update comments on assorted `CXXMemberCallExpr` matchers.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Thanks for tidying up the docs. LG with one nit. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:5018 +/// memberExpr(hasObjectExpression(hasType(cxxRecordDecl(hasN

[PATCH] D57087: [clang-tidy] add OverrideMacro to modernize-use-override check

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. I tend to think that a better migration strategy is to change the compiler to a C++11-compatible one first, and then turn on C++11 mode and migrate the code (possibly file-by-file or

[PATCH] D56303: [clang-tidy] Handle case/default statements when simplifying boolean expressions

alexfh added inline comments. Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:517 + has(compoundStmt( + has(caseStmt(hasDescendant(ifStmt(hasThen(returnsBool(Value)), +unless(hasElse(st

[PATCH] D56849: [ASTMatchers][NFC] Update comments on assorted `CXXMemberCallExpr` matchers.

alexfh accepted this revision. alexfh added a comment. LG CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56849/new/ https://reviews.llvm.org/D56849 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman

[PATCH] D55125: [clang-tidy] Fix a false positive in misc-redundant-expression check

alexfh added inline comments. Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:136 + case Stmt::UnaryExprOrTypeTraitExprClass: +if (cast(Left)->isArgumentType() && +cast(Right)->isArgumentType()) Any reasons not to pull out the results of `ca

[PATCH] D56851: [ASTMatchers] Adds `CXXMemberCallExpr` matcher `invokedAtType`.

alexfh added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3300 +/// matches `x.m()` and `p->m()`. +AST_MATCHER_P_OVERLOAD(clang::CXXMemberCallExpr, invokedAtType, + clang::ast_matchers::internal::Matcher, The

[PATCH] D56851: [ASTMatchers] Adds `CXXMemberCallExpr` matcher `invokedAtType`.

alexfh added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3300 +/// matches `x.m()` and `p->m()`. +AST_MATCHER_P_OVERLOAD(clang::CXXMemberCallExpr, invokedAtType, + clang::ast_matchers::internal::Matcher, yman

[PATCH] D53953: [clang-tidy] Get ClangTidyContext out of the business of storing diagnostics. NFC

alexfh added a comment. LG in general. > The most natural implementation (ClangTidyDiagnosticsConsumer::take() > finalizes diagnostics) causes a test failure: > clang-tidy-run-with-database.cpp > asserts that clang-tidy exits successfully when trying to process a file > that doesn't exist. >

[PATCH] D53953: [clang-tidy] Get ClangTidyContext out of the business of storing diagnostics. NFC

alexfh added a comment. > error: unable to handle compilation, expected exactly one compiler job in '' > [clang-diagnostic-error] > Suppressed 1 warnings (1 with check filters). > Found compiler error(s). That's preferred to the current behavior, but from a user perspective a more descriptive

[PATCH] D53936: [clang-tidy] More clearly separate public, check-facing APIs from internal ones.

alexfh added inline comments. Comment at: clang-tidy/ClangTidy.h:11 +// +// It should remain as stable as possible, as many out-of-tree checks exist. +//===--===// sammccall wrote: > steveire wro

[PATCH] D53953: [clang-tidy] Get ClangTidyContext out of the business of storing diagnostics. NFC

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG with a nit. Comment at: clang-tidy/ClangTidyDiagnosticConsumer.h:259 + std::vector Errors; + std::vector FinalizedErrors; std::unique_ptr HeaderFilter; --

[PATCH] D53830: [clang-tidy]: Abseil: new check 'abseil-upgrade-duration-conversions'

alexfh added inline comments. Comment at: clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp:33-36 + anyOf(hasAncestor( +functionTemplateDecl(HasMatchingDependentDescendant)), +hasAncestor( +class

[PATCH] D53953: [clang-tidy] Get ClangTidyContext out of the business of storing diagnostics. NFC

alexfh accepted this revision. alexfh added a comment. LG Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53953 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

alexfh added inline comments. Comment at: unittests/clang-tidy/AddConstTest.cpp:733 + StringRef T = "template void f(T v) \n"; + StringRef S = "{ T target = v; }"; + auto Cat = [&T](StringRef S) { return (T + S).str(); }; It would be interesting to see test c

[PATCH] D53830: [clang-tidy]: Abseil: new check 'abseil-upgrade-duration-conversions'

alexfh added inline comments. Comment at: clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp:33-36 + anyOf(hasAncestor( +functionTemplateDecl(HasMatchingDependentDescendant)), +hasAncestor( +class

[PATCH] D54453: Remove myself as owner of clang-query.

alexfh added a comment. > Thanks Aaron for volunteering, I'm very thankful for all your work on the > reviews, it's much appreciated :D +1 ;) Repository: rL LLVM https://reviews.llvm.org/D54453 ___ cfe-commits mailing list cfe-commits@lists.llv

[PATCH] D53427: [clangd] Replace StringRef in SymbolLocation with a char pointer.

alexfh added inline comments. Comment at: clang-tools-extra/trunk/clangd/index/Index.h:71 + // via llvm::StringRef. + const char *FileURI = ""; }; If the users of the SymbolLocation have a way to get a common "context", you could keep a list of filenames in t

[PATCH] D53427: [clangd] Replace StringRef in SymbolLocation with a char pointer.

alexfh added inline comments. Comment at: clang-tools-extra/trunk/clangd/index/Index.h:71 + // via llvm::StringRef. + const char *FileURI = ""; }; hokein wrote: > alexfh wrote: > > If the users of the SymbolLocation have a way to get a common "context", > > y

[PATCH] D53830: [clang-tidy]: Abseil: new check 'abseil-upgrade-duration-conversions'

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG with one more comment. Please address other reviewers' comments though. Comment at: clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp:153 + // required so we provide

[PATCH] D54941: [clang-tidy] Ignore bool -> single bit bitfield conversion in readability-implicit-bool-conversion

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG Comment at: docs/clang-tidy/checks/readability-implicit-bool-conversion.rst:77-78 -- boolean expression/literal to integer, +- boolean expression/literal to integer (co

[PATCH] D54945: This commit adds a chapter about clang-tidy integrations

alexfh added a comment. Should this chapter be a separate page linked from index.md? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54945/new/ https://reviews.llvm.org/D54945 ___ cfe-commits mailing list

[PATCH] D55255: Fix a false positive in misplaced-widening-cast

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG in general, but a few comments re: test. Comment at: test/clang-tidy/bugprone-misplaced-widening-cast-explicit-only.cpp:67-73 +MON = 0, +TUE = 1, +WED = 2, +THR = 3, +FRI

[PATCH] D54945: This commit adds a chapter about clang-tidy integrations

alexfh added a comment. In D54945#1316251 , @MarinaKalashina wrote: > Fixes: > > - empty line before 'Standalone tool' > - table columns with '+/-' aligned > - line width limited to 80 (except for the table) > > Additions: > - clang-tidy-vs plugin > - C

[PATCH] D54401: [analyzer] Prefer returns values to out-params in CheckerRegistry.cpp

alexfh added a comment. In D54401#1315354 , @NoQ wrote: > The code looks good, but i vaguely remember that we might accidentally break > clang-tidy integration that uses this API to enable/disable specific checkers > via `-checks=-analyzer-...`. > > *sum

[PATCH] D55125: [clang-tidy] Fix a false positive in misc-redundant-expression check

alexfh added inline comments. Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:601 +static bool compareToks(Token &T1, Token &T2, const SourceManager &SM) { + if (T1.getLength() != T2.getLength()) Should this function compare token kinds first? =

[PATCH] D54945: This commit adds a chapter about clang-tidy integrations

alexfh added a comment. In D54945#1318283 , @MarinaKalashina wrote: > In D54945#1318278 , @alexfh wrote: > > > In D54945#1316251 , > > @MarinaKalashina wrote: > > > > > Fix

[PATCH] D55346: [clang-tidy] check for using declaration scope and qualification

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. Could you send each check in a separate patch, please? This makes the review much more straightforward and I'm sure it will be faster too. Repository: rCTE Clang Tools Extra CHAN

[PATCH] D55346: [clang-tidy] check for using declaration scope and qualification

alexfh added inline comments. Comment at: clang-tidy/abseil/AbseilTidyModule.cpp:25 #include "StrCatAppendCheck.h" Please upload the patch with full context. See https://llvm.org/docs/Phabricator.html Repository: rCTE Clang Tools Extra CHANGES SINCE LAST

[PATCH] D54945: This commit adds a chapter about clang-tidy integrations

alexfh added a comment. In D54945#1318343 , @MarinaKalashina wrote: > @alexfh Thanks a lot for your patience and help. I've made another revision, > now with the diff made by 'git show HEAD -U99' to have the full context > availlable. I don't know

[PATCH] D54945: This commit adds a chapter about clang-tidy integrations

alexfh added a comment. In D54945#1321301 , @alexfh wrote: > In D54945#1318343 , @MarinaKalashina > wrote: > > > @alexfh Thanks a lot for your patience and help. I've made another > > revision, now with the diff m

[PATCH] D54401: [analyzer] Prefer returns values to out-params in CheckerRegistry.cpp

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. In D54401#1320772 , @Szelethus wrote: > In D54401#1318282 , @alexfh wrote: > > > In D54401#1315354

[PATCH] D49890: Clang-Tidy Export Problem

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. In D49890#1200061 , @TheAhmad wrote: > In D49890#1182556 , @alexfh wrote: > > > Could you describe th

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

alexfh added a comment. Thank you for the contribution! Please see the comments inline. Comment at: clang-tidy/modernize/UseOverrideCheck.cpp:22 // Only register the matcher for C++11. - if (getLangOpts().CPlusPlus11) -Finder->addMatcher(cxxMethodDecl(isOverride()).bind

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

alexfh added inline comments. Comment at: clang-tidy/modernize/UseOverrideCheck.h:23 + : ClangTidyCheck(Name, Context), +IgnoreDestructors(Options.get("IgnoreDestructors", false)) {} void registerMatchers(ast_matchers::MatchFinder *Finder) override; --

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

alexfh added inline comments. Comment at: test/clang-tidy/modernize-use-override-no-destructors.cpp:6 +struct Base { + virtual ~Base(){}; + virtual void f(){}; Remove the semicolons after methods. Clang-format the test (but ensure clang-format doesn't break co

[PATCH] D58606: [clang-tidy] misc-string-integer-assignment: fix false positive

alexfh accepted this revision. alexfh added a comment. LG. Thanks for improving this check! Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58606/new/ https://reviews.llvm.org/D58606 ___ cfe-commits maili

[PATCH] D57087: [clang-tidy] add OverrideMacro to modernize-use-override check

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG. Sorry for the delay. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57087/new/ https://reviews.llvm.org/D57087 ___ cfe-commits mailin

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

alexfh added a comment. A few more comments. Comment at: clang-tidy/utils/FixItHintUtils.cpp:83 +return (llvm::Twine(' ') + +llvm::Twine(DeclSpec::getSpecifierName(Qualifier))) +.str(); The first operand being llvm::Twine is enough for t

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good! Do you need someone to commit the patch for you? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58731/new/ https://reviews.llvm.org/D58731 __

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

alexfh added a comment. In D58731#1412831 , @lewmpk wrote: > I'm trying to find a way to run the test. If someone else has already tested > it then yes please. If you've set up the build with ninja, `ninja check-clang-tools` should do the right thing.

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

alexfh added a comment. In D58731#1412843 , @alexfh wrote: > In D58731#1412831 , @lewmpk wrote: > > > I'm trying to find a way to run the test. If someone else has already > > tested it then yes please. > > > If yo

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

alexfh added a comment. In D58731#1413552 , @JonasToth wrote: > In D58731#1413498 , @MyDeveloperDay > wrote: > > > In D58731#1413476 , @lewmpk wrote: > > > > > I'm happy to

[PATCH] D58819: [clang-format] clang-format off/on not respected when using C Style comments

alexfh added inline comments. Comment at: clang/unittests/Format/SortIncludesTest.cpp:132 + "#include \n" + "/* clang-format off */\n" + "#include \n" Add a test with `/* clang-format officially supports C++ */` ;)

[PATCH] D58819: [clang-format] clang-format off/on not respected when using C Style comments

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58819/new/ https://reviews.llvm.org/D58819 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D59054: [analyzer] C++17: PR40022: Support aggregate initialization with compound values in presence of base classes.

alexfh added a comment. LG in general. A couple of comments. Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:2348 +// Virtual bases still aren't allowed. Multiple bases are fine though. +for (auto B : CRD->bases()) { + assert(B.isVirtual() == false); ---

[PATCH] D59054: [analyzer] C++17: PR40022: Support aggregate initialization with compound values in presence of base classes.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59054/new/ https://reviews.llvm.org/D59054 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D48866: [clang-tidy] Add incorrect-pointer-cast checker

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. As per the very first comment, "bugprone" will be a more suitable category for this check. The rename_check.py script should be able to help here. Repository: rCTE Clang Tools Ext

[PATCH] D48866: [clang-tidy] Add incorrect-pointer-cast checker

alexfh added a comment. And one more comment that hasn't yet been addressed: "Some patterns are covered by compiler diagnostics: https://godbolt.org/g/HvsjnP. Is there any benefit in re-implementing them?" And one more: please update file headers. The license has changed since. Repository:

[PATCH] D59255: [clang-tidy] NOLINT support for "clang-diagnostic-*".

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59255/new/ https://reviews.llvm.org/D59255 ___ cf

[PATCH] D59135: Add check for matching HeaderFilter before emitting Diagnostic

alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:453 + + StringRef FileName(Loc.printToString(Loc.getManager())); + if(getHeaderFilter

[PATCH] D59135: Add check for matching HeaderFilter before emitting Diagnostic

alexfh added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:454 + StringRef FileName(Loc.printToString(Loc.getManager())); + if(getHeaderFilter()->match(FileName)) + { aaron.ballman wrote: > Formatting is incorrect he

[PATCH] D59360: [clang-tidy] Fix more false positives for bugprone-string-integer-assignment

alexfh added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp:50 +public: + CharExpressionDetector(const QualType CharType, const ASTContext &Ctx) + : CharType(CharType), Ctx(Ctx) {} No need for the top-le

[PATCH] D59528: [clang-tidy] Expand modular headers for PPCallbacks

alexfh created this revision. alexfh added reviewers: gribozavr, usaxena95, sammccall. Herald added subscribers: jdoerfert, kbarton, xazax.hun, mgorny, nemanjai. Herald added a project: clang. Add a way to expand modular headers for PPCallbacks. Checks can opt-in for this expansion by overriding t

[PATCH] D59528: [clang-tidy] Expand modular headers for PPCallbacks

alexfh updated this revision to Diff 191227. alexfh added a comment. - Fix a typo in the comment. Rearrange paragraphs to make the comment more readable. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59528/new/ https://reviews.llvm.org/D59528 Fil

[PATCH] D59528: [clang-tidy] Expand modular headers for PPCallbacks

alexfh updated this revision to Diff 191230. alexfh added a comment. - Fix naming. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59528/new/ https://reviews.llvm.org/D59528 Files: clang-tools-extra/clang-tidy/CMakeLists.txt clang-tools-extra/cl

[PATCH] D59540: [clang-tidy] [PR41119] readability-identifier-naming incorrectly fixes lambda capture

alexfh added a comment. Thanks for fixing this! Could you expand the test a bit? See the inline comment. Comment at: test/clang-tidy/readability-identifier-naming.cpp:506 +bool Foo() { + bool Columns=false; +// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for lo

[PATCH] D57662: [clang-tidy] Parallelize clang-tidy-diff.py

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG with a nit Comment at: clang-tidy/tool/clang-tidy-diff.py:90 + # the top level key 'Diagnostics' in the output yaml files + mergekey="Diagnostics" + merged=[] -

[PATCH] D59449: [clang-tidy] Integrate clang-tidy-diff.py machinery into run-clang-tidy.py

alexfh added a comment. How is this functionality different from the clang-tidy-diff.py script with the -j option being added in the other patch? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59449/new/ https://reviews.llvm.org/D59449 __

[PATCH] D59360: [clang-tidy] Fix more false positives for bugprone-string-integer-assignment

alexfh added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp:115 +Expr::EvalResult EvalResult; +if (!E->EvaluateAsInt(EvalResult, Ctx, Expr::SE_AllowSideEffects)) + return false; courbet wrote: > a

[PATCH] D58278: Prepare ground for re-lexing modular headers.

alexfh abandoned this revision. alexfh added a comment. I've found an alternative way to deal with PPCallbacks when analyzing code in modular mode. See https://reviews.llvm.org/D59528. Abandoning this revision. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D59528: [clang-tidy] Expand modular headers for PPCallbacks

alexfh added inline comments. Comment at: clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp:24 + void recordContent(const FileEntry *File, + const SrcMgr::ContentCache *ContentCache, + llvm::vfs::InMemoryFileSystem &InMemory

[PATCH] D59528: [clang-tidy] Expand modular headers for PPCallbacks

alexfh updated this revision to Diff 191703. alexfh marked 15 inline comments as done. alexfh added a comment. - Addressed review comments. Parse code to the end (seems right to do this, but no specific test that would fail otherwise). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

[PATCH] D59528: [clang-tidy] Expand modular headers for PPCallbacks

alexfh added a comment. Thank you for the review! I hope I covered all the points. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59528/new/ https://reviews.llvm.org/D59528 ___ cfe-commits mailing list

[PATCH] D59528: [clang-tidy] Expand modular headers for PPCallbacks

alexfh marked 3 inline comments as done. alexfh added inline comments. Comment at: clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp:166 +Preprocessor->Lex(CurrentToken); +} + gribozavr wrote: > Haha, so the test that I asked to add did catch a

[PATCH] D59528: [clang-tidy] Expand modular headers for PPCallbacks

alexfh updated this revision to Diff 191871. alexfh marked an inline comment as done. alexfh added a comment. - - Update a comment according to the review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59528/new/ https://reviews.llvm.org/D

[PATCH] D59528: [clang-tidy] Expand modular headers for PPCallbacks

This revision was automatically updated to reflect the committed changes. alexfh marked an inline comment as done. Closed by commit rL356750: [clang-tidy] Expand modular headers for PPCallbacks (authored by alexfh, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commi

[PATCH] D59360: [clang-tidy] Fix more false positives for bugprone-string-integer-assignment

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG. Thanks for improving the check! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59360/new/ https://reviews.llvm.org/D59360

[PATCH] D59714: [clang-tidy] Separate the check-facing interface

alexfh updated this revision to Diff 191937. alexfh added a comment. - Reverted the part to simplify the review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59714/new/ https://reviews.llvm.org/D59714 Files: clang-tools-extra/clang-tidy/CMake

[PATCH] D59714: [clang-tidy] Separate the check-facing interface

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE356890: [clang-tidy] Separate the check-facing interface (authored by alexfh, committed by ). Changed prior to commit: https://reviews.llvm.org/D59714?vs=191937&id=192085#toc Repository: rCTE Clang

[PATCH] D59622: [analyzer] C++17: PR41142: Ignore transparent InitListExprs in ExprEngine as well.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG. This fixes (at least, most of) the crashes we observe. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59622/new/ https://reviews.llvm.org/D59622

[PATCH] D59857: [analyzer] PR37501: Disable the assertion for reverse-engineering logical op short circuits.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59857/new/ https://reviews.llvm.org/D59857 ___ cfe-commits mail

[PATCH] D59901: [analyzer] PR41239: Fix a crash on invalid source location in NoStoreFuncVisitor.

alexfh added a comment. I've posted another test case to https://bugs.llvm.org/show_bug.cgi?id=41239. It may already be covered by the fix. Could you check? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59901/new/ https://reviews.llvm.org/D59901 _

[PATCH] D59963: [clang-tidy] Add module for the Linux kernel.

alexfh added a comment. Looks reasonable in general, but we usually add modules with at least one check. Let's do the same here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59963/new/ https://reviews.llvm.org/D59963 __

[PATCH] D59449: [clang-tidy] Integrate clang-tidy-diff.py machinery into run-clang-tidy.py

alexfh added a comment. In D59449#1435836 , @zinovy.nis wrote: > In D59449#1435032 , @alexfh wrote: > > > How is this functionality different from the clang-tidy-diff.py script with > > the -j option being added in

[PATCH] D59859: [clang-tidy] FIXIT for implicit bool conversion now obeys upper case suffixes if enforced.

alexfh added a comment. In D59859#1444333 , @aaron.ballman wrote: > In D59859#1444176 , @lebedev.ri > wrote: > > > I'm not sure why we want this? What is wrong with simply applying > > clang-tidy twice? > > > It

[PATCH] D59859: [clang-tidy] FIXIT for implicit bool conversion now obeys upper case suffixes if enforced.

alexfh added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp:48 case CK_IntegralToBoolean: -return Type->isUnsignedIntegerType() ? "0u" : "0"; +if (UppercaseSuffix) { + return Type->isUnsignedIntegerType() ? "

[PATCH] D59859: [clang-tidy] FIXIT for implicit bool conversion now obeys upper case suffixes if enforced.

alexfh added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp:270 + AllowPointerConditions(Options.get("AllowPointerConditions", false)), + UseUppercaseLiteralSuffix(Context->isCheckEnabled("readability-uppercase-li

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:75 Context.diag(CheckName, PD->getLocation().asLocation(), - PD->getShortDescrip

[PATCH] D60197: [clang-tidy] Remove the old ClangTidyCheck::registerPPCallbacks method

alexfh created this revision. alexfh added reviewers: sammccall, hokein. Herald added subscribers: kadircet, arphaman, jkorous, xazax.hun. Herald added a project: clang. All in-tree clang-tidy checks have been migrated to the new ClangTidyCheck::registerPPCallbacks method. Time to drop the old one

[PATCH] D60151: [clang-tidy] Add using SmallSet to LLVM.h to fix bug in typedef in llvm checkers.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. The change looks fine, but I don't understand the description of this revision. Could you clarify which checkers you're talking about and which bug you observe? Repository: rG LLVM Github M

[PATCH] D59963: [clang-tidy] Add a module for the Linux kernel.

alexfh added a comment. Can you verify that the add_new_check.py script works fine with this new module? Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59963/new/ https://reviews.llvm.org/D59963 ___

[PATCH] D60197: [clang-tidy] Remove the old ClangTidyCheck::registerPPCallbacks method

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE357582: [clang-tidy] Remove the old ClangTidyCheck::registerPPCallbacks method (authored by alexfh, committed by ). Changed prior to commit: https://reviews.llvm.org/D60197?vs=193481&id=193486#toc Re

[PATCH] D33841: [clang-tidy] redundant keyword check

alexfh added inline comments. Comment at: docs/clang-tidy/checks/readability-redundant-keyword.rst:8 + +`extern` is redundant in function declarations + koldaniel wrote: > alexfh wrote: > > xazax.hun wrote: > > > alexfh wrote: > > > > Could you explain, why you t

[PATCH] D59449: [clang-tidy] Integrate clang-tidy-diff.py machinery into run-clang-tidy.py

alexfh added a comment. In D59449#1449664 , @zinovy.nis wrote: > > Why not just use clang-tidy-diff.py? The clang-tidy-diff.py script has a > > distinct and somewhat self-documenting name and a very specific purpose > > (find clang-tidy regressions in a

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

alexfh added a comment. This looks like a more promising direction. Thanks for the readiness to experiment with this. See the comments inline. Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:130 + + // If we have alternative fixes (attached via diagnostic::Notes),

[PATCH] D60151: [clang-tidy] Add using SmallSet to LLVM.h to fix bug in typedef in llvm checkers.

alexfh added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h:21 -typedef llvm::SmallSet HeaderFileExtensionsSet; +using HeaderFileExtensionsSet = SmallSet; hintonda wrote: > aaron.ballman wrote: > > hintonda wrote:

[PATCH] D60151: [clang-tidy] Rename llvm checkers to llvm-project

alexfh added a comment. In D60151#1454105 , @hintonda wrote: > - Rename llvm directory to llvm_project. > - Change llvm- to llvm-project-. > - Rename files. Awesome! Thanks for doing this. Could you ensure that the add_new_check.py script still works?

[PATCH] D59540: [clang-tidy] [PR41119] readability-identifier-naming incorrectly fixes lambda capture

alexfh added a comment. In D59540#1456011 , @MyDeveloperDay wrote: > friendly ping Sorry for the delay. Feel free to ping earlier. And more frequently :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59540/new/ https://reviews.llvm.org/D59540

[PATCH] D59540: [clang-tidy] [PR41119] readability-identifier-naming incorrectly fixes lambda capture

alexfh added a comment. Thanks, looks better now, but still a few comments, mostly nits. Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:701 + const ASTContext::DynTypedNodeList &Parents = Context->getParents(*DeclRef); + if (!Parents.empty()) { +const Stmt *

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

alexfh added inline comments. Comment at: clang/include/clang/Tooling/Core/Diagnostic.h:67-71 + // Get the chosen fix to apply for this diagnostic. + // FIXME: currently we choose the first non-empty fix, extend it to support + // fix selection. + const llvm::StringMap *getCh

[PATCH] D59135: Add check for matching HeaderFilter before emitting Diagnostic

alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:454 + StringRef FileName(Loc.printToString(Loc.getManager())); + if(getHeaderFilter()

[PATCH] D59746: [LibTooling] Fix '-h' option

alexfh added a comment. Can you give a specific example of how this problem manifests? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59746/new/ https://reviews.llvm.org/D59746 ___ cfe-commits mailing l

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

alexfh added inline comments. Comment at: clang/include/clang/Tooling/Core/Diagnostic.h:67-71 + // Get the chosen fix to apply for this diagnostic. + // FIXME: currently we choose the first non-empty fix, extend it to support + // fix selection. + const llvm::StringMap *getCh

[PATCH] D60139: [clang-tidy] Add misc-placement-new-target-type-mismatch check

alexfh added a comment. Looks like this check would fit better into the `bugprone` module. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60139/new/ https://reviews.llvm.org/D60139 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

[PATCH] D59977: [Lexer] Fix an off-by-one bug in Lexer::getAsCharRange() - NFC.

alexfh added a comment. It looks like there's a number of users of this function beyond what you've mentioned: clang-tidy/readability/IsolateDeclarationCheck.cpp:210 clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:43 clang-tidy/readability/NamespaceCommentCheck.cpp:106 clang/lib/ARCMigrate/P

[PATCH] D59135: Add check for matching HeaderFilter before emitting Diagnostic

alexfh added a comment. In D59135#1458265 , @thorsten-klein wrote: > Hello @alexfh , > Let me extend your example > > $ cat a.cc > #include "b.h" > #include "d.h" > int main(){check(nullptr);} > $ cat b.h > #include "c.h" > inline void b(

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

alexfh added a comment. Thanks for the useful check! I have a few comments, see inline. Comment at: clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp:34-35 + binaryOperator(anyOf(hasOperatorName("=="), hasOperatorName("!=")), + any

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Thanks! The change looks good now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59932/new/ https://reviews.llvm.org/D59932 _

[PATCH] D60776: [clang-tidy] Add test support for the fix description.

alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: clang-tools-extra/test/clang-tidy/check_clang_tidy.py:109 check_notes_prefix = 'CHECK-NOTES' + file_check_suffix +check_fix_descriptions_pr

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_check'

alexfh added inline comments. Comment at: clang-tools-extra/clang-tidy/add_new_check.py:381 + # Don't allow 'clang' or 'llvm' namespaces + if module == 'llvm' or module == 'clang': +namespace = module + '_check' We don't have the `clang` module. No need to

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