[PATCH] D93244: Remove the ast_type_traits namespace.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9c49b0bba0fc: Remove the ast_type_traits namespace. (authored by alexfh). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

[PATCH] D91302: Handle template instantiations better in clang-tidy check

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. Please fix the typo that results in a compile error. Comment at: clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp:107 + h

[PATCH] D91303: Simplify implementation of container-size-empty

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. Pre-merge builder can't apply this patch: https://buildkite.com/llvm-project/diff-checks/builds/18651 Is it based on https://reviews.llvm.org/D91302 ? Do we need the intermediate sta

[PATCH] D89117: Remove old create(MainFile)?IncludeInsertion overloads

alexfh created this revision. alexfh added a reviewer: hokein. Herald added a project: clang. alexfh requested review of this revision. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D89117 Files: clang-tools-extra/clang-tidy/utils/IncludeInserter.cpp clang-tools-extra/clang-

[PATCH] D89117: Remove old create(MainFile)?IncludeInsertion overloads

This revision was automatically updated to reflect the committed changes. Closed by commit rGfe4715c47f9c: Remove old create(MainFile)?IncludeInsertion overloads (authored by alexfh). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89117/new/ https://

[PATCH] D79674: [clang-tidy] Better support for Override function in RenamerClangTidy based checks

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good! Thanks for the fix! IIUC, this is related to https://bugs.llvm.org/show_bug.cgi?id=34879? Makes sense to specify this in the patch description. Repository: rG LLVM Github Monor

[PATCH] D79674: [clang-tidy] Better support for Override function in RenamerClangTidy based checks

alexfh added a comment. Feel free to ping patches every week or so. It looks like in this case all the reviewers were swamped with something else at the time. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79674/new/ https://reviews.llvm.org/D79674

[PATCH] D82089: [clang-tidy] modernize-loop-convert reverse iteration support

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

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

alexfh added a comment. Sorry for the delay. This patch fell through the cracks. If you're still interested, could you rebase it on top of current HEAD and upload a full diff? Or use the arcanist tool, see https://llvm.org/docs/Phabricator.html. Repository: rCTE Clang Tools Extra CHANGES SI

[PATCH] D89380: [clang-tidy] Fix for cppcoreguidelines-prefer-member-initializer to handle classes declared in macros

alexfh added a comment. Thanks for the fix! However, I'm not sure it's possible to correctly rewrite code in all cases where macros are involved. See a couple of motivating examples in the comment. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pref

[PATCH] D89380: [clang-tidy] Fix for cppcoreguidelines-prefer-member-initializer to handle classes declared in macros

alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp:170 + auto Diag = +diag(BeginLoc, "%0 should be initia

[PATCH] D89276: Support ObjC in IncludeInserter

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

[PATCH] D89276: Support ObjC in IncludeInserter

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGcc175c2cc8e6: Support ObjC in IncludeInserter (authored by alexfh). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D89886: [clang-tidy] Fix redefinition of module in the same module.modulemap file

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good! Comment at: clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp:27 +// detection. +if (!File->getName().endswith("module.modulemap")) +

[PATCH] D89886: [clang-tidy] Fix redefinition of module in the same module.modulemap file

alexfh added a comment. Ah, btw, any chance of adding a test for this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89886/new/ https://reviews.llvm.org/D89886 ___ cfe-commits mailing list cfe-commits@li

[PATCH] D89380: [clang-tidy] Fix for cppcoreguidelines-prefer-member-initializer to handle classes declared in macros

alexfh added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp:492 + +#define MACRO1 struct InMacro1 { int i; InMacro1() { i = 0; } }; +// CHECK-MESSAGES: :[[@LINE-1]]:54: warning: 'i' should be initialized in

[PATCH] D89886: [clang-tidy] Fix redefinition of module in the same module.modulemap file

alexfh accepted this revision. alexfh added a comment. In D89886#2347376 , @DmitryPolukhin wrote: > In D89886#2346851 , @alexfh wrote: > >> Ah, btw, any chance of adding a test for this? > > Oh, I was not able to c

[PATCH] D80499: Remove obsolete ignore*() matcher uses

alexfh added a comment. In D80499#2352339 , @steveire wrote: > @alexfh This change is based on the behavior of AST Matchers being changed to > ignore invisible/implicit AST nodes by default. As the default was not > changed in the end, this patch would n

[PATCH] D93688: [ASTMatchers] Ensure that we can match inside lambdas

alexfh added a comment. This patch causes practically infinite traversal times on code that contains deeply nested lambdas. I'll try to get a suitable repro, but could you maybe revert this in the meantime? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D93688: [ASTMatchers] Ensure that we can match inside lambdas

alexfh added a comment. In D93688#2525947 , @alexfh wrote: > This patch causes practically infinite traversal times on code that contains > deeply nested lambdas. I'll try to get a suitable repro, but could you maybe > revert this in the meantime? Actua

[PATCH] D95573: [ASTMatchers] Avoid pathological traversal over nested lambdas

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. This fixes the issue with exponential traversal times for deeply nested lambdas. Please add a test though. For example, this one: void f() { [] { [] { [] { [] { [] {

[PATCH] D95573: [ASTMatchers] Avoid pathological traversal over nested lambdas

alexfh added a comment. Thanks for the prompt fix, btw! Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:2064 + if (const auto *MD = dyn_cast(D)) { +if (const auto *RD = MD->getParent()) { + if (RD->isLambda()) { Please specify the type expli

[PATCH] D95614: [clang-tidy] Applied clang-tidy fixes. NFC

alexfh updated this revision to Diff 319979. alexfh added a comment. Manually cleaned up suboptimal fixes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95614/new/ https://reviews.llvm.org/D95614 Files: clang-tools-extra/clang-tidy/abseil/TimeSu

[PATCH] D95614: [clang-tidy] Applied clang-tidy fixes. NFC

alexfh added inline comments. Comment at: clang-tools-extra/clang-tidy/abseil/TimeSubtractionCheck.cpp:96 void TimeSubtractionCheck::registerMatchers(MatchFinder *Finder) { - for (auto ScaleName : + for (const auto *ScaleName : {"Hours", "Minutes", "Seconds", "Millis",

[PATCH] D95607: Fix traversal with hasDescendant into lambdas

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

[PATCH] D95562: [ASTMatchers] Fix traversal below range-for elements

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

[PATCH] D89380: [clang-tidy] Fix for cppcoreguidelines-prefer-member-initializer to handle classes declared in macros

alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Herald added a subscriber: shchenz. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer.cpp:6 +//of this

[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

[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] 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] 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] 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] 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] 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] 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.h:24-26 +/// * `FlagBasicIncrements`- if set to `true`, additional to flagging +// functions exceeding the threshold also every piece of code (loops, if +//

[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] 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] 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#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] 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] 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] 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] 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 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] 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] 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] 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

[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

[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 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 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

[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] 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 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 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] 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] 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] 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] 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] 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 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#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#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#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#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] 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] 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] 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] 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. 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] 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. 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] 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] 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] 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] 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] 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. 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#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#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#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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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 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] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

alexfh added inline comments. Comment at: include/clang/Tooling/Core/Diagnostic.h:51 +/// Represents a single source ranges to be associated with a diagnostic. +struct DiagnosticAssociatedRange { + DiagnosticAssociatedRange() = default; This comment was lost by

[PATCH] D69746: [analyzer] FixItHint: Apply and test hints with the Clang Tidy's script

alexfh added a comment. Herald added subscribers: martong, steakhal. A few nits. Comment at: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:93 + + bool IncludePath = false, ShouldEmitAsError = false, FixitsAsRemarks = false, + ApplyFixIts = false; ---

[PATCH] D72730: [clang-tidy] run-clang-tidy -export-fixes exports only fixes, not all warnings

alexfh added a comment. In D72730#1866043 , @AlexanderLanin wrote: > On second thought maybe this should be fixed in clang-tidy and not here? > Maybe besides `-export-fixes` there should be an `-export-warnings` or just > `-yaml-export`? I like the id

[PATCH] D69782: Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed b

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG. Thanks! Do you have commit rights or should someone help you committing the patch? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69782/new/ https://reviews.llvm.org/D69782 ___

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