[PATCH] D59812: [analyzer] PR41185: Fix regression where __builtin_* functions weren't recognized

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

[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:382 + if module == 'llvm' or module == 'clang': +namespace = module + '_checker' + else: hintonda wrote: > aaron.ballman wrote: > > I thought we were going with `llvm_ch

[PATCH] D60808: [analyzer] pr41335: NoStoreFuncVisitor: Fix crash when no-store event is in a body-farmed function.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG with a couple of nits. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:582 +if (!L.hasValidLocation()) { + // Do we need to suppress our report

[PATCH] D60857: [clang-tidy] Address post-commit comments

alexfh added inline comments. Comment at: clang-tools-extra/test/clang-tidy/alternative-fixes.cpp:1-7 +// RUN: clang-tidy -checks='-*,llvm-namespace-comment,clang-diagnostic-*' %s -- \ +// RUN: | FileCheck -implicit-check-not='{{warning|error|note}}:' %s + +// Verify clang-tidy

[PATCH] D60857: [clang-tidy] Address post-commit comments

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. There's one more nit. Otherwise good to go. Thanks! Comment at: clang/include/clang/Tooling/Core/Diagnostic.h:97 +/// Get the first fix to apply for this diagnostic. +/// Re

[PATCH] D60868: [clang-format] Fix an assertion failure

alexfh created this revision. alexfh added a reviewer: krasimir. Herald added a project: clang. Before this patch clang-format crashed when trying to issue a diagnostic about an unsupported BOM. See the bug report here: https://bugs.llvm.org/show_bug.cgi?id=26032 Repository: rG LLVM Github M

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

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG with a comment. Comment at: clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp:110-111 + Lexer::getSourceText(TextRange, Sources, getLangOpts()) +

[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/rename_check.py:271 +new_namespace = new_module + if old_module != new_module or new_namespace == 'llvm_check': check_implementation_files = glob.glob( Why `new_namespace == 'llvm_ch

[PATCH] D61106: [analyzer][UninitializedObjectChecker] PR41590: Regard _Atomic types as primitive

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG. Thanks for the fix! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61106/new/ https://reviews.llvm.org/D61106 ___

[PATCH] D61480: Added an AST matcher for declarations that are in the `std` namespace

alexfh added a comment. Please regenerate the HTML docs using clang/docs/tools/dump_ast_matchers.py. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61480/new/ https://reviews.llvm.org/D61480 ___ cfe-com

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

alexfh added a comment. Apart from NOLINT handling there's more logic in `ClangTidyDiagnosticConsumer::HandleDiagnostic`, which isn't properly transferred to `ClangTidyContext::diag` in this patch. The logic that is transferred seems to change the behavior w.r.t. notes that can "unmute" ignored

[PATCH] D61644: Documentation for bugprone-inaccurate-erase: added an example of a bug that this checker catches

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG. Thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61644/new/ https://reviews.llvm.org/D61644 _

[PATCH] D61739: check_clang_tidy.py now passes `-format-style=none` to clang_tidy

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

[PATCH] D52136: [clang-tidy] Add modernize-concat-nested-namespaces check

alexfh added inline comments. Comment at: clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:102-108 + if (childrenCount(ND.decls()) == 0) { +SourceRange Removal(Namespaces.front().Begin, Namespaces.front().RBrace); + +diag(Namespaces.front().Begin, + "Nested n

[PATCH] D48714: [clang-tidy] fix PR37913, templated exception factory diagnosed correctly

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG Comment at: test/clang-tidy/hicpp-exception-baseclass.cpp:191 +void templated_thrower() { throw T{}(); } +// CHECK-MESSAGES: [[@LINE-1]]:34: warning: throwing an exception

[PATCH] D52187: [clang-tidy] use CHECK-NOTES in bugprone-unused-return-value

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG with a comment. Comment at: test/clang-tidy/bugprone-unused-return-value.cpp:81 std::async(std::launch::async, increment, 42); - // CHECK-MESSAGES: [[@LINE-1]]:3: warn

[PATCH] D52186: [clang-tidy] use CHECK-NOTES in bugprone-forwarding-reference-overload

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52186 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llv

[PATCH] D52178: [clang-tidy] use CHECK-NOTES in tests for bugprone-argument-comment

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52178 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llv

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

alexfh added a comment. This is intended, IIUC. The syntax of the clang-tidy-diff.py mirrors the syntax of clang-tidy itself, and the `--` option is used in the same way as in clang-tidy - to denote the start of compiler arguments (and switch to the fixed compilation database). Do you have a us

[PATCH] D52334: [clang-tidy] Build it even without static analyzer

alexfh added a comment. > ! In https://reviews.llvm.org/D52334#1242955, @JonasToth wrote: > ... to me it makes sense to have clang-tidy without CSA. Yep, it seems reasonable. Comment at: test/CMakeLists.txt:69 -clang-tidy -) -endif() + clang-tidy + ) -

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

alexfh added inline comments. Comment at: clang-tidy/tool/clang-tidy-diff.py:123-130 if args.fix: command.append('-fix') if args.checks != '': command.append('-checks=' + quote + args.checks + quote) if args.quiet: command.append('-quiet') if args.build

[PATCH] D52691: [clang-tidy] NFC use CHECK-NOTES in tests for performance-move-constructor-init

alexfh added inline comments. Comment at: test/clang-tidy/performance-move-constructor-init.cpp:115-117 + // CHECK-NOTES: 7:1: note: FIX-IT applied suggested code changes + // CHECK-NOTES: 113:28: note: FIX-IT applied suggested code changes + // CHECK-NOTES: 113:29: note: FIX-

[PATCH] D52690: [clang-tidy] NFC use CHECK-NOTES in tests for misc-misplaced-const

alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: test/clang-tidy/misc-misplaced-const.c:18 + // CHECK-NOTES: :[[@LINE-1]]:12: warning: 'i3' declared with a const-qualified typedef type; results i

[PATCH] D52686: [clang-tidy] NFC use CHECK-NOTES in test for cppgoreguidelines-avoid-goto

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52686 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llv

[PATCH] D52684: [clang-tidy] NFC refactor lexer-utils slightly to be easier to use

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. So far the change doesn't look really helpful. Are you going to use the function from code where no `ASTContext` is available? Can you give an example? Repository: rCTE Clang Tool

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

alexfh added inline comments. Comment at: clang-tidy/tool/clang-tidy-diff.py:123-130 if args.fix: command.append('-fix') if args.checks != '': command.append('-checks=' + quote + args.checks + quote) if args.quiet: command.append('-quiet') if args.build

[PATCH] D52687: [clang-tidy] NFC use CHECK-NOTES in tests for cppcoreguidelines-owning-memory

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52687 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llv

[PATCH] D52690: [clang-tidy] NFC use CHECK-NOTES in tests for misc-misplaced-const

alexfh added inline comments. Comment at: test/clang-tidy/misc-misplaced-const.c:18 + // CHECK-NOTES: :[[@LINE-1]]:12: warning: 'i3' declared with a const-qualified typedef type; results in the type being 'int *const' instead of 'const int *' + // CHECK-NOTES: :[[@LINE-14]]:1

[PATCH] D36836: [clang-tidy] Implement sonarsource-function-cognitive-complexity check

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. In https://reviews.llvm.org/D36836#1249971, @JonasToth wrote: > Anything new here? Is there a LLVM foundation lawyer or something like that > we can ask? I didn't notice any substa

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: clang-tidy/tool/clang-tidy-diff.py:123-130 if args.fix: command.append('-fix') if args.checks != '': command.append('-checks=' + quo

[PATCH] D52880: [clang-tidy] fix PR39167, bugprone-exception-escape hangs-up

alexfh added a comment. Have you figured out why exactly does the check hang? Disabling it for -fno-exceptions may just hide a logical problem in the check. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52880 ___ cfe-commits maili

[PATCH] D52880: [clang-tidy] fix PR39167, bugprone-exception-escape hangs-up

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D52880#1255249, @alexfh wrote: > Have you figured out why exactly does the check hang? Disabling it for > -fno-exceptions may just hide a logical problem in the che

[PATCH] D52691: [clang-tidy] NFC use CHECK-NOTES in tests for performance-move-constructor-init

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG. Thanks Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52691 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://l

[PATCH] D52882: [clang-tidy] Added pointer types to clang-tidy readability-identifier-naming check.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52882 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llv

[PATCH] D52688: [clang-tidy] NFC use CHECK-NOTES in tests for fuchsia-default-arguments

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52688 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llv

[PATCH] D51332: [clang-tidy] Replace deprecated std::ios_base aliases

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG once other comments are addressed. https://reviews.llvm.org/D51332 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.o

[PATCH] D52684: [clang-tidy] NFC refactor lexer-utils slightly to be easier to use

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D52684#1250885, @JonasToth wrote: > This patch is related to https://reviews.llvm.org/D51949 > > To isolate variable declarations (split `int * p, v;` up) it is nece

[PATCH] D52971: [clang-tidy] Customize FileCheck prefix in check_clang-tidy.py to support multiple prefixes

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good with a comment. Comment at: test/clang-tidy/check_clang_tidy.py:52 parser.add_argument('temp_file_name') + parser.add_argument('-check-suffix', default=[], typ

[PATCH] D52989: [clang-tidy] Fix handling of parens around new expressions in make_ checks.

alexfh created this revision. alexfh added a reviewer: hokein. Herald added a subscriber: xazax.hun. Extra parentheses around a new expression result in incorrect code after applying fixes. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52989 Files: clang-tidy/modernize/MakeS

[PATCH] D57573: Disable tidy checks with too many hits

alexfh added a comment. I wonder whether a list of specific checks (without wildcards) would make more sense for llvm? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57573/new/ https://reviews.llvm.org/D57573 ___ cfe

[PATCH] D57655: clang-format with UseTab: Always sometimes doesn't insert the right amount of tabs.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Seems reasonable. LG with a couple of nits. Please let me know if you need to commit this for you. Comment at: D:/Format/llvm/tools/clang/lib/Format/WhitespaceManager.cpp:68

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

alexfh created this revision. alexfh added reviewers: bkramer, klimek, rsmith. Herald added subscribers: jdoerfert, jsji, kbarton, nemanjai. Herald added a project: clang. When a clang tool runs on a translation unit that uses modular headers, no PPCallbacks will be invoked for the code in the mod

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

alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Herald added a subscriber: jdoerfert. Comment at: clang-tidy/modernize/UseOverrideCheck.cpp:32 +: ClangTidyCheck(Name, Context), + OverrideMacro(Option

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

alexfh added inline comments. Comment at: clang-tidy/tool/clang-tidy-diff.py:98 + parser.add_argument('-j', type=int, default=0, + help='number of tidy instances to be run in parallel.') The "clang-tidy runs are independent" assumption is

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

alexfh added inline comments. Herald added a project: clang. Comment at: clang-tidy/readability/ContainerSizeEmptyCheck.cpp:65 hasType(references(ValidContainer), -callee(cxxMethodDecl(hasName("size"))), WrongUse,

[PATCH] D57655: clang-format with UseTab: Always sometimes doesn't insert the right amount of tabs.

This revision was automatically updated to reflect the committed changes. Closed by commit rC354183: clang-format with UseTab: Always sometimes doesn't insert the right amount of… (authored by alexfh, committed by ). Changed prior to commit: https://reviews.llvm.org/D57655?vs=186629&id=187096#t

[PATCH] D57112: [ASTTypeTraits] OMPClause handling

alexfh added a comment. In D57112#1399516 , @lebedev.ri wrote: > Ping @hokein / @alexfh (as per git blame). > Not sure who is best suited to review this. I only made a couple of random fixes to these files, so I don't feel particularly competent to rev

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

alexfh added inline comments. Comment at: clang-tidy/tool/clang-tidy-diff.py:98 + parser.add_argument('-j', type=int, default=0, + help='number of tidy instances to be run in parallel.') zinovy.nis wrote: > alexfh wrote: > > The "clang-tid

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

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

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

alexfh added a comment. A few more comments. Comment at: clang-tidy/tool/clang-tidy-diff.py:58 + with lock: +sys.stdout.write((' '.join(command)).decode('utf-8') + '\n' + stdout.decode('utf-8') + '\n') +if stderr: nit: let's wrap this to 8

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

alexfh marked an inline comment as done. alexfh added inline comments. Comment at: clang/include/clang/Lex/PPCallbacks.h:346 + + virtual void setPreprocessor(Preprocessor *preprocessor) { +preprocessor_ = preprocessor; sammccall wrote: > This seems pretty ta

[PATCH] D58612: Use std::atomic<> for static counters.

alexfh created this revision. alexfh added a reviewer: ilya-biryukov. Herald added a subscriber: jfb. Herald added a project: clang. Fixes a data race and makes it possible to run clang-based tools in multithreaded environment with TSan. Repository: rG LLVM Github Monorepo https://reviews.llv

[PATCH] D58609: [clang-tidy] bugprone-string-integer-assignment: Reduce false positives.

alexfh added inline comments. Comment at: clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp:45 + const auto* RHS = BinOp->getRHS()->IgnoreParenImpCasts(); + // & , mask is a compile time constant. + Expr::EvalResult RHSVal; What about ` & `?

[PATCH] D58612: Use std::atomic<> for static counters.

alexfh updated this revision to Diff 188163. alexfh added a comment. Make the counters non-static members of ASTContext. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58612/new/ https://reviews.llvm.org/D58612 Files: clang/include/clang/AST/ASTC

[PATCH] D58612: Use std::atomic<> for static counters.

alexfh added a comment. In D58612#1408845 , @riccibruno wrote: > In D58612#1408843 , @ilya-biryukov > wrote: > > > In D58612#1408839 , @riccibruno > > wrote: > > > > > I do

[PATCH] D58612: Use std::atomic<> for static counters.

alexfh added a comment. In D58612#1408942 , @riccibruno wrote: > Okay, but what about the other similar uses of static members which have the > same problem ? Do you have any example in mind? I've only seen TSan warnings for these counters, nothing els

[PATCH] D58612: Use std::atomic<> for static counters.

alexfh added a comment. In D58612#1409024 , @riccibruno wrote: > In D58612#1408991 , @alexfh wrote: > > > In D58612#1408942 , @riccibruno > > wrote: > > > > > Okay, but what

[PATCH] D42645: New simple Checker for mmap calls

alexfh added inline comments. Herald added a subscriber: jdoerfert. Herald added a project: LLVM. Comment at: cfe/trunk/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:31-33 + static int ProtWrite; + static int ProtExec; + static int ProtRead; Can these b

[PATCH] D58612: Use std::atomic<> for static counters.

This revision was automatically updated to reflect the committed changes. Closed by commit rL354795: Make static counters in ASTContext non-static. (authored by alexfh, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://review

[PATCH] D58612: Use std::atomic<> for static counters.

alexfh added a comment. > In D58612#1409024 , @riccibruno > wrote: > >> ... >> For example in `DeclBase.cpp` >> >> #define DECL(DERIVED, BASE) static int n##DERIVED##s = 0; >> #define ABSTRACT_DECL(DECL) >> #include "clang/AST/DeclNodes.inc" >> >>

[PATCH] D58612: Make the static counters in ASTContext non-static

alexfh added a comment. In D58612#1409216 , @riccibruno wrote: > In D58612#1409215 , @alexfh wrote: > > > > In D58612#1409024 , @riccibruno > > > wrote: > > > > > >> ... >

[PATCH] D37138: Make run-clang-tidy compatible with Python 3.x

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Please don't forget go add cfe-commits to subscribers when you create a differential revision. LG with one comment. Comment at: clang-tidy/tool/run-clang-tidy.py:242 #

[PATCH] D37188: [clang-tools-extra] [cmake] Support running extra clang tool tests without static analyzer

alexfh added a comment. This could be done even more granularly, since clang-tidy only needs static analyzer to run clang-analyzer-* checks. But that would involve some careful #ifdef-ing of the parts of clang-tidy/ClangTidy.cpp, so I'll understand if you say you have little interest in this.

[PATCH] D37188: [clang-tools-extra] [cmake] Support running extra clang tool tests without static analyzer

alexfh added a comment. IIUC, these are the only clang-tidy tests that need static analyzer: clang/tools/extra/test/clang-tidy/nolint.cpp clang/tools/extra/test/clang-tidy/static-analyzer.cpp clang/tools/extra/test/clang-tidy/static-analyzer-config.cpp clang/tools/extra/test/clang-tidy/temporarie

[PATCH] D37188: [clang-tools-extra] [cmake] Support running extra clang tool tests without static analyzer

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D37188#854144, @mgorny wrote: > I might be missing something but currently clang-tidy is not built at all > when static analyzer is disabled. Yes, but it could be

[PATCH] D37479: run-clang-tidy: Report progress

alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: clang-tidy/tool/run-clang-tidy.py:155 args.quiet) -sys.stdout.write(' '.join(invocation) + '\n') +sys.

[PATCH] D37564: Update users of llvm::sys::ExecuteAndWait etc.

alexfh created this revision. Clang part of https://reviews.llvm.org/D37563 https://reviews.llvm.org/D37564 Files: include/clang/Driver/Compilation.h include/clang/Driver/Job.h lib/Driver/Compilation.cpp lib/Driver/Job.cpp lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp Index: lib/S

[PATCH] D37566: [clang-tidy] fixed misc-unused-parameters omitting parameters default value

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Thank you for the fix! LG with one nit. Comment at: test/clang-tidy/misc-unused-parameters.cpp:73 +// CHECK-FIXES: staticFunctionE(); + staticFunctionF(1); +// CHECK-FIXES:

[PATCH] D37572: [clang-tidy] SuspiciousEnumUsageCheck bugfix

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG modulo comments. Thank you for the fix! Comment at: test/clang-tidy/misc-suspicious-enum-usage.cpp:1 -// RUN: %check_clang_tidy %s misc-suspicious-enum-usage %t -- -confi

[PATCH] D37572: [clang-tidy] SuspiciousEnumUsageCheck bugfix

alexfh added inline comments. Comment at: test/clang-tidy/misc-suspicious-enum-usage.cpp:122 +struct a> { + enum { ah = ad::m, + ai = ae::m, aaron.ballman wrote: > alexfh wrote: > > aaron.ballman wrote: > > > This seems like a lot of complicated code for

[PATCH] D37572: [clang-tidy] SuspiciousEnumUsageCheck bugfix

alexfh added inline comments. Comment at: test/clang-tidy/misc-suspicious-enum-usage.cpp:122 +struct a> { + enum { ah = ad::m, + ai = ae::m, alexfh wrote: > aaron.ballman wrote: > > alexfh wrote: > > > aaron.ballman wrote: > > > > This seems like a lot o

[PATCH] D37572: [clang-tidy] SuspiciousEnumUsageCheck bugfix

alexfh accepted this revision. alexfh added a comment. Still LG. Fine to commit. Thanks for the fix! If you find time to construct the test case that triggers both affected code paths (my test seems to only inspect one of them, IIUC), it would be nice to do as a follow up. https://reviews.ll

[PATCH] D37566: [clang-tidy] fixed misc-unused-parameters omitting parameters default value

This revision was automatically updated to reflect the committed changes. Closed by commit rL313150: [clang-tidy] fixed misc-unused-parameters omitting parameters default value (authored by alexfh). Changed prior to commit: https://reviews.llvm.org/D37566?vs=114538&id=115047#toc Repository:

[PATCH] D37566: [clang-tidy] fixed misc-unused-parameters omitting parameters default value

alexfh added a comment. In https://reviews.llvm.org/D37566#866096, @PriMee wrote: > Done :) Could you please commit this for me? Sure, just committed the patch. Repository: rL LLVM https://reviews.llvm.org/D37566 ___ cfe-commits mailing list c

[PATCH] D37564: Update users of llvm::sys::ExecuteAndWait etc.

This revision was automatically updated to reflect the committed changes. Closed by commit rL313156: Update users of llvm::sys::ExecuteAndWait etc. (authored by alexfh). Repository: rL LLVM https://reviews.llvm.org/D37564 Files: cfe/trunk/include/clang/Driver/Compilation.h cfe/trunk/inclu

[PATCH] D37846: [clang-tidy] Fixed misc-unused-parameters omitting parameters square brackets

alexfh added a comment. Once we got that far, could you also ensure this works for function pointer arguments (e.g. `void f(void (*fn)()) {}`)? Repository: rL LLVM https://reviews.llvm.org/D37846 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D37846: [clang-tidy] Fixed misc-unused-parameters omitting parameters square brackets

alexfh added inline comments. Comment at: clang-tidy/misc/UnusedParametersCheck.cpp:141 !Indexer->getOtherRefs(Function).empty() || isOverrideMethod(Function)) { -SourceRange RemovalRange(Param->getLocation(), - Param->DeclaratorDecl::getSou

[PATCH] D37846: [clang-tidy] Fixed misc-unused-parameters omitting parameters square brackets

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG. I'll commit the patch for you. https://reviews.llvm.org/D37846 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/

[PATCH] D37846: [clang-tidy] Fixed misc-unused-parameters omitting parameters square brackets

This revision was automatically updated to reflect the committed changes. Closed by commit rL313355: [clang-tidy] Fixed misc-unused-parameters omitting parameters square brackets (authored by alexfh). Changed prior to commit: https://reviews.llvm.org/D37846?vs=115369&id=115395#toc Repository:

[PATCH] D37023: [analyzer] Fix bugreporter::getDerefExpr() again.

alexfh added a comment. Any news here? I'm wondering mainly because this patch is supposed to fix https://bugs.llvm.org/show_bug.cgi?id=34373. https://reviews.llvm.org/D37023 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm

[PATCH] D37978: [analyzer] Fix an assertion fail in VirtualCallChecker

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks reasonable, but please wait for someone who actually knows this code. Repository: rL LLVM https://reviews.llvm.org/D37978 ___ cfe-commit

[PATCH] D37263: [clang-format] Ignore case when sorting using-declarations

alexfh added a comment. As discussed offline, the sorting should also be stable to avoid no-op replacements for identical using declarations and randomizing the order of using declarations differing only in case (not that I'd expect this to be a frequent thing, but it's better to handle it prop

[PATCH] D37978: [analyzer] Fix an assertion fail in VirtualCallChecker

alexfh added a comment. Actually, after looking at the getBestDynamicClassType() implementation, I'm pretty sure it does all things IgnoreImpCasts() used to do that are relevant to the use case. So the patch seems to be trivially correct. Other reviewers' comments (if any) can be addressed post

[PATCH] D37263: [clang-format] Ignore case and stable sort using-declarations

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG, thanks! https://reviews.llvm.org/D37263 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinf

[PATCH] D20689: [clang-tidy] Suspicious Call Argument checker

alexfh added a comment. In https://reviews.llvm.org/D20689#871947, @barancsuk wrote: > @alexfh, would you mind taking a look at the changes that have been > introduced in the new patch? > > The main improvements are: > > - The checker has been shifted to the module `readability`. > - It is c

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

alexfh added a comment. > The metric is implemented as per COGNITIVE COMPLEXITY by SonarSource > specification version 1.2 (19 April 2017), Err, "I did ask them, and received an answer that it is it can be implemented in clang-tidy" might not be enough. Repository: rL LLVM https://reviews.

[PATCH] D38171: Implement clang-tidy check aliases.

alexfh added a comment. András, that's definitely an interesting idea. However, it might be interesting to explore a more principled approach: 1. Make `clang-diagnostic-*` checks first-class citizens and take full control of all diagnostics, i.e. disable all Clang diagnostics by default, and en

[PATCH] D38214: [analyzer] Fix crash on modeling of pointer arithmetic

alexfh added a comment. Thanks for the fix! Repository: rL LLVM https://reviews.llvm.org/D38214 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61989: [clang-tidy] enable modernize-concat-nested-namespaces on header files

alexfh added inline comments. Comment at: clang-tools-extra/test/clang-tidy/modernize-concat-nested-namespaces.h:1 +#pragma once + Please put the header under Inputs// directory to keep the tests directory a bit tidier. The header can then be renamed to somethi

[PATCH] D64671: [clang-tidy] New check: misc-init-local-variables

alexfh added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/InitLocalVariablesCheck.cpp:21 + Finder->addMatcher( + varDecl(unless(hasInitializer(anything(.bind("vardecl"), this); +} I believe, this should skip matches within template ins

[PATCH] D64671: [clang-tidy] New check: misc-init-local-variables

alexfh added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/InitLocalVariablesCheck.cpp:21 + Finder->addMatcher( + varDecl(unless(hasInitializer(anything(.bind("vardecl"), this); +} jpakkane wrote: > alexfh wrote: > > I believe, this sho

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

alexfh added a comment. In D33841#1589212 , @koldaniel wrote: > Hi, do you have any additional comments? Could you mark "Done" the comments you've addressed? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D33841/new/ https://reviews.llvm.org/D33

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

alexfh added inline comments. Comment at: clang-tidy/readability/RedundantExternCheck.cpp:38-40 +Message = "'extern' keyword has no effect"; + } else { +Message = "redundant 'extern' keyword"; These messages alone will be quite confusing unless the user

[PATCH] D64671: [clang-tidy] New check: misc-init-local-variables

alexfh added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/InitLocalVariablesCheck.cpp:21 + Finder->addMatcher( + varDecl(unless(hasInitializer(anything(.bind("vardecl"), this); +} jpakkane wrote: > alexfh wrote: > > jpakkane wrote: > >

[PATCH] D64861: [clang-tidy] Adjust location of namespace comment diagnostic

alexfh accepted this revision. alexfh added a comment. LG. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64861/new/ https://reviews.llvm.org/D64861 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

alexfh added a comment. Seems good now. Haojian, do you have any concerns? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55044/new/ https://reviews.llvm.org/D55044 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.o

[PATCH] D64671: [clang-tidy] New check: misc-init-local-variables

alexfh added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/InitLocalVariablesCheck.cpp:21 + Finder->addMatcher( + varDecl(unless(hasInitializer(anything(.bind("vardecl"), this); +} jpakkane wrote: > alexfh wrote: > > jpakkane wrote: > >

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

alexfh added a comment. LessClangTidyError only compares location and message, but it could also compare other things like notes, fixes, etc. For the problem outlined in the description of this patch we can probably include the checker name into the key. WDYT? Repository: rCTE Clang Tools E

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

alexfh added a comment. I think it will be a strict improvement to include the check name into the deduplication key (probably after the file and offset and before the message). I don't see any reason to hide this behind a flag or otherwise retain the old behavior. As for expanding the key to i

[PATCH] D64671: [clang-tidy] New check: misc-init-local-variables

alexfh added a comment. A general comment: "misc" is a sort of a heap of checks that otherwise don't have a good home. This one would probably better go to bugprone (or maybe there's a relevant CERT or C++ Core Guidelines rule?). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64671/new/

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