[PATCH] D154093: [clang-format] Break long string literals in C#, etc.

alexfh added a comment. This patch makes clang-format produce invalid JS/TS code In D154093#4644512 , @eaeltsin wrote: > This introduces an invalid TS transformation, from > > type x = 'ab'; > > to > > type x = ('a'+'b'); > > Which doesn't compile -

[PATCH] D159346: Revert "Reapply: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas"

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rGb7f491564484: Revert "Reapply: [IRGen] Emit lifetime intrinsics around temporary aggregate… (authored by alexfh). Reposit

[PATCH] D74094: Reapply: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas

alexfh added a comment. In D74094#4633785 , @alexfh wrote: > This commit caused invalid AddressSanitizer: stack-use-after-scope errors in > our internal setup. Trying to create a standalone repro, but so far we think > that the patch is incorrect. @vital

[PATCH] D74094: Reapply: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas

alexfh added subscribers: vitalybuka, alexfh. alexfh added a comment. This commit caused invalid AddressSanitizer: stack-use-after-scope errors in our internal setup. Trying to create a standalone repro, but so far we think that the patch is incorrect. @vitalybuka says "The patch seems wrong, ca

[PATCH] D159346: Revert "Reapply: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas"

alexfh created this revision. alexfh added reviewers: vitalybuka, nickdesaulniers. Herald added a subscriber: jvesely. Herald added a project: All. alexfh requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This reverts commit e698695fbbf62e667

[PATCH] D158226: [CUDA/NVPTX] Improve handling of memcpy for -Os compilations.

alexfh accepted this revision. alexfh 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/D158226/new/ https://reviews.llvm.org/D158226 ___

[PATCH] D154324: [C++20] [Modules] [ODRHash] Use CanonicalType for base classes

alexfh added a comment. In D154324#4522552 , @Hahnfeld wrote: > In D154324#4522551 , @alexfh wrote: > >> BTW, if in a.h I change >> >> typename std::enable_if<::p::P::value>::type> >> >> to >> >> typename std:

[PATCH] D154324: [C++20] [Modules] [ODRHash] Use CanonicalType for base classes

alexfh added a comment. In D154324#4522541 , @alexfh wrote: > In D154324#4520096 , @alexfh wrote: > >> In D154324#4516964 , @alexfh wrote: >> >>> In D154324#4516917

[PATCH] D154324: [C++20] [Modules] [ODRHash] Use CanonicalType for base classes

alexfh added a comment. In D154324#4520096 , @alexfh wrote: > In D154324#4516964 , @alexfh wrote: > >> In D154324#4516917 , @ChuanqiXu >> wrote: >> >>> Maybe we got somet

[PATCH] D154324: [C++20] [Modules] [ODRHash] Use CanonicalType for base classes

alexfh added a comment. In D154324#4516964 , @alexfh wrote: > In D154324#4516917 , @ChuanqiXu > wrote: > >> Maybe we got something wrong with this. I'd like to revert this patch in >> case it breaks something. B

[PATCH] D154324: [C++20] [Modules] [ODRHash] Use CanonicalType for base classes

alexfh added a comment. In D154324#4516917 , @ChuanqiXu wrote: > In D154324#4516605 , @alexfh wrote: > >> Hi, we've started seeing compilation errors with our modularized build after >> this commit. The errors sa

[PATCH] D154324: [C++20] [Modules] [ODRHash] Use CanonicalType for base classes

alexfh added a comment. Hi, we've started seeing compilation errors with our modularized build after this commit. The errors say `'SomeType' has different definitions in different modules`, but then point to the same definition that comes from the same textual header included into two modules.

[PATCH] D154417: [libc++] Disable tree invariant check in asserts mode

alexfh added a comment. Given the comment in https://reviews.llvm.org/D153672#4468348, I think, this should be fine to submit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154417/new/ https://reviews.llvm.org/D154417

[PATCH] D154417: [libc++] Disable tree invariant check in asserts mode

alexfh accepted this revision. alexfh added a comment. Thanks for the temporary fix, Hans! We're also affected by this. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154417/new/ https://reviews.llvm.org/D154417 __

[PATCH] D153430: [Clang][Driver] Warn on invalid Arm or AArch64 baremetal target triple

