[PATCH] D150226: [Clang] Remove ability to downgrade warning on the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

carlosgalvezp added a comment. > I still think even if we can subset this, for whatever we're going to turn > into a hard error, it should be a warning-as-error in system headers first > for at least a release. (so perhaps the transition should look like: null (no > diagnostic) -> warning ->

[PATCH] D150226: [Clang] Remove ability to downgrade warning on the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

carlosgalvezp added a comment. @shafik @aaron.ballman @dblaikie Hello! Just wanted to check if there's any blockers for merging this patch? We are now on Clang 18, i.e. 2 releases after the warning was introduced, so IMO I believe it's a good time to turn it into a hard error and test it in

[PATCH] D159436: [clang-tidy] Add support for optional parameters in config.

carlosgalvezp added inline comments. Comment at: clang-tools-extra/test/clang-tidy/infrastructure/optional-parameter.cpp:18 +// RUN: -config='{CheckOptions: { \ +// RUN: bugprone-easily-swappable-parameters.MinimumLength: "false", \ +// RUN: }}' -- What

[PATCH] D159436: [clang-tidy] Add support for optional parameters in config.

carlosgalvezp added a comment. Thanks for the patch! I must admit I don't fully understand what problem it solves, i.e. parameters are already optional today (one just simply doesn't specify them in the config file). Why would we want to explicitly spell out parameters with some default value

[PATCH] D158486: [clang-tidy] Ignore used special-members in modernize-use-equals-delete

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. LGTM, thanks for fixing! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158486/new/ https://reviews.llvm.org/D158486

[PATCH] D56644: [clang-tidy] readability-container-size-empty handle std::string length()

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

[PATCH] D158929: [clang-tidy] Add exit code support to clang-tidy-diff.py

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

[PATCH] D157374: [clang-tidy] Ignore decltype in misc-redundant-expression

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157374/new/ https://reviews.llvm.org/D157374

[PATCH] D157326: [clang-tidy] Fix handling of out-of-line functions in readability-static-accessed-through-instance

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

[PATCH] D157239: [clang-tidy] Implement bugprone-incorrect-enable-if

carlosgalvezp added a comment. Do you have plans to also detect the bugprone scenario described in the `Notes` here? https://en.cppreference.com/w/cpp/types/enable_if No need to have it in this patch, but would be good to keep it in mind if we want to add it in the future (preferably) or

[PATCH] D157185: [clang-tidy] Fix false-positives in performanc-noexcept-swap

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/performance/NoexceptSwapCheck.cpp:31 + // Match function with 2 arguments, both are non-const references to same type + // and return void void swap(Type&, Type&) + auto FunctionMatcher = allOf(

[PATCH] D157181: [clang-tidy] Re-add cppcoreguidelines-macro-to-enum alias

carlosgalvezp added a comment. In D157181#4563170 , @PiotrZSL wrote: > In D157181#4563147 , @carlosgalvezp > wrote: > >> Strange, it seems the alias was never fully added? >> https://reviews.llvm.org/D117522 > >

[PATCH] D157190: [clang-tidy] Fixed false-negative in readability-identifier-naming

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. LGTM, minor question that can be fixed post-review unless you want to discuss further! Comment at:

[PATCH] D157185: [clang-tidy] Fix false-positives in performanc-noexcept-swap

