[PATCH] D140772: [clang-tidy] Fix minor bug in add_new_check.py

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/add_new_check.py:324-328 doc_files = [] - for subdir in list(filter(lambda s: not s.endswith('.rst') and not s.endswith('.py'), - os.listdir(docs_dir))): + for subdir in

[PATCH] D140018: [clang-tidy] Support std::string_view in readability-redundant-string-cstr

carlosgalvezp added a comment. @tberghammer Do you need help to land the patch? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140018/new/ https://reviews.llvm.org/D140018 ___ cfe-commits mailing list

[PATCH] D140772: [clang-tidy] Fix minor bug in add_new_check.py

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. LGTM, I have a suggestion for further improvement. Comment at: clang-tools-extra/clang-tidy/add_new_check.py:324-328 doc_files = [] - for subdir in

[PATCH] D140290: [clang-tidy] Add misc-static-declaration-in-header check

carlosgalvezp added a comment. I'm happy with the check now, feel free to review! I have run it on the LLVM repo and it seems to do what it's supposed to (including fix-its). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140290/new/

[PATCH] D140290: [clang-tidy] Add misc-static-declaration-in-header check

carlosgalvezp updated this revision to Diff 485396. carlosgalvezp added a comment. Narrow warnings further, and provide fix-its. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140290/new/ https://reviews.llvm.org/D140290 Files:

[PATCH] D140290: [clang-tidy] Add misc-static-declaration-in-header check

carlosgalvezp updated this revision to Diff 485236. carlosgalvezp added a comment. Remove remaining function declaration in the check class. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140290/new/ https://reviews.llvm.org/D140290 Files:

[PATCH] D140290: [clang-tidy] Add misc-static-declaration-in-header check

carlosgalvezp updated this revision to Diff 485235. carlosgalvezp added a comment. Warn only about cases where there really is a problem. There are other cases which are just a readability issue, where a separate check for "redundant static" would fit better. Remove also the automatic fix-it

[PATCH] D140328: [ASTMatchers] Add isInAnonymousNamespace narrowing matcher

This revision was automatically updated to reflect the committed changes. Closed by commit rG125ccd375147: [ASTMatchers] Add isInAnonymousNamespace narrowing matcher (authored by carlosgalvezp). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D138655: [clang-tidy] Fix `cppcoreguidelines-init-variables` for invalid vardecl

carlosgalvezp added a comment. Now that I think a bit better about this I wonder - does it really make sense that we increase the complexity of the check to cover for cases where code does not compile? If it fails to include a header, there's many other things that can possibly go wrong -

[PATCH] D137302: [clang-tidy] Add modernize-type-traits check

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.cpp:140 +const internal::VariadicDynCastAllOfMatcher +dependentNameTypeLoc; // NOLINT(readability-identifier-naming) + I don't see a reason why the naming

[PATCH] D140018: [clang-tidy] Support std::string_view in readability-redundant-string-cstr

carlosgalvezp accepted this revision. carlosgalvezp added a comment. LGTM, perhaps give a couple days to @njames93 to give a final look? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140018/new/ https://reviews.llvm.org/D140018

[PATCH] D140328: [ASTMatchers] Add isInAnonymousNamespace narrowing matcher

carlosgalvezp created this revision. Herald added a project: All. carlosgalvezp requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Used in a couple clang-tidy checks so it could be extracted out as its own matcher. Repository: rG LLVM

[PATCH] D140290: [clang-tidy] Add misc-static-declaration-in-header check

carlosgalvezp updated this revision to Diff 484000. carlosgalvezp added a comment. Replace quotes with single backticks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140290/new/ https://reviews.llvm.org/D140290 Files:

[PATCH] D140290: [clang-tidy] Add misc-static-declaration-in-header check

carlosgalvezp added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/misc/static-declaration-in-header.rst:39 + A semicolon-separated list of filename extensions of header files (the filename + extensions should not include "." prefix). Default is

[PATCH] D140290: [clang-tidy] Add misc-static-declaration-in-header check