alexfh added a comment. After the patch, LLVM still has a number of `aarch64-arm-none-eabi` usages in tests, which is also considered invalid now: https://gcc.godbolt.org/z/z8cY5j68M Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153430/new/ https:

[PATCH] D143840: [clang] Add the check of membership for the issue #58674 and improve the lookup process

alexfh added a comment. In D143840#4147582 , @joanahalili wrote: > In D143840#4147234 , @joanahalili > wrote: > >> Heads up: We are experiencing a series of clang crashes because of this >> commit. >> >> What w

[PATCH] D140158: [CUDA] Allow targeting NVPTX directly without a host toolchain

alexfh added a comment. In D140158#4082804 , @jhuber6 wrote: > In D140158#4082789 , @alexfh wrote: > >> This patch breaks our cuda compilations. The output file isn't created after >> it: >> >> $ echo 'extern "

[PATCH] D140158: [CUDA] Allow targeting NVPTX directly without a host toolchain

alexfh added a comment. This patch breaks our cuda compilations. The output file isn't created after it: $ echo 'extern "C" __attribute__((global)) void q() {}' >q.cc $ good-clang \ -nocudainc -x cuda \ --cuda-path=somepath/cuda/ \ -Wno-unknown-cuda-version --cuda-device-onl

[PATCH] D142500: Fix one of the regressions found in revert of concept sugaring

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Thanks for preparing this fix! I've verified that it fixes the original reproducer as well. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142500/new/ https://reviews.llvm.org/D142500

[PATCH] D137213: [clang][modules] NFCI: Pragma diagnostic mappings: write/read FileID instead of SourceLocation

alexfh added inline comments. Comment at: clang/lib/Serialization/ASTReader.cpp:6343 "Invalid data, missing pragma diagnostic states"); - SourceLocation Loc = ReadSourceLocation(F, Record[Idx++]); - auto IDAndOffset = SourceMgr.getDecomposedLoc(Loc); -

[PATCH] D137213: [clang][modules] NFCI: Pragma diagnostic mappings: write/read FileID instead of SourceLocation

alexfh added inline comments. Comment at: clang/lib/Serialization/ASTReader.cpp:6343 "Invalid data, missing pragma diagnostic states"); - SourceLocation Loc = ReadSourceLocation(F, Record[Idx++]); - auto IDAndOffset = SourceMgr.getDecomposedLoc(Loc); -

[PATCH] D137213: [clang][modules] NFCI: Pragma diagnostic mappings: write/read FileID instead of SourceLocation

alexfh added inline comments. Comment at: clang/lib/Serialization/ASTReader.cpp:6343 "Invalid data, missing pragma diagnostic states"); - SourceLocation Loc = ReadSourceLocation(F, Record[Idx++]); - auto IDAndOffset = SourceMgr.getDecomposedLoc(Loc); -

[PATCH] D137213: [clang][modules] NFCI: Pragma diagnostic mappings: write/read FileID instead of SourceLocation

alexfh added a subscriber: jgorbe. alexfh added inline comments. Comment at: clang/lib/Serialization/ASTReader.cpp:6343 "Invalid data, missing pragma diagnostic states"); - SourceLocation Loc = ReadSourceLocation(F, Record[Idx++]); - auto IDAndOffset = Sou

[PATCH] D137213: [clang][modules] NFCI: Pragma diagnostic mappings: write/read FileID instead of SourceLocation

alexfh added inline comments. Comment at: clang/lib/Serialization/ASTReader.cpp:6343 "Invalid data, missing pragma diagnostic states"); - SourceLocation Loc = ReadSourceLocation(F, Record[Idx++]); - auto IDAndOffset = SourceMgr.getDecomposedLoc(Loc); -

[PATCH] D137213: [clang][modules] NFCI: Pragma diagnostic mappings: write/read FileID instead of SourceLocation

alexfh added a comment. Heads up: this commit has broken compilation in a small number of cases: some #includes from modularized headers now fail to be found. I'm still trying to figure out what exactly happened, but some behavior has definitely changed by this commit. Repository: rG LLVM G

[PATCH] D136565: [clang] Instantiate alias templates with sugar

alexfh added a comment. And another problem with this patch: there's another pattern (or multiple different patterns?) in the code, that result in around 3x clang memory usage increase after this patch. The result of `-print-stats` doesn't make it clear where the additional allocations come fro