carlosgalvezp accepted this revision. carlosgalvezp added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clang-tidy/performance/NoexceptSwapCheck.cpp:22 Finder->addMatcher( - functionDecl(unless(isDeleted()),

[PATCH] D157180: [clang-tidy] Exclude class/struct scope variables from cppcoreguidelines-avoid-non-const-global-variables

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

[PATCH] D157178: [clang-tidy] Fix inline namespaces in llvm-namespace-comment

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157178/new/ https://reviews.llvm.org/D157178

[PATCH] D145138: [clang-tidy] Implement FixIts for C arrays

carlosgalvezp added a comment. In D145138#4561799 , @PiotrZSL wrote: > Test are failing, and I do not think that converting c-strings into > std::array is a good idea +1, they should typically be `char const*` instead. Repository: rG LLVM

[PATCH] D157181: [clang-tidy] Re-add cppcoreguidelines-macro-to-enum alias

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. Strange, it seems the alias was never fully added? https://reviews.llvm.org/D117522 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D157182: [clang-tidy][NFC] Update documentation for hicpp-avoid-goto

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

[PATCH] D156624: [clang-tidy] Access checks not done classes derived of std::array

carlosgalvezp added a comment. What about the use case of privately inheriting `std::array`, and overriding the `operator[]` there with bounds check? Would it be possible to check only _public_ inheritance? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156624/new/

[PATCH] D156608: [clang-tidy] Do not warn on macros starting with underscore and lowercase letter in bugprone-reserved-identifier

This revision was automatically updated to reflect the committed changes. Closed by commit rG2992d084774f: [clang-tidy] Do not warn on macros starting with underscore and lowercase… (authored by carlosgalvezp). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D156608: [clang-tidy] Do not warn on macros starting with underscore and lowercase letter in bugprone-reserved-identifier

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

[PATCH] D156608: [clang-tidy] Do not warn on macros starting with underscore and lowercase letter in bugprone-reserved-identifier

carlosgalvezp created this revision. Herald added subscribers: PiotrZSL, 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 #64130

[PATCH] D156452: [clang-tidy] Sort options in --dump-config

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. LGTM, thanks for fixing! Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp:90 +std::vector> SortedOptions; +

[PATCH] D156452: [clang-tidy] Sort options in --dump-config

carlosgalvezp added a comment. Looks great, small comments! Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp:89 if (IO.outputting()) { +std::vector> SortedOptions; +SortedOptions.reserve(Options.size()); Maybe add a one-line comment

[PATCH] D156303: [clang-tidy] Remove AnalyzeTemporaryDestructors configuration option

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb7c6b39651b3: [clang-tidy] Remove AnalyzeTemporaryDestructors configuration option (authored by carlosgalvezp). Repository: rG LLVM Github

[PATCH] D156303: [clang-tidy] Remove AnalyzeTemporaryDestructors configuration option

carlosgalvezp created this revision. Herald added subscribers: PiotrZSL, 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. Since it was

[PATCH] D156161: [clang-tidy] Add --enable-module-headers-parsing option

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. LGTM! Feel free to add the comment about the implications of using the flag in the docs. Comment at: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp:270

[PATCH] D156161: [clang-tidy] Add --enable-module-headers-parsing option

carlosgalvezp added a comment. Do we want to keep the `experimental` word in the flag? Comment at: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp:266 +static cl::opt EnableModuleHeadersParsing("enable-module-headers-parsing", +

[PATCH] D156024: [clang-tidy] Add --experimental-disable-module-headers-parsing option

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h:202 + bool canEnableModuleHeadersParsing() const { +return !DisableModuleHeadersParsing; Add docs? Comment at:

[PATCH] D156161: [clang-tidy] Add --experimental-enable-module-headers-parsing option

carlosgalvezp added a comment. Please squash into previous patch, I see no reason to make them into separate commits. The first one is missing Release Notes, for example. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156161/new/

[PATCH] D156056: [clang-tidy] Initialize DiagnosticEngine in ExpandModularHeaders

carlosgalvezp accepted this revision. carlosgalvezp added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp:75 + Diags(new DiagnosticIDs, +new

[PATCH] D155890: [clang-tidy] Add folly::Optional to unchecked-optional-access

carlosgalvezp added a comment. The review is marked as accepted, should we land it? Let me know if you need help with that :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155890/new/ https://reviews.llvm.org/D155890

[PATCH] D156056: [clang-tidy] Initialize DiagnosticEngine in ExpandModularHeaders

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp:75 + Diags(new DiagnosticIDs, +new DiagnosticOptions(Compiler.getDiagnosticOpts()), new

[PATCH] D156056: [clang-tidy] Initialize DiagnosticEngine in ExpandModularHeaders

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp:75 + Diags(new DiagnosticIDs, +new DiagnosticOptions(Compiler.getDiagnosticOpts()), new

[PATCH] D156056: [clang-tidy] Initialize DiagnosticEngine in ExpandModularHeaders

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp:75 + Diags(new DiagnosticIDs, +new DiagnosticOptions(Compiler.getDiagnosticOpts()), new

[PATCH] D156056: [clang-tidy] Initialize DiagnosticEngine in ExpandModularHeaders

carlosgalvezp added a comment. A thought came to mind - since we are doing workarounds anyway, would it be easier to ask people to simply add `-clang-diagnostic*` to the `Checks` in their config file? It's fair to assume they will get those warnings when compiling the code. I feel the more

[PATCH] D156056: [clang-tidy] Initialize DiagnosticEngine in ExpandModularHeaders

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp:83 Diags.setSourceManager(); + ProcessWarningOptions(Diags, Compiler.getDiagnosticOpts()); A bit unclear to me why we should add this line

[PATCH] D155890: [clang-tidy] Add folly::Optional to unchecked-optional-access

carlosgalvezp added a comment. I have opened a refactoring ticket here: https://github.com/llvm/llvm-project/issues/64037 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155890/new/ https://reviews.llvm.org/D155890

[PATCH] D156031: [clang-tidy] Ignore implcit casts in cppcoreguidelines-owning-memory

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

[PATCH] D155890: [clang-tidy] Add folly::Optional to unchecked-optional-access

carlosgalvezp added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:62 +return N != nullptr && (isTopLevelNamespaceWithName(*N, "base") || +isTopLevelNamespaceWithName(*N, "folly")); }

[PATCH] D155890: [clang-tidy] Add folly::Optional to unchecked-optional-access

carlosgalvezp resigned from this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. > I think it'd be good to add reviewers there I realize the CodeOwners for Analysis are already in the list of reviewers, I won't interfere then :) Repository: rG LLVM

[PATCH] D155890: [clang-tidy] Add folly::Optional to unchecked-optional-access

carlosgalvezp added a comment. In D155890#4523262 , @gribozavr2 wrote: > In D155890#4523243 , @adukeman > wrote: > >> In D155890#4522266 , @ymandel >> wrote: >> >>> In

[PATCH] D155890: [clang-tidy] Add folly::Optional to unchecked-optional-access

carlosgalvezp requested changes to this revision. carlosgalvezp added a comment. This revision now requires changes to proceed. This should be a configuration option, we should not hardcore project-specific things in the source code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D155625: [clang-tidy] Warn only for copyable/movable classes in cppcoreguidelines-avoid-const-or-ref-members

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb70e6e968192: [clang-tidy] Warn only for copyable/movable classes in cppcoreguidelines-avoid… (authored by carlosgalvezp). Repository: rG LLVM

[PATCH] D155625: [clang-tidy] Warn only for copyable/movable classes in cppcoreguidelines-avoid-const-or-ref-members

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.cpp:91-102 + Finder->addMatcher( + fieldDecl(unless(isMemberOfLambda()), +hasDeclContext(cxxRecordDecl(isCopyableOrMovable())),

[PATCH] D155625: [clang-tidy] Warn only for copyable/movable classes in cppcoreguidelines-avoid-const-or-ref-members

carlosgalvezp updated this revision to Diff 541963. carlosgalvezp added a comment. Merge matchers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155625/new/ https://reviews.llvm.org/D155625 Files:

[PATCH] D155625: [clang-tidy] Warn only for copyable/movable classes in cppcoreguidelines-avoid-const-or-ref-members

carlosgalvezp added a comment. Using `hasSimpleCopyConstructor` and so on greatly simplifies the logic, great! Let me know if you are happy with it or I should go ahead and merge. Comment at:

[PATCH] D155625: [clang-tidy] Warn only for copyable/movable classes in cppcoreguidelines-avoid-const-or-ref-members

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

[PATCH] D155625: [clang-tidy] Warn only for copyable/movable classes in cppcoreguidelines-avoid-const-or-ref-members

carlosgalvezp added a comment. In D155625#4512123 , @PiotrZSL wrote: > LGTM, but I'm not sure if isCopyableOrMovable will always work correctly, for > a user defined special members it looks ok, but for some implicit ones I > worry it may not always

[PATCH] D147955: [clang-tidy] Extend CheckOptions to support grouping checks options

carlosgalvezp requested changes to this revision. carlosgalvezp added inline comments. This revision now requires changes to proceed. Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp:94-95 +// Nested option names need to be converted to null terminated strings

[PATCH] D153423: [clang-tidy] Allow explicit throwing in bugprone-exception-escape for special functions

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. LGTM, thanks for the fix! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153423/new/ https://reviews.llvm.org/D153423

[PATCH] D155625: [clang-tidy] Warn only for copyable/movable classes in cppcoreguidelines-avoid-const-or-ref-members

carlosgalvezp updated this revision to Diff 541684. carlosgalvezp added a comment. Split getInfo function into 2 functions, remove structured bindings. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155625/new/ https://reviews.llvm.org/D155625

[PATCH] D155625: [clang-tidy] Warn only for copyable/movable classes in cppcoreguidelines-avoid-const-or-ref-members

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

[PATCH] D155625: [clang-tidy] Warn only for copyable/movable classes in cppcoreguidelines-avoid-const-or-ref-members

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

[PATCH] D155625: [clang-tidy] Warn only for copyable/movable classes in cppcoreguidelines-avoid-const-or-ref-members

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

[PATCH] D155625: [clang-tidy] Warn only for copyable/movable classes in cppcoreguidelines-avoid-const-or-ref-members

carlosgalvezp created this revision. Herald added subscribers: PiotrZSL, shchenz, kbarton, xazax.hun, nemanjai. 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:

[PATCH] D153458: [clang-tidy] Model noexcept more properly in bugprone-exception-escape

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153458/new/ https://reviews.llvm.org/D153458

[PATCH] D151495: [clang-tidy] Improve build-in type handling in bugprone-swapped-arguments

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. LGTM, thank you for the fix! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151495/new/ https://reviews.llvm.org/D151495

[PATCH] D153458: [clang-tidy] Model noexcept more properly in bugprone-exception-escape

carlosgalvezp added a comment. Looks good, great to see all these issues fixed! Have a couple small comments. Comment at: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp:319 +static bool cannotThrow(const FunctionDecl *Func) { + const auto *FunProto =

[PATCH] D150071: [clang-tidy] Fix bugprone-assert-side-effect to actually give warnings

carlosgalvezp added a comment. Thanks for the review! Fixed your comments before landing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150071/new/ https://reviews.llvm.org/D150071 ___ cfe-commits

[PATCH] D150071: [clang-tidy] Fix bugprone-assert-side-effect to actually give warnings

This revision was automatically updated to reflect the committed changes. carlosgalvezp marked 2 inline comments as done. Closed by commit rG0d6d8a853a6e: [clang-tidy] Fix bugprone-assert-side-effect to actually give warnings (authored by carlosgalvezp). Changed prior to commit:

[PATCH] D148995: [clang-tidy] Extract areStatementsIdentical

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. > Separating the changes into two patches would only prolong this process and > potentially delay the completion of this task. I understand the concern, but in practice it's

[PATCH] D150071: [clang-tidy] Fix bugprone-assert-side-effect to actually give warnings

carlosgalvezp created this revision. Herald added subscribers: PiotrZSL, 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. Some time ago a

[PATCH] D148458: [clang-tidy][NFC] Split bugprone-exception-escape tests

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. LGTM! > A lot of our test files uses macros to differentiate between specific C++ > standards, why not do that here too? It's not that easy, because the #ifdef lines do not

[PATCH] D149899: [clang-tidy] Support SystemHeaders in .clang-tidy

This revision was automatically updated to reflect the committed changes. Closed by commit rG26f476286fbc: [clang-tidy] Support SystemHeaders in .clang-tidy (authored by carlosgalvezp). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149899/new/

[PATCH] D149899: [clang-tidy] Support SystemHeaders in .clang-tidy

carlosgalvezp updated this revision to Diff 520152. carlosgalvezp added a comment. Revert unwanted change to UseColor Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149899/new/ https://reviews.llvm.org/D149899 Files:

[PATCH] D149899: [clang-tidy] Support SystemHeaders in .clang-tidy

carlosgalvezp updated this revision to Diff 520151. carlosgalvezp added a comment. Herald added a subscriber: arphaman. - Add test where both command line and config settings are used. - Document that the command-line option overrides the config option, for consistency with other command-line

[PATCH] D149899: [clang-tidy] Support SystemHeaders in .clang-tidy

carlosgalvezp added inline comments. Comment at: clang-tools-extra/test/clang-tidy/infrastructure/system-headers.cpp:11 +// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -config='SystemHeaders: false' %s -- -isystem %S/Inputs/system-headers 2>&1

[PATCH] D149899: [clang-tidy] Support SystemHeaders in .clang-tidy

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

[PATCH] D149899: [clang-tidy] Support SystemHeaders in .clang-tidy

carlosgalvezp created this revision. Herald added subscribers: PiotrZSL, 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. A previous patch

[PATCH] D131319: [clang-tidy] Update llvm-prefer-isa-or-dyn-cast-in-conditionals with new syntax

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.h:61 +private: + mutable std::optional HasIsPresent; }; njames93 wrote: > PiotrZSL wrote: > > no need for mutable, its used only form

[PATCH] D148995: [clang-tidy] Extract areStatementsIdentical

carlosgalvezp added a comment. Thanks for the refactoring, great to remove some code duplication! As a reviewer it's not possible for me to confidently review the functional changes to `areStatementsIdentical`, since they don't show side by side as they are in different files. Please perform

[PATCH] D148110: [clang-tidy] Ctor arguments are sequenced if ctor call is written as list-initialization.

carlosgalvezp added a comment. The commit message doesn't really tell me "what" this commit is fixing, it only points to a section of the Standard. It talks about "a false positive" but it doesn't tell what this FP is about. Could you write a little bit more about what the problem is?

[PATCH] D148462: [clang-tidy] Ignore declarations in bugprone-exception-escape

carlosgalvezp added a comment. I would agree that the fact that a function throws or not can only be found out when analyzing the function definition (it's impossible to know from the declaration). There's also a duplicate diagnostic that I don't see much value on, so I would agree with this

[PATCH] D148458: [clang-tidy][NFC] Split bugprone-exception-escape tests

carlosgalvezp added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-throw.cpp:4 +void throwing_throw_nothing() throw() { +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function

[PATCH] D148793: [WIP][clang-tidy] Implement an include-cleaner check.

carlosgalvezp added a comment. I believe this should be discussed in an RFC. We already have the standalone `include-cleaner` tool, why is that not sufficient? Can it be extended instead? There's also the include-what-you-use tool

[PATCH] D148697: [clang-tidy] Handle more cases of functions which should always be noexcept

carlosgalvezp added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/noexcept-special-functions.rst:10 +`performance-noexcept-special-functions <../performance/noexcept-special-functions.html>`_ +for more information.

[PATCH] D148697: [clang-tidy] Handle more cases of functions which should always be noexcept

carlosgalvezp added a comment. First of all thank you for the contribution! I had just a quick look so here's some very preliminary comments, will have more time to deeply review during the weekend: - Unfortunately we cannot simply rename a check like this, since it breaks backwards

[PATCH] D148460: [clang-tidy] Add alias cppcoreguidelines-use-default-member-init

This revision was automatically updated to reflect the committed changes. Closed by commit rGb507bda45523: [clang-tidy] Add alias cppcoreguidelines-use-default-member-init (authored by carlosgalvezp). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D144748: [clang-tidy] Add bugprone-empty-catch check

carlosgalvezp added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone/empty-catch.rst:103 +{ +try +{ Use 2 spaces indentation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D144748: [clang-tidy] Add bugprone-empty-catch check

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.cpp:100 +void EmptyCatchCheck::check(const MatchFinder::MatchResult ) { + const auto *MatchedCatchStmt = Result.Nodes.getNodeAs("catch"); + Assert that it's

[PATCH] D148444: [clang-tidy] Prevent `llvmlibc-inline-function-decl` triggering on lambdas

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/llvmlibc/InlineFunctionDeclCheck.cpp:43 + // Consider only functions with an external and visible declaration. + if (const auto *MethodDecl = dyn_cast(FuncDecl)) +if

[PATCH] D148458: [clang-tidy][NFC] Split bugprone-exception-escape tests

carlosgalvezp added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp:267 const derived *p = -throw p; } catch(base *) { I run into this often as well. If you don't want to get push back during

[PATCH] D148460: [clang-tidy] Add alias cppcoreguidelines-use-default-member-init

carlosgalvezp updated this revision to Diff 513996. carlosgalvezp added a comment. Remove check from list of checks, since we do not add aliases there. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148460/new/ https://reviews.llvm.org/D148460

[PATCH] D148460: [clang-tidy] Add alias cppcoreguidelines-use-default-member-init

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

[PATCH] D148460: [clang-tidy] Add alias cppcoreguidelines-use-default-member-init

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

[PATCH] D148460: [clang-tidy] Add alias cppcoreguidelines-use-default-member-init

carlosgalvezp updated this revision to Diff 513992. carlosgalvezp added a comment. Revert code removal, document deprecation notice instead. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148460/new/ https://reviews.llvm.org/D148460 Files:

[PATCH] D148460: [clang-tidy] Add alias cppcoreguidelines-use-default-member-init

carlosgalvezp created this revision. Herald added subscribers: PiotrZSL, jeroen.dobbelaere, shchenz, kbarton, xazax.hun, nemanjai. Herald added a reviewer: njames93. Herald added a project: All. carlosgalvezp requested review of this revision. Herald added a project: clang-tools-extra. Herald

[PATCH] D147929: [clang-tidy] Fix handling of UseAssignment option in cppcoreguidelines-prefer-member-initializer

carlosgalvezp accepted this revision. carlosgalvezp added a subscriber: aaron.ballman. carlosgalvezp added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp:134 +

[PATCH] D145865: [clang-tidy] Multiple fixes to bugprone-exception-escape

carlosgalvezp added a comment. There's a lot of changes in this patch (at least 6 as per commit message) - please split into smaller patches fixing 1 problem at a time. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145865/new/

[PATCH] D144036: [clang-tidy] Add bugprone-non-zero-enum-to-bool-conversion check

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. LGTM, thanks! Would be good to fix the last nit before landing. Comment at: clang-tools-extra/clang-tidy/bugprone/NonZeroEnumToBoolConversionCheck.h:17 + +///

[PATCH] D98416: [clang-tidy] Fix cppcoreguidelines-narrowing-conversions false positive

carlosgalvezp added a comment. Took the liberty to land it for you, hope it's ok! :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98416/new/ https://reviews.llvm.org/D98416 ___ cfe-commits mailing list

[PATCH] D98416: [clang-tidy] Fix cppcoreguidelines-narrowing-conversions false positive

This revision was automatically updated to reflect the committed changes. Closed by commit rG4530c3bc4897: [clang-tidy] Fix cppcoreguidelines-narrowing-conversions false positive (authored by njames93, committed by carlosgalvezp). Changed prior to commit:

[PATCH] D148340: [clang-tidy] Apply cppcoreguidelines-avoid-capture-default-when-capturin-this only to by-value capture default

carlosgalvezp marked an inline comment as done. carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/MisleadingCaptureDefaultByValueCheck.h:1 +//===--- MisleadingCaptureDefaultByValueCheck.h - clang-tidy*- C++ +//-*-===//

[PATCH] D148424: [clang-tidy][NFC] Improve doc of cppcoreguidelines-misleading-capture-default-by-value

This revision was automatically updated to reflect the committed changes. Closed by commit rGfa3de2ed2964: [clang-tidy][NFC] Improve doc of cppcoreguidelines-misleading-capture-default… (authored by carlosgalvezp). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D148418: [clang-tidy] Fix typedefs handling in bugprone-dangling-handle

carlosgalvezp accepted this revision. carlosgalvezp added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone/dangling-handle.cpp:55-56 - operator basic_string_view() const noexcept; + typedef

[PATCH] D148424: [clang-tidy][NFC] Improve doc of cppcoreguidelines-misleading-capture-default-by-value

carlosgalvezp created this revision. Herald added subscribers: PiotrZSL, shchenz, kbarton, xazax.hun, nemanjai. 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:

[PATCH] D148340: [clang-tidy] Apply cppcoreguidelines-avoid-capture-default-when-capturin-this only to by-value capture default

This revision was automatically updated to reflect the committed changes. Closed by commit rGeedbe81b1c6d: [clang-tidy] Apply cppcoreguidelines-avoid-capture-default-when-capturin-this… (authored by carlosgalvezp). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D148418: [clang-tidy] Fix typedefs handling in bugprone-dangling-handle

carlosgalvezp added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone/dangling-handle.cpp:55-56 - operator basic_string_view() const noexcept; + typedef basic_string_view str_view; + operator str_view() const noexcept;

  1   2   3   4   5   6   7   8   9   >