carlosgalvezp added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/misc/static-declaration-in-header.rst:39 + A semicolon-separated list of filename extensions of header files (the filename + extensions should not include "." prefix). Default is

[PATCH] D140290: [clang-tidy] Add misc-static-declaration-in-header check

carlosgalvezp updated this revision to Diff 483981. carlosgalvezp marked an inline comment as done. carlosgalvezp added a comment. Add description in release notes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140290/new/

[PATCH] D140290: [clang-tidy] Add misc-static-declaration-in-header check

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/StaticDeclarationInHeaderCheck.cpp:46 +: ClangTidyCheck(Name, Context), + RawStringHeaderFileExtensions(Options.getLocalOrGlobal( + "HeaderFileExtensions",

[PATCH] D140290: [clang-tidy] Add misc-static-declaration-in-header check

carlosgalvezp created this revision. Herald added a subscriber: xazax.hun. Herald added a reviewer: njames93. Herald added a project: All. carlosgalvezp requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Static declarations in

[PATCH] D138655: [clang-tidy] Fix `cppcoreguidelines-init-variables` for invalid vardecl

carlosgalvezp added a comment. Looks good in general, IMHO it would be good with a couple more comments to make the code and test easier to understand. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp:63 + if (MatchedDecl->isInvalidDecl())

[PATCH] D139966: [clang-tidy] Use Python3 for add_new_check.py and rename_check.py

This revision was automatically updated to reflect the committed changes. Closed by commit rG66bf54abb54e: [clang-tidy] Use Python3 for add_new_check.py and rename_check.py (authored by carlosgalvezp). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D139966: [clang-tidy] Use Python3 for add_new_check.py and rename_check.py

carlosgalvezp added a comment. In D139966#3999473 , @njames93 wrote: > I'm just a little uneasy with the description, this isn't "fixing" the linked > issue, its more a workaround. The issue itself is also a bit of a non-issue. Thanks for reviewing!

[PATCH] D140018: [clang-tidy] Support std::string_view in readability-redundant-string-cstr

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. LGTM! Perhaps give the other reviewers a couple days to take a look in case I've missed anything. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D140018: [clang-tidy] Support std::string_view in readability-redundant-string-cstr

carlosgalvezp added a comment. Please document the changes in the Release Notes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140018/new/ https://reviews.llvm.org/D140018 ___ cfe-commits mailing list

[PATCH] D139966: [clang-tidy] Use Python3 for add_new_check.py and rename_check.py

carlosgalvezp updated this revision to Diff 482722. carlosgalvezp added a comment. Wrap to 80 chars. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139966/new/ https://reviews.llvm.org/D139966 Files: clang-tools-extra/clang-tidy/add_new_check.py

[PATCH] D139966: [clang-tidy] Use Python3 for add_new_check.py and rename_check.py

carlosgalvezp updated this revision to Diff 482721. carlosgalvezp added a comment. Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139966/new/ https://reviews.llvm.org/D139966 Files: clang-tools-extra/clang-tidy/add_new_check.py

[PATCH] D139966: [clang-tidy] Use Python3 for add_new_check.py and rename_check.py

carlosgalvezp updated this revision to Diff 482611. carlosgalvezp added a comment. Mention change in the Release Notes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139966/new/ https://reviews.llvm.org/D139966 Files:

[PATCH] D139966: [clang-tidy] Use Python3 for add_new_check.py and rename_check.py

carlosgalvezp updated this revision to Diff 482609. carlosgalvezp added a comment. Revert format changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139966/new/ https://reviews.llvm.org/D139966 Files:

[PATCH] D139966: [clang-tidy] Use Python3 for add_new_check.py and rename_check.py

carlosgalvezp created this revision. Herald added a subscriber: xazax.hun. Herald added a reviewer: njames93. Herald added a project: All. carlosgalvezp requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Fixes #58782 Repository:

[PATCH] D137514: [clang-tidy] add check for capturing lambda coroutines

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.cpp:40 + + diag(Lambda->getBeginLoc(), "found capturing coroutine lambda"); +} carlosgalvezp wrote: > Perhaps it would be good

[PATCH] D137514: [clang-tidy] add check for capturing lambda coroutines

carlosgalvezp requested changes to this revision. carlosgalvezp added a comment. This revision now requires changes to proceed. Looking good! Some minor comments Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.cpp:40 + +

[PATCH] D139919: use std::optional in ClangTidyCheck

carlosgalvezp added a comment. Sounds like the change to the YAML parser can potentially affect many other components other than clang-tidy - should that be done in a separate patch? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139919/new/ https://reviews.llvm.org/D139919

[PATCH] D139919: use std::optional in ClangTidyCheck

carlosgalvezp added a comment. In D139919#3991250 , @MaskRay wrote: > In D139919#3991242 , @carlosgalvezp > wrote: > >> AFAIK it's preferred to use the LLVM types instead of the Standard types: >> >>> When both

[PATCH] D139919: use std::optional in ClangTidyCheck

carlosgalvezp added a comment. In D139919#3991247 , @addr2line wrote: > Sorry, I just saw a lot of changes in the llvm repo that change > llvm::Optional to std::optional, like this one >

[PATCH] D139919: use std::optional in ClangTidyCheck

carlosgalvezp requested changes to this revision. carlosgalvezp added a comment. This revision now requires changes to proceed. AFAIK it's preferred to use the LLVM types instead of the Standard types: > When both C++ and the LLVM support libraries provide similar functionality, > and there

[PATCH] D139113: [clang-tidy] Fix a couple additional cases in misc-use-anonymous-namespace

This revision was automatically updated to reflect the committed changes. Closed by commit rG35d9f873e3f2: [clang-tidy] Fix a couple additional cases in misc-use-anonymous-namespace only (authored by carlosgalvezp). Changed prior to commit: https://reviews.llvm.org/D139113?vs=481257=482119#toc

[PATCH] D139113: [clang-tidy] Fix a couple additional cases in misc-use-anonymous-namespace

carlosgalvezp added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/misc/use-anonymous-namespace.cpp:46-48 +// OK +static const int v8{123}; +static constexpr int v9{123}; vingeldal wrote: > Is it really the best behavior to allow these?

[PATCH] D139113: [clang-tidy] Fix a couple additional cases in misc-use-anonymous-namespace

carlosgalvezp added a comment. Friendly ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139113/new/ https://reviews.llvm.org/D139113 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D139113: [clang-tidy] Fix a couple additional cases in misc-use-anonymous-namespace

carlosgalvezp updated this revision to Diff 481257. carlosgalvezp added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139113/new/ https://reviews.llvm.org/D139113 Files:

[PATCH] D139197: [clang-tidy] Do not warn about redundant static in misc-use-anonymous-namespace

This revision was automatically updated to reflect the committed changes. Closed by commit rG7fd838791716: [clang-tidy] Do not warn about redundant static in misc-use-anonymous-namespace (authored by carlosgalvezp). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D139197: [clang-tidy] Do not warn about redundant static in misc-use-anonymous-namespace

carlosgalvezp added a comment. PS thanks a lot for the review! :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139197/new/ https://reviews.llvm.org/D139197 ___ cfe-commits mailing list

[PATCH] D139197: [clang-tidy] Do not warn about redundant static in misc-use-anonymous-namespace

carlosgalvezp added a comment. In D139197#3978488 , @aaron.ballman wrote: > LGTM, though please add a release note describing the change The original check is just a few days old. It didn't exist in the previous release. Should I still add a comment

[PATCH] D139197: [clang-tidy] Do not warn about redundant static in misc-use-anonymous-namespace

carlosgalvezp added a comment. @njames93 @JonasToth @LegalizeAdulthood Do you have any comments that I should address? If not, could you approve the patch? To recap: this check is a few days old (I added it myself). I realized there's functionality that shouldn't be here so I'm just removing

[PATCH] D139197: [clang-tidy] Do not warn about redundant static in misc-use-anonymous-namespace

carlosgalvezp added a comment. In D139197#3978174 , @lebedev.ri wrote: > In D139197#3977370 , @carlosgalvezp > wrote: > >> @lebedev.ri If you are happy with the patch, would you mind approving it? Or >> do you

[PATCH] D139197: [clang-tidy] Do not warn about redundant static in misc-use-anonymous-namespace

carlosgalvezp added a comment. @lebedev.ri If you are happy with the patch, would you mind approving it? Or do you have additional comments that should be addressed? The patch does not need to be approved by the Code Owner. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D139197: [clang-tidy] Do not warn about redundant static in misc-use-anonymous-namespace

carlosgalvezp updated this revision to Diff 479656. carlosgalvezp added a comment. Simplify anonymous namespace matcher. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139197/new/ https://reviews.llvm.org/D139197 Files:

[PATCH] D139197: [clang-tidy] Do not warn about redundant static in misc-use-anonymous-namespace

carlosgalvezp updated this revision to Diff 479651. carlosgalvezp edited the summary of this revision. carlosgalvezp added a comment. Update commit message. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139197/new/ https://reviews.llvm.org/D139197

[PATCH] D139197: Do not warn about redundant static in misc-use-anonymous-namespace

carlosgalvezp added a comment. Ah, then it has the exact same functionality! My idea for readability-redundant-static was to warn about: - static in anonymous namespace - static const/constexpr at global scope (since const gives implicit internal linkage in C++). I will see what I can do

[PATCH] D139197: Do not warn about redundant static in misc-use-anonymous-namespace

carlosgalvezp added a comment. My plan was to apply this removal in the same patch where I add the new check, but I know I'd get comments like "please do the removal in a separate patch". So that's what I'm doing :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D139197: Do not warn about redundant static in misc-use-anonymous-namespace

carlosgalvezp added a comment. In D139197#3966549 , @lebedev.ri wrote: > This patch should not land before that one does. The original code is 1 day old. Do we really need to be so strict? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D139197: Do not warn about redundant static in misc-use-anonymous-namespace

carlosgalvezp updated this revision to Diff 479616. carlosgalvezp added a comment. Reorder matchers to reduce diff. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139197/new/ https://reviews.llvm.org/D139197 Files:

[PATCH] D139197: Do not warn about redundant static in misc-use-anonymous-namespace

carlosgalvezp created this revision. Herald added a reviewer: njames93. Herald added a project: All. carlosgalvezp requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. I will create a new check "readability-redundant-static" where

[PATCH] D139113: Fix a couple additional cases in misc-use-anonymous-namespace

carlosgalvezp updated this revision to Diff 479409. carlosgalvezp retitled this revision from "Fix a couple additional cases in misc-use-anonymous-namespace only" to "Fix a couple additional cases in misc-use-anonymous-namespace". carlosgalvezp edited the summary of this revision. carlosgalvezp

[PATCH] D139113: Fix a couple additional cases in misc-use-anonymous-namespace only

carlosgalvezp marked an inline comment as done. carlosgalvezp added a comment. > The LLVM coding style says to prefer static over anonymous namespaces. Yes, but this is not an LLVM check, it's a `misc` check. Other project are allowed to have other guidelines. Repository: rG LLVM Github

[PATCH] D139113: Fix a couple additional cases in misc-use-anonymous-namespace only

carlosgalvezp updated this revision to Diff 479403. carlosgalvezp added a comment. Fix typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139113/new/ https://reviews.llvm.org/D139113 Files:

[PATCH] D139113: Fix a couple additional cases in misc-use-anonymous-namespace only

carlosgalvezp added a comment. I'm happy with my changes now, ready for a new round of review! :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139113/new/ https://reviews.llvm.org/D139113 ___

[PATCH] D139113: Fix a couple additional cases in misc-use-anonymous-namespace only

carlosgalvezp updated this revision to Diff 479397. carlosgalvezp added a comment. Remove accidentally added newline. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139113/new/ https://reviews.llvm.org/D139113 Files:

[PATCH] D139113: Fix a couple additional cases in misc-use-anonymous-namespace only

carlosgalvezp updated this revision to Diff 479394. carlosgalvezp added a comment. Document ignored cases in the documentation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139113/new/ https://reviews.llvm.org/D139113 Files:

[PATCH] D139113: Fix a couple additional cases in misc-use-anonymous-namespace only

carlosgalvezp updated this revision to Diff 479391. carlosgalvezp marked an inline comment as done. carlosgalvezp added a comment. Add options for users to define their own header file extensions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D139113: Fix a couple additional cases in misc-use-anonymous-namespace only

carlosgalvezp marked 2 inline comments as done. carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.cpp:44 + const SourceManager = MatchedDecl->getASTContext().getSourceManager(); + if

[PATCH] D139113: Fix a couple additional cases in misc-use-anonymous-namespace only

carlosgalvezp updated this revision to Diff 479383. carlosgalvezp added a comment. Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139113/new/ https://reviews.llvm.org/D139113 Files:

[PATCH] D139113: Fix a couple additional cases in misc-use-anonymous-namespace only

carlosgalvezp updated this revision to Diff 479284. carlosgalvezp added a comment. Fix naming convention Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139113/new/ https://reviews.llvm.org/D139113 Files:

[PATCH] D139113: Fix a couple additional cases in misc-use-anonymous-namespace only

carlosgalvezp created this revision. Herald added a reviewer: njames93. Herald added a project: All. carlosgalvezp requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. - Do not analyze header files, since we don't want to promote

[PATCH] D128372: [Clang-Tidy] Empty Check

carlosgalvezp added a comment. In D128372#3957982 , @cjdb wrote: > @njames93 if you don't have any further concerns, I am going to merge this on > Friday afternoon, as it will have been four months since there has been a > maintainer's input. @cjdb I

[PATCH] D137340: [clang-tidy] Add misc-use-anonymous-namespace check

This revision was automatically updated to reflect the committed changes. carlosgalvezp marked an inline comment as done. Closed by commit rG65d6d67fc9a9: [clang-tidy] Add misc-use-anonymous-namespace check (authored by carlosgalvezp). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D137340: [clang-tidy] Add misc-use-anonymous-namespace check

carlosgalvezp marked an inline comment as done. carlosgalvezp added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/misc/use-anonymous-namespace.rst:7-9 +that could instead be moved into an anonymous namespace. It also detects +instances moved to an

[PATCH] D137340: [clang-tidy] Add misc-use-anonymous-namespace check

carlosgalvezp updated this revision to Diff 478603. carlosgalvezp added a comment. Replace "the" anonymous namespace with "an" anonymous namespace Rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137340/new/ https://reviews.llvm.org/D137340

[PATCH] D137205: [clang-tidy] Add performance-unnecessary-copy-on-last-use check

carlosgalvezp added a comment. > we can't rehash the discussion here because it's too big Indeed, it's a major task to undertake so I don't mean to hijack this thread with that :) Just wanted to point out clang-format supports it in case it's of interest (it's a fairly new addition that not

[PATCH] D137205: [clang-tidy] Add performance-unnecessary-copy-on-last-use check

carlosgalvezp added a comment. > by design clang-format only performs white-space changes clang-format does support east/west const enforcement with the `QualifierAlignment` option. From experience, I strongly encourage repo owners to enable it repo-wide to avoid these kinds of discussions.

[PATCH] D137340: [clang-tidy] Add misc-use-anonymous-namespace check

carlosgalvezp added a comment. Another week, another ping @njames93 @aaron.ballman I have addressed all comments since 3 weeks ago, and have not received any objections. I intend to land this patch next week (1st December) if I don't receive any further feedback. Repository: rG LLVM Github

[PATCH] D137340: [clang-tidy] Add misc-use-anonymous-namespace check

carlosgalvezp added a comment. @njames93 Ping again. Alternatively, could someone else review? @aaron.ballman maybe? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137340/new/ https://reviews.llvm.org/D137340

[PATCH] D137340: [clang-tidy] Add misc-use-anonymous-namespace check

carlosgalvezp added a comment. @njames93 Friendly ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137340/new/ https://reviews.llvm.org/D137340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D137340: [clang-tidy] Add misc-use-anonymous-namespace check

carlosgalvezp updated this revision to Diff 473494. carlosgalvezp added a comment. Simplify code for printing the type of the declaration. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137340/new/ https://reviews.llvm.org/D137340 Files:

[PATCH] D137340: [clang-tidy] Add misc-use-anonymous-namespace check

carlosgalvezp updated this revision to Diff 473214. carlosgalvezp added a comment. Fix typo in binding names Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137340/new/ https://reviews.llvm.org/D137340 Files:

[PATCH] D137340: [clang-tidy] Add misc-use-anonymous-namespace check

carlosgalvezp updated this revision to Diff 473211. carlosgalvezp added a comment. Remove some code duplication Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137340/new/ https://reviews.llvm.org/D137340 Files:

[PATCH] D137340: [clang-tidy] Add misc-use-anonymous-namespace check

carlosgalvezp updated this revision to Diff 473159. carlosgalvezp retitled this revision from "[clang-tidy] Add modernize-use-anonymous-namespace check" to "[clang-tidy] Add misc-use-anonymous-namespace check". carlosgalvezp added a comment. Update commit message Repository: rG LLVM Github

[PATCH] D137340: [clang-tidy] Add modernize-use-anonymous-namespace check

carlosgalvezp updated this revision to Diff 473156. carlosgalvezp added a comment. Move to misc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137340/new/ https://reviews.llvm.org/D137340 Files: clang-tools-extra/clang-tidy/misc/CMakeLists.txt

[PATCH] D137340: [clang-tidy] Add modernize-use-anonymous-namespace check

carlosgalvezp added a comment. In D137340#3906250 , @njames93 wrote: > I'm not entirely sure why this belongs in the modernize module given > anonymous namespaces have been in c++ forever, maybe its more of a misc > check? Also the modernize checks are

[PATCH] D137340: [clang-tidy] Add modernize-use-anonymous-namespace check

carlosgalvezp updated this revision to Diff 472995. carlosgalvezp added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137340/new/ https://reviews.llvm.org/D137340 Files:

[PATCH] D137340: [clang-tidy] Add modernize-use-anonymous-namespace check

carlosgalvezp updated this revision to Diff 472948. carlosgalvezp added a comment. Add reference to undeprecation of static. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137340/new/ https://reviews.llvm.org/D137340 Files:

[PATCH] D137340: [clang-tidy] Add modernize-use-anonymous-namespace check

carlosgalvezp updated this revision to Diff 472946. carlosgalvezp added a comment. Fix release notes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137340/new/ https://reviews.llvm.org/D137340 Files:

[PATCH] D137340: [clang-tidy] Add modernize-use-anonymous-namespace check

carlosgalvezp created this revision. Herald added a subscriber: xazax.hun. Herald added a project: All. carlosgalvezp requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo

[PATCH] D132461: [clang-tidy] Add cppcoreguidelines-avoid-do-while check