[PATCH] D137751: Produce a more informative diagnostics when Clang runs out of source locations

alexfh added a comment. Thanks for adding this diagnostic! I wonder whether we can include SLoc usage information into `-print-stats` output? Comment at: clang/lib/Basic/SourceManager.cpp:2251 + uint64_t CountedSize = 0; + for (int IDIndex = -(int)LoadedSLocEntryTable.size()

[PATCH] D136565: [clang] Instantiate alias templates with sugar

alexfh added a comment. In D136565#3913932 , @mizvekov wrote: > @alexfh Thanks! > > While there is a huge increase in the amount of UsingTypes, it seems the > total amount is still reasonable and does not explain the perf hit. > > Perhaps this is a case

[PATCH] D136565: [clang] Instantiate alias templates with sugar

alexfh added a comment. In D136565#3913065 , @mizvekov wrote: > In D136565#3911884 , @alexfh wrote: > >> Hi Matheus, 279fe6281d2ca5b2318c7437316c28750feaac8d >>

[PATCH] D136565: [clang] Instantiate alias templates with sugar

alexfh added a comment. Hi Matheus, 279fe6281d2ca5b2318c7437316c28750feaac8d causes compilation timeout on some of our internal files. We're trying to get a test case we can share, but so far the only information I can provid

[PATCH] D136539: [Lex] Bring back the magic number 50 in updateConsecutiveMacroArgTokens.

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. Undo LGTM, since we decided to go with a revert. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136539/new/ https://reviews.llvm.org/D1

[PATCH] D136539: [Lex] Bring back the magic number 50 in updateConsecutiveMacroArgTokens.

alexfh added a comment. FYI, I've reverted f83347b0bedb22ea676861c8e4e2ed9c31371ade and 74e4f778cf16cbf7163b5c6de6027a43f5e9169f . Repository: rG LLVM G

[PATCH] D136539: [Lex] Bring back the magic number 50 in updateConsecutiveMacroArgTokens.

alexfh added a comment. In D136539#3877872 , @sammccall wrote: > This is a subtle change that needs careful review, and honestly should have a > test. > > I realize there's a breakage that needs to be fixed with some urgency and you > believe you're jus

[PATCH] D136539: [Lex] Bring back the magic number 50 in updateConsecutiveMacroArgTokens.

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/D136539/new/ https://reviews.llvm.org/D136539

[PATCH] D134929: [clang-tidy] Skip variadic ctors in use-equals-default

alexfh accepted this revision. alexfh 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/D134929/new/ https://reviews.llvm.org/D134929 __

[PATCH] D111283: [clang] template / auto deduction deduces common sugar

alexfh added a comment. In D111283#3786678 , @mizvekov wrote: > @alexfh This new revision that I just pushed should be good. > > Do you want to give it a look / test, or should we go ahead and merge it? Thanks for the fix! If it fixes the test case I pro

[PATCH] D111283: [clang] template / auto deduction deduces common sugar

alexfh added a comment. I've reverted d200db38637884fd0b421802c6094b2a03ceb29e in 637da9de4c6619c0e179c2c2f0dbfebd08ac2a0f . And a bit more compact test ca

[PATCH] D111283: [clang] template / auto deduction deduces common sugar

alexfh added a comment. In D111283#3785259 , @alexfh wrote: > In D111283#3785240 , @alexfh wrote: > >> In D111283#3783627 , @alexfh wrote: >> >>> Hi Matheus, an early head

[PATCH] D111283: [clang] template / auto deduction deduces common sugar

alexfh added a comment. In D111283#3785240 , @alexfh wrote: > In D111283#3783627 , @alexfh wrote: > >> Hi Matheus, an early heads up: this commit is causing clang crashes on some >> of our code. I'll post more de

[PATCH] D111283: [clang] template / auto deduction deduces common sugar

alexfh added a comment. In D111283#3783627 , @alexfh wrote: > Hi Matheus, an early heads up: this commit is causing clang crashes on some > of our code. I'll post more details a bit later. The stack trace of the failure looks like this: #0 0x564

[PATCH] D111283: [clang] template / auto deduction deduces common sugar

alexfh added a comment. Hi Matheus, an early heads up: this commit is causing clang crashes on some of our code. I'll post more details a bit later. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111283/new/ https://reviews.llvm.org/D111283 __

[PATCH] D133102: [clang-tidy] Extend simplify-boolean-expr check

alexfh added a comment. For example, LLVM coding standards recommends using early exits: https://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133102/new/ https://rev

[PATCH] D133102: [clang-tidy] Extend simplify-boolean-expr check

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. FWIW, I'm not sure this will usually be a simplification of the code. Splitting the condition and using early return is quite a frequent recommendation in code reviews, and I would e

[PATCH] D132550: Changes to code ownership in clang and clang-tidy

alexfh accepted this revision. alexfh added a comment. LGTM. Thanks for driving this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132550/new/ https://reviews.llvm.org/D132550 ___ cfe-commits mailing li

[PATCH] D132998: [clang-tidy] Restrict use-equals-default to c++11-or-later

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. In D132998#3762578 , @alexander-shaposhnikov wrote: > Regarding the practical side - yeah, I've come across this issue (and others) > while trying to

[PATCH] D133006: [clang-tidy] Skip copy assignment operators with nonstandard return types

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133006/new/ https://reviews.llvm.org/D133006 ___ cfe-comm

[PATCH] D132998: [clang-tidy] Restrict use-equals-default to c++11-or-later

alexfh added a comment. In D132998#3760603 , @alexander-shaposhnikov wrote: > My assumption was that a codebase needs to compile with c++11 in the first > place - otherwise the automatic fixit will break the build (as it happens > right now). > I was l

[PATCH] D128113: Clang: fix AST representation of expanded template arguments.

alexfh added a comment. In D128113#3751477 , @mizvekov wrote: > In D128113#3747946 , @alexfh wrote: > >> For now we've added a workaround for the most problematic use case and got >> us unblocked. Thus there is n

[PATCH] D128113: Clang: fix AST representation of expanded template arguments.

alexfh added a comment. In D128113#3746674 , @mizvekov wrote: > In D128113#3745888 , @alexfh wrote: > >> The main questions we need to answer here are: >> >> 1. is the cost of storing parameter pack substitution i

[PATCH] D128113: Clang: fix AST representation of expanded template arguments.

alexfh added a comment. In D128113#3744353 , @mizvekov wrote: > In D128113#3744315 , @alexfh wrote: > >> The whole project seems like a great improvement in clang diagnostics, but I >> don't yet see how template

[PATCH] D128113: Clang: fix AST representation of expanded template arguments.

alexfh added a comment. In D128113#3744296 , @mizvekov wrote: > In D128113#3744290 , @alexfh wrote: > >> Do you have a practical example that would use the substitution index? I >> believe you had something in mi

[PATCH] D128113: Clang: fix AST representation of expanded template arguments.

alexfh added a comment. In D128113#3744128 , @mizvekov wrote: > In D128113#3743936 , @alexfh wrote: > >> I wonder what is the practical application of the substitution index in >> SubstTemplateTypeParmType? Diagn

[PATCH] D128113: Clang: fix AST representation of expanded template arguments.

alexfh added a comment. In D128113#3743736 , @mizvekov wrote: > In D128113#3742779 , @joanahalili > wrote: > >> This is the reproducer we managed to create for the memory increase. As >> mentioned above we noti

[PATCH] D128113: Clang: fix AST representation of expanded template arguments.

alexfh added subscribers: kadircet, sammccall, alexfh. alexfh added a comment. Adding to the comment @joanahalili posted: this particular translation unit happens to have a large number of reverse dependencies and Clang's memory allocation has pushed it over the limits, which makes this issue qu

[PATCH] D129608: [Clang][OpenMP] Fix segmentation fault when data field is used in is_device_pt.

alexfh added a comment. In D129608#3735496 , @jyu2 wrote: > Hi @alexfh, > > How could I reproduce the problem? Thanks. > Thanks. > > Jennifer It should be reproducible by running the tests with the address sanitizer enabled. But it looks like this migh

[PATCH] D129608: [Clang][OpenMP] Fix segmentation fault when data field is used in is_device_pt.

alexfh added a comment. It looks like this commit introduces an AddressSanitizer: stack-use-after-scope error: ==2796==ERROR: AddressSanitizer: stack-use-after-scope on address 0x7f544e9acc20 at pc 0x55f1f7f4b83e bp 0x7ffdfb37c560 sp 0x7ffdfb37c558 READ of size 4 at 0x7f544e9acc20 thread T0

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

alexfh added a comment. One more problem related to this patch: it changes the behavior of __PRETTY_FUNCTION__: https://gcc.godbolt.org/z/Mvnj9j74E. There may be dependencies upon the specific format of expansions of this macro: tests, log processing tools, libraries like ctti, and probably oth

[PATCH] D128449: [clang] Introduce -Warray-parameter

alexfh added a comment. Serge, this diagnostic doesn't handle non-type template parameters correctly in some cases. Here's an example derived from a real code: https://gcc.godbolt.org/z/cvP8od5c6 template struct T { static void F(int a[8 * K]); }; template void T::F(int a[8 * K])

[PATCH] D128571: [X86] Support `_Float16` on SSE2 and up

alexfh added a comment. @pengfei could you fix the Darwin tests as well? And a general comment regarding the ongoing `_Float16` effort: I think that this change should have been a part of https://reviews.llvm.org/D107082 to make it possible to build a consistently working toolchain. Thus, if th

[PATCH] D121306: [Sema] add warning for tautological FP compare with literal

alexfh added a comment. What's the right fix for this warning, using a literal of the same floating point type? Did you consider adding a fixit hint to the diagnostic? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121306/new/ https://reviews.llvm.

[PATCH] D117376: Remove reference type when checking const structs

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

[PATCH] D116048: [clang][CodeGen][UBSan] VLA size checking for unsigned integer parameter

alexfh added a comment. In D116048#3261573 , @alexfh wrote: > We've started seeing some tests dying with SIGILL (illegal instruction) after > this patch. I'm working on a reduced repro, but please take a look, if you > can find any issues yourself. Fal

[PATCH] D116048: [clang][CodeGen][UBSan] VLA size checking for unsigned integer parameter

alexfh added a comment. We've started seeing some tests dying with SIGILL (illegal instruction) after this patch. I'm working on a reduced repro, but please take a look, if you can find any issues yourself. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

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

alexfh added a comment. Do you need help landing the change? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D49864/new/ https://reviews.llvm.org/D49864 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

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

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good with one suggestion. Comment at: clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py:123 + 'lines.' +

[PATCH] D117460: [clang-tidy][NFC] Reduce map lookups in IncludeSorter

alexfh added a comment. I wonder what motivated the patch. Is this a performance optimization? If so, do you have any measurements? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117460/new/ https://reviews.llvm.org/D117460 ___

[Diffusion] rGc93f93b2e3f2: Revert "Revert "Recommit "Revert "[CVP] processSwitch: Remove default case when…

alexfh added a comment. Reduced the problematic file to this: F20567816: test2.cc $ time ./clang-before -O3 -c test2.cc real 0m0.242s user 0m0.178s sys 0m0.064s $ time ./clang-after -O3 -c test2.cc real 0m41.063s user 0m40.971s sys 0m0.0

[Diffusion] rGc93f93b2e3f2: Revert "Revert "Recommit "Revert "[CVP] processSwitch: Remove default case when…

alexfh added a comment. In rGc93f93b2e3f28997f794265089fb8138dd5b5f13#1039669 , @bgraur wrote: > Early heads up that this revision causes a large regression in compilation > time for some of our internal source files:

[Diffusion] rGc93f93b2e3f2: Revert "Revert "Recommit "Revert "[CVP] processSwitch: Remove default case when…

alexfh added a subscriber: junparser. BRANCHES fix_asan, main Users: junparser (Author) https://reviews.llvm.org/rGc93f93b2e3f2 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[Diffusion] rGc93f93b2e3f2: Revert "Revert "Recommit "Revert "[CVP] processSwitch: Remove default case when…

alexfh added subscribers: cfe-commits, alexfh, bgraur. BRANCHES main Users: junparser (Author) https://reviews.llvm.org/rGc93f93b2e3f2 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm

[PATCH] D113148: Add new clang-tidy check for string_view(nullptr)

alexfh added a comment. In D113148#3127646 , @ymandel wrote: > This looks really good. An impressive effort! Just a few changes. Also, > please ping Alex to get his opinion on the high level issue. Since this check focuses really well on the issue of c

[PATCH] D111109: AddGlobalAnnotations for function with or without function body.

alexfh added a comment. Reduced the test further to: struct k { ~k() __attribute__((annotate(""))) {} }; void m() { k(); } Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D09/new/ https://reviews.llvm.org/D09 __

[PATCH] D111109: AddGlobalAnnotations for function with or without function body.

alexfh added a comment. Reduced test case: template void b(a); template class d { public: class e { public: c *f(); }; e *g(); }; template class j; template class j { public: class k { public: k(int *); ~k(); }; int n(); d l;

[PATCH] D111109: AddGlobalAnnotations for function with or without function body.

alexfh added a comment. It looks like this patch causes clang to crash on valid code with the following stack trace: #0 0x560442f12f45 SignalHandler(int) #1 0x7ffac7c789a0 __restore_rt #2 0x560440083992 llvm::LazyCallGraph::Node::populateSlow() #3 0x560440082a60 llvm:

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

alexfh added a comment. In D49864#2857630 , @janosimas wrote: > That makes sense. > Should I add it somewhere? Or do I need to talk to someone? Please update clang-tools-extra/docs/ReleaseNotes.rst. It looks like there's no other docs for this script.

[PATCH] D34654: Allow passing a regex for headers to exclude from clang-tidy

alexfh added a comment. Repeating my question from an earlier comment: would a header glob list (similar to how the format of the `-checks=` flag) be enough for the use cases folks have? On the one hand glob list doesn't support all the features of regular expressions, but are they all useful f

[PATCH] D104601: [Preprocessor] Implement -fminimize-whitespace.

alexfh added a comment. In D104601#2916973 , @Meinersbur wrote: > @romanovvlad This is due to -fms-extensions. It Expands `__Pragma` to > `#pragma` instead of keeping it a `__Pragma`. This is a one-line fix. > > @alexfh This was fixed by D106924

[PATCH] D104601: [Preprocessor] Implement -fminimize-whitespace.

alexfh added a comment. Looking at this once again, I'm pretty sure that reverting this is much safer than trying to fix forward. Unless there's a really trivial fix to the outstanding issues, I'll revert this later today. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[PATCH] D104601: [Preprocessor] Implement -fminimize-whitespace.

alexfh added a comment. In D104601#2915102 , @alexfh wrote: > This commit changes the behavior of clang -E -P even when no > -fminimize-whitespace is used. This breaks certain use cases like using clang > to preprocess files for flex, which turns out to

[PATCH] D104601: [Preprocessor] Implement -fminimize-whitespace.

alexfh added a comment. This commit changes the behavior of clang -E -P even when no -fminimize-whitespace is used. This breaks certain use cases like using clang to preprocess files for flex, which turns out to be sensitive to the presence of line breaks in places where C++ compilers aren't.

[PATCH] D105890: [NFC] Add paranthesis around logical expression to silence -Wlogical-op-parentheses warning.

alexfh added a comment. In D105890#2873969 , @thakis wrote: > I think this is incorrect. See D105892 and > https://reviews.llvm.org/D104915#2873851 . Don't blindly land changes to > suppress warnings – the warnings exi

[PATCH] D105890: [NFC] Add paranthesis around logical expression to silence -Wlogical-op-parentheses warning.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe9533b849207: [NFC] Add paranthesis around logical expression to silence -Wlogical-op… (authored by bgraur, committed by alexfh). Repository: rG L

[PATCH] D105890: [NFC] Add paranthesis around logical expression to silence -Wlogical-op-parentheses warning.

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG. I can commit the patch for you. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105890/new/ https://reviews.llvm.org/D105890

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

alexfh added a comment. Function-wise the patch looks good, but I'm somewhat concerned about silently breaking users. At the very least there should be a relevant entry in the release notes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D49864/new/

[PATCH] D69560: [clang-tidy] Add 'bugprone-easily-swappable-parameters' check

alexfh added a comment. I thought I sent this batch of comments loong ago =\ Better late than never. Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:113-120 +#define FB(Name, K) MIX_##Name = (1ull << (K##ull - 1ull)) + + FB(None, 1),

[PATCH] D69560: [clang-tidy] Add 'bugprone-easily-swappable-parameters' check

alexfh added a comment. BTW, any ideas why "patch application failed"? (See the Build status in the Diff Detail section) It would be really nice to get pre-merge checks to work for the patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69560/new

[PATCH] D69560: [clang-tidy] Add 'bugprone-easily-swappable-parameters' check

alexfh added a comment. Thanks for the great work! This is mostly looking fine, but I've added a few comments. I could only make a quite superficial review so far, but I'll try to take a deeper look into this soon. Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwap

[PATCH] D98070: [clang-tidy] Add option to ignore macros in readability-function-cognitive-complexity check.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG8a944d82cd14: [clang-tidy] Add option to ignore macros in readability-function-cognitive… (authored by massberg, committed by alexfh). Repository:

[PATCH] D98070: [clang-tidy] Add option to ignore macros in readability-function-cognitive-complexity check.

alexfh accepted this revision. alexfh added a comment. LG with a couple of nits. Comment at: clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:230 + explicit FunctionASTVisitor(const bool IgnoreMacros) + : IgnoreMacros(IgnoreMacros){}; + -

[PATCH] D98321: [NFC] Unify FIME with FIXME in comments

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG481079e2841f: [NFC] Unify FIME with FIXME in comments (authored by b1f6c1c4, committed by alexfh). Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D98321: [NFC] Unify FIME with FIXME in comments

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good. Thanks for the fix! I'll get it landed for you. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98321/new/ https://reviews.llvm.org/D9

[PATCH] D97288: Added `Follow` parameter to llvm::vfs::FileSystem::status()

alexfh added a comment. In D97288#2581908 , @sammccall wrote: > How strong is the need for this? > This adds complexity to a widely implemented and used interface, and the > combination of virtual + default parameters can be at least a little > confusing

[PATCH] D97288: Added `Follow` parameter to llvm::vfs::FileSystem::status()

alexfh created this revision. alexfh added a reviewer: sammccall. Herald added subscribers: dexonsmith, usaxena95, kadircet, arphaman, hiraditya. alexfh requested review of this revision. Herald added projects: clang, LLVM. Herald added a subscriber: llvm-commits. Currently, status() calls always

[PATCH] D96281: [clang-tidy] Add options to flag individual core increments and to ignore macros to readability-function-cognitive-complexity check.

alexfh added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.h:24-26 +/// * `FlagBasicIncrements`- if set to `true`, additional to flagging +// functions exceeding the threshold also every piece of code (loops, if +//

[PATCH] D96542: [clang-tidy] Fix `TransformerClangTidyCheck`'s handling of include insertions.

alexfh added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp:108 + Diag << Inserter.createIncludeInsertion( + Result.SourceManager->getFileID(T.Range.getBegin()), T.Replacement); break; Can this b

[PATCH] D96281: [clang-tidy] Add options to flag individual core increments and to ignore macros to readability-function-cognitive-complexity check.

alexfh added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:230 + explicit FunctionASTVisitor(const bool IgnoreMacros) + : RecursiveASTVisitor(), IgnoreMacros(IgnoreMacros){}; + Is a default base

[PATCH] D90851: [clang-tidy] Extending bugprone-signal-handler with POSIX functions.

alexfh added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/system-header-posix-api.h:1 -//===--- signal.h - Stub header for tests ---*- C++ -*-===// +//===--- system-header-posix-api.h - Stub header for tests --*-

[PATCH] D95403: [clang-tidy][analyzer][WIP] Clang-tidy reverse integration into Static Analyzer.

alexfh added a comment. Artem, could you set the repository to `rG LLVM Github Monorepo` when uploading patches as mentioned in https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface ? This way you'll allow pre-merge checks to run. In D95403#2534714

[PATCH] D95403: [clang-tidy][analyzer][WIP] Clang-tidy reverse integration into Static Analyzer.

alexfh added inline comments. Comment at: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:54-60 +#include "../../clang-tools-extra/clang-tidy/ClangTidyCheck.h" +#include "../../clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h" +#include "../../clang-tools-extra/c

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

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. Apologies again for the long delay. A diff with full context would still be appreciated. Please read https://llvm.org/docs/Phabricator.html for instructions. The clang-tools-extra/t

  1   2   3   4   5   6   7   8   9   10   >