This revision was automatically updated to reflect the committed changes. Closed by commit rG1ae33bf42680: [clang-tidy] Add cppcoreguidelines-avoid-do-while check (authored by carlosgalvezp). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132461/new/

[PATCH] D132461: [clang-tidy] Add cppcoreguidelines-avoid-do-while check

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

[PATCH] D132461: [clang-tidy] Add cppcoreguidelines-avoid-do-while check

carlosgalvezp added a comment. In D132461#3793634 , @njames93 wrote: > In D132461#3773792 , @carlosgalvezp > wrote: > >> @njames93 Friendly ping. The patch has addressed all comments and remained >> unchanged

[PATCH] D132461: [clang-tidy] Add cppcoreguidelines-avoid-do-while check

carlosgalvezp updated this revision to Diff 460419. carlosgalvezp added a comment. Adjust warning message for consistency. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132461/new/ https://reviews.llvm.org/D132461 Files:

[PATCH] D132461: [clang-tidy] Add cppcoreguidelines-avoid-do-while check

carlosgalvezp added a comment. @njames93 Friendly ping. The patch has addressed all comments and remained unchanged for 2 weeks. I would like to land it latest next week. If you are happy with the patch, please accept the review. Alternatively, please let me know if you intend to continue

[PATCH] D132461: [clang-tidy] Add cppcoreguidelines-avoid-do-while check

carlosgalvezp added a comment. @njames93 Is there anything else I should address? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132461/new/ https://reviews.llvm.org/D132461 ___ cfe-commits mailing list

[PATCH] D132461: [clang-tidy] Add cppcoreguidelines-avoid-do-while check

carlosgalvezp updated this revision to Diff 455804. carlosgalvezp added a comment. Rename option to IgnoreMacros and update logic and tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132461/new/ https://reviews.llvm.org/D132461 Files:

[PATCH] D132461: [clang-tidy] Add cppcoreguidelines-avoid-do-while check

carlosgalvezp added a comment. In D132461#3748818 , @carlosgalvezp wrote: > In D132461#3748812 , @njames93 > wrote: > >> The AllowWhileFlase option seems the wrong way to go about silencing do >> while(false)

[PATCH] D132461: [clang-tidy] Add cppcoreguidelines-avoid-do-while check

carlosgalvezp added a comment. In D132461#3748812 , @njames93 wrote: > The AllowWhileFlase option seems the wrong way to go about silencing do > while(false) macros. Would it not make more sense to just ignore macros, or > if you want more specificty

[PATCH] D132461: [clang-tidy] Add cppcoreguidelines-avoid-do-while check

carlosgalvezp updated this revision to Diff 455545. carlosgalvezp added a comment. Add a couple more tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132461/new/ https://reviews.llvm.org/D132461 Files:

[PATCH] D132461: [clang-tidy] Add cppcoreguidelines-avoid-do-while check

carlosgalvezp added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-do-while.rst:21 + +The check implements +`rule ES.75 of C++ Core Guidelines

[PATCH] D132461: [clang-tidy] Add cppcoreguidelines-avoid-do-while check

carlosgalvezp added a comment. PS: @aaron.ballman I didn't add you as reviewer since you have expressed in the past that you'd prefer not to review C++ Core Guidelines patches. Let me know if that has changed :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D132461: [clang-tidy] Add cppcoreguidelines-avoid-do-while check

carlosgalvezp added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-do-while.rst:21 + +The check implements +`rule ES.75 of C++ Core Guidelines

[PATCH] D132461: [clang-tidy] Add cppcoreguidelines-avoid-do-while check

carlosgalvezp updated this revision to Diff 454841. carlosgalvezp marked 2 inline comments as done. carlosgalvezp added a comment. Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132461/new/ https://reviews.llvm.org/D132461 Files:

[PATCH] D132461: [clang-tidy] Add cppcoreguidelines-avoid-do-while check

carlosgalvezp updated this revision to Diff 454793. carlosgalvezp added a comment. Fix typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132461/new/ https://reviews.llvm.org/D132461 Files:

[PATCH] D132461: [clang-tidy] Add cppcoreguidelines-avoid-do-while check

carlosgalvezp updated this revision to Diff 454789. carlosgalvezp added a comment. Add check to list of checks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132461/new/ https://reviews.llvm.org/D132461 Files:

[PATCH] D132461: [clang-tidy] Add cppcoreguidelines-avoid-do-while check

carlosgalvezp updated this revision to Diff 454787. carlosgalvezp added a comment. Document default value Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132461/new/ https://reviews.llvm.org/D132461 Files:

[PATCH] D132461: [clang-tidy] Add cppcoreguidelines-avoid-do-while check

carlosgalvezp updated this revision to Diff 454786. carlosgalvezp added a comment. Fix typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132461/new/ https://reviews.llvm.org/D132461 Files:

[PATCH] D132461: [clang-tidy] Add cppcoreguidelines-avoid-do-while check

carlosgalvezp created this revision. Herald added subscribers: shchenz, kbarton, xazax.hun, mgorny, nemanjai. Herald added a project: All. carlosgalvezp requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Implements rule ES.75 of

<    1   2   3   4   5   6   7   8   9   >