[PATCH] D141757: [clangd] allow extracting to variable for complete lambda expressions

2023-01-23 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. branch cut is upon us, so it'd be great if you can land this soon (or let us know if you don't have commit access) Comment at: clang-tools-extra/clangd/refactor/tweaks/E

[PATCH] D139921: [include-cleaner] Ranking of providers based on hints

2023-01-23 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 491286. kadircet marked 23 inline comments as done. kadircet added a comment. - Address comments & rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139921/new/ https://reviews.llvm.org/D139921 Files: cl

[PATCH] D139921: [include-cleaner] Ranking of providers based on hints

2023-01-23 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h:133 + + Header(std::in_place_t, decltype(Storage) Sentinel) + : Storage(std::move(Sentinel)) {} sammccall wrote: > this is a bit confusing..

[PATCH] D142092: [include-mapping] Allow multiple headers for the same symbol. Choose the first header of available ones.

2023-01-23 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang/tools/include-mapping/cppreference_parser.py:174 - # std::remove<> has variant algorithm. - "std::remove": ("algorithm"), - } this is actually checking for something else (sorry for the confusing naming

[PATCH] D139921: [include-cleaner] Ranking of providers based on hints

2023-01-23 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG749c6a708340: [include-cleaner] Ranking of providers based on hints (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139921/new/ https:

[PATCH] D142092: [include-mapping] Allow multiple headers for the same symbol. Choose the first header of available ones.

2023-01-23 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang/include/clang/Tooling/Inclusions/StdSymbolMap.inc:100 SYMBOL(atoll, std::, ) +SYMBOL(atomic, std::, ) +SYMBOL(atomic, std::, ) hokein wrote: > Conceptually, this (and other `atomic_*` symbols) doesn't feel correc

[PATCH] D141757: [clangd] allow extracting to variable for complete lambda expressions

2023-01-23 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet requested changes to this revision. kadircet added a comment. This revision now requires changes to proceed. sorry i'll need to take a look at this again. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141757/new/ https://reviews.llvm.org/D

[PATCH] D127629: [clang] Use correct visibility parameters when following a Using declaration

2022-06-13 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a reviewer: kadircet. kadircet added inline comments. Comment at: clang/lib/Sema/SemaCodeComplete.cpp:1269 if (const auto *Using = dyn_cast(R.Declaration)) { CodeCompletionResult Result(Using->getTargetDecl(), getBasePriorit

[PATCH] D127629: [clang] Use correct visibility parameters when following a Using declaration

2022-06-13 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. Also if you put `Fixes https://github.com/clangd/clangd/issues/1137` in the commit message, it'll automatically close the issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127629/new/ https://reviews.llvm.org/D127629

[PATCH] D127749: [clangd] Wire up compilation for style blocks

2022-06-14 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: adamcz. Herald added subscribers: usaxena95, arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Repos

[PATCH] D127749: [clangd] Wire up compilation for style blocks

2022-06-14 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG3ecfeb4c2f45: [clangd] Wire up compilation for style blocks (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127749/new/ https://review

[PATCH] D127125: [clangd] Improve ObjC protocol suggestions from the index

2022-06-15 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127125/new/ https://reviews.llvm.org/D127125 __

[PATCH] D127832: [clangd] Always desugar type aliases in hover

2022-06-15 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: hokein. Herald added subscribers: jeroen.dobbelaere, usaxena95, arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-

[PATCH] D127833: [clangd] Enable AKA type printing by default

2022-06-15 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: hokein. Herald added subscribers: usaxena95, arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. This

[PATCH] D127629: [clang] Use correct visibility parameters when following a Using declaration

2022-06-15 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! do you have commit access or should i land this for you? if i should, can you provide an email address for attribution of the commit Repository: rG LLVM Github Monorepo

[PATCH] D127833: [clangd] Enable AKA type printing by default

2022-06-15 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGa67beef3acac: [clangd] Enable AKA type printing by default (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127833/new/ https://reviews

[PATCH] D127844: [clangd] Pull suppression logic into common path, apply for driver diagnostics

2022-06-15 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: ilya-biryukov. Herald added subscribers: usaxena95, arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang-tools-extra. Fixes https:/

[PATCH] D127844: [clangd] Pull suppression logic into common path, apply for driver diagnostics

2022-06-15 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 437109. kadircet added a comment. Just pass optional directly Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127844/new/ https://reviews.llvm.org/D127844 Files: clang-tools-extra/clangd/Diagnostics.cpp cla

[PATCH] D127856: [clangd] Support multiline semantic tokens

2022-06-15 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added reviewers: sammccall, nridge. Herald added subscribers: usaxena95, arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-ext

[PATCH] D127629: [clang] Use correct visibility parameters when following a Using declaration

2022-06-15 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG462def25ec13: [clang] Use correct visibility parameters when following a Using declaration (authored by furkanusta, committed by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D127832: [clangd] Always desugar type aliases in hover

2022-06-15 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 437148. kadircet marked 2 inline comments as done. kadircet added a comment. - Leave the Ctx refactoring out of the patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127832/new/ https://reviews.llvm.org/D12

[PATCH] D127832: [clangd] Always desugar type aliases in hover

2022-06-15 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7212977fbb41: [clangd] Always desugar type aliases in hover (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D127863: [clang] Dont print implicit forrange initializer

2022-06-15 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: usaxena95. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, ilya-biryukov. Herald added a project: clang. Fixes https://github.com/clangd/clangd/issues/1158 Repository: rG

[PATCH] D127863: [clang] Dont print implicit forrange initializer

2022-06-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG32805e60c9de: [clang] Dont print implicit forrange initializer (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D128197: [clangd] Handle initializers that contain =

2022-06-20 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: usaxena95, arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Re

[PATCH] D127856: [clangd] Support multiline semantic tokens

2022-06-20 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 438387. kadircet added a comment. Split highlights into multiple tokens rather than trimming Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127856/new/ https://reviews.llvm.org/D127856 Files: clang-tools-ext

[PATCH] D127856: [clangd] Support multiline semantic tokens

2022-06-20 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:948 +} else { + // If a tokens length is past the end of the line, it should be treated as + // if the token ends at the end of the line and will not wrap onto the ---

[PATCH] D128197: [clangd] Handle initializers that contain =

2022-06-20 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. kadircet marked an inline comment as done. Closed by commit rG1c92e06ded2d: [clangd] Handle initializers that contain = (authored by kadircet). Changed prior to commit: https://reviews.llvm.org/D128197?vs=438369&id=438392

[PATCH] D127856: [clangd] Support multiline semantic tokens

2022-06-21 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 438711. kadircet marked 4 inline comments as done. kadircet added a comment. Get rid of the copy in common case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127856/new/ https://reviews.llvm.org/D127856 Files

[PATCH] D127856: [clangd] Support multiline semantic tokens

2022-06-21 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:947 +Out->tokenModifiers = Tok.Modifiers; +Last = Tok; sammccall wrote: > this copy feels gratuitous, can we just use Tokens.back(), or do the copy in > the r

[PATCH] D128204: [clangd] Add fix-it for inserting IWYU pragma: keep

2022-06-21 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. as discussed offline I agree that we should have this, as no matter how hard we try there are going to be cases that we can't get right due to ADL/template instantiations or depending on 3rd party code that cannot be edited and also doesn't have relevant pragmas inside

[PATCH] D126859: [clangd] Validate clang-tidy CheckOptions in clangd config

2022-06-21 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. I also agree with the typo correction verdict. In theory there'll be two cases: - typo correction helps, in which case it'll be obvious from the warning itself. - typo correction doesn't help, because the option doesn't exist at all, we'll be just showing a random opt

[PATCH] D128204: [clangd] Add fix-it for inserting IWYU pragma: keep

2022-06-22 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. > I had another idea about offering the export pragma when in a header file but > I don't know if that's going too far There are definitely cases where this is conceptually "right" fix, but it's hard to guess that in clangd (well at least without having some codebase w

[PATCH] D128379: [clangd] Change the url for clang-tidy check documentation

2022-06-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. luckily this document links were introduced only recently, hence they didn't make it to clangd-14, but they'll start being around from clangd-15 and such changes will be breaking all the links in existing clangd's. i wonder if we should actually point these at releases

[PATCH] D122677: [prototype] include-cleaner library

2022-06-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. hi Serge, sorry for the silence here. we'll be moving this towards a production ready version over the next months (also releasing some comments i've forgotten to send, we've already discussed these offline so no need for action). we'll probably be leaving some todos/fi

[PATCH] D153769: [clangd] Implement the 'Organize Imports' source action. Fix include-cleaner findings in batch.

2023-10-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:810-815 + Server->applyTweak(Args.tweakID, + {Args.file.file().str(), + Args.selection, + Args.requestedActionKinds, +

[PATCH] D152900: [clangd] Update symbol collector to use include-cleaner.

2023-07-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks a lot for bearing with me, let's ship it! Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:91 +const Inclusion &Inc, ParsedAST &AST, +std::shared_pt

[PATCH] D155173: [clangd] Refine the workflow for diagnostic Fixits.

2023-07-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:671 + // FIMXE: this is tricky + llvm::StringRef(LSPDiag.Message) + .starts_with_insensitive(Diag.Message)) hokein wrote: > hokein wro

[PATCH] D155173: [clangd] Refine the workflow for diagnostic Fixits.

2023-07-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1014-1016 + llvm::ArrayRef LSPDiags = Params.context.diagnostics; + std::map + ToLSPDiagsIndex; we're already making a copy of `Params.context.diagnostics` when creati

[PATCH] D155173: [clangd] Refine the workflow for diagnostic Fixits.

2023-07-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1063 + OnlyFix->isPreferred = true; + if (LSPDiags.size() == 1 && LSPDiags.front().range == Sel

[PATCH] D155619: [clangd] Always run preamble indexing on a separate thread

2023-07-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: sammccall. Herald added a subscriber: arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. This has bee

[PATCH] D155619: [clangd] Always run preamble indexing on a separate thread

2023-07-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG036a1b2202cb: [clangd] Always run preamble indexing on a separate thread (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155619/new/ h

[PATCH] D155614: [clangd] Make an include always refer to itself. Background: clang-review expects all referents to have definition, declaration or reference(s).

2023-07-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:1365 // Add the #include line to the references list. ReferencesResult::Reference Result; Result.Loc.range = i guess this patch is not needed anymore, but one comment about th

[PATCH] D155619: [clangd] Always run preamble indexing on a separate thread

2023-07-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 541938. kadircet added a comment. Fix some tsan issues Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155619/new/ https://reviews.llvm.org/D155619 Files: clang-tools-extra/clangd/ClangdServer.cpp clang-too

[PATCH] D78038: [clangd] WIP: fix several bugs relating to include insertion

2023-07-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, i think this LG. Comment at: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:1697-1698 + )cpp"); + TU.HeaderFilename = "Foo.h"; + auto Symbols = T

[PATCH] D155392: [clangd] add a config knob to disable modules

2023-07-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/ModulesTests.cpp:34 +protected: + ModulesTest() : WithCfg(Config::Key, makeModuleConfig(GetParam())) {} +}; testing behaviour indirectly through config options cause some troubles in

[PATCH] D155385: [clangd] enable unused-include warnings for standard library headers

2023-07-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:75-77 +if (tooling::stdlib::Header::named(Inc.Written)) return true; return false; ---

[PATCH] D142967: [clangd] Introduce source.organizeImports code action.

2023-07-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet resigned from this revision. kadircet added a comment. Herald added a subscriber: kadircet. resigning in favor of https://reviews.llvm.org/D153769 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142967/new/ https://reviews.llvm.org/D142967

[PATCH] D153769: [clangd] Implement the 'Organize Imports' source action. Fix include-cleaner findings in batch.

2023-07-20 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. there are unittests for tweaks under `clang-tools-extra/clangd/unittests/tweaks/` can you create one for `OrganizeImports` there and test this in there? i've got another concern around triggering of this interaction. in theory code-action context has a `only` field, di

[PATCH] D153769: [clangd] Implement the 'Organize Imports' source action. Fix include-cleaner findings in batch.

2023-07-20 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. so as we were discussing offline (just to summarize it here) there's also the possibility of propagating context kind in selection, and making this code action fire when code actions are requested for "source code action kind" or the selection intersects with includes.

[PATCH] D133843: [clangd] Prefer definitions for gototype and implementation

2023-07-21 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet abandoned this revision. kadircet added a comment. abandoning in favor of D155898 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133843/new/ https://reviews.llvm.org/D133843 __

[PATCH] D133843: [clangd] Prefer definitions for gototype and implementation

2023-07-21 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG0a093f62d154: [clangd] Prefer definitions for gototype and implementation (authored by kadircet). Changed prior to commit: https://reviews.llvm.org/D133843?vs=460003&id=542874#toc Repository: rG LLVM

[PATCH] D156044: [clangd] Exclude builtin headers from system include extraction

2023-07-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks a lot! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156044/new/ https://reviews.llvm.org/D156044 __

[PATCH] D155992: [clangd] Use xxh3_64bits for background index file digests

2023-07-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. thanks for doing this! as Sam pointed out this will result in invalidation of all the index shards, but that's not something new. we already don't clean up non-relevant index shards when people delete sources over time and rely on people having new checkouts or cleari

[PATCH] D156122: [include-cleaner] Introduce support for always_keep pragma

2023-07-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: VitaNuo. Herald added subscribers: PiotrZSL, carlosgalvezp, arphaman. Herald added a reviewer: njames93. Herald added a project: All. kadircet requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subs

[PATCH] D156123: [include-cleaner] Unify always_keep with rest of the keep logic

2023-07-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: VitaNuo. Herald added subscribers: PiotrZSL, carlosgalvezp, arphaman. Herald added a reviewer: njames93. Herald added a project: All. kadircet requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subs

[PATCH] D156122: [include-cleaner] Introduce support for always_keep pragma

2023-07-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. FWIW, details of pragma are explained in https://github.com/include-what-you-use/include-what-you-use/blob/master/docs/IWYUPragmas.md#iwyu-pragma-always_keep Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156122/new/ https

[PATCH] D155619: [clangd] Always run preamble indexing on a separate thread

2023-07-24 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet closed this revision. kadircet added a comment. landed with 27ade4b554774187d2c0afcf64cd16fa6d5f619d Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155619/new/ https://r

[PATCH] D155819: [include-cleaner] Loose matching for verbatim headers

2023-07-26 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h:198 llvm::StringMap> BySpelling; + llvm::StringMap> BySpellingAlternate; llvm::DenseMap> ByFile; maybe add a comment saying that these spel

[PATCH] D155878: [clangd] Loose include-cleaner matching for verbatim headers

2023-07-26 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/Headers.cpp:185 + // The entries will be the same, but canonicalizing to find out is expensive! + if (SearchPathCanonical.empty()) { +for (const auto &Dir : nit: early exit =

[PATCH] D155878: [clangd] Loose include-cleaner matching for verbatim headers

2023-07-26 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Comment at: clang-tools-extra/clangd/Hover.cpp:1250 const SourceManager &SM = AST.getSourceManager(); - const auto &ConvertedMainFileIncludes = - convert

[PATCH] D156403: [clangd] Revert the symbol collector behavior to old pre-include-cleaner-library behavior due to a regression.

2023-07-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:841-847 auto [It, Inserted] = SymbolProviders.try_emplace(S.ID); if (Inserted) { auto Headers = include_cleaner::headersForSymbol(Sym, SM, Opts.PragmaIncludes);

[PATCH] D155819: [include-cleaner] Loose matching for verbatim headers

2023-07-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/include-cleaner/lib/Types.cpp:171-173 +for (unsigned I : BySpellingAlternate.lookup(Spelling)) + if (!llvm::is_contained(Result

[PATCH] D156403: [clangd] Revert the symbol collector behavior to old pre-include-cleaner-library behavior due to a regression.

2023-07-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:901 } +if (auto StdSym = tooling::stdlib::Symbol::named(S->Scope, S->Name, Lang)) + if (a

[PATCH] D156403: [clangd] Revert the symbol collector behavior to old pre-include-cleaner-library behavior due to a regression.

2023-07-27 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:842 - auto [It, Inserted] = SymbolProviders.try_emplace(S.ID); - if (Inserted) { -auto Headers = -include_cleaner::headersForSymbol(Sym, SM, Opts.PragmaIncludes); -i

[PATCH] D157610: [include-cleaner][clangd][clang-tidy] Ignore resource dir during include-cleaner analysis.

2023-08-10 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp:122 llvm::DenseSet SeenSymbols; + std::string ResourceDir = HS->getHeaderSearchOpts().ResourceDir; // FIXME: Find a way to have less code duplication between include-clean

[PATCH] D154503: [Sema] Fix handling of functions that hide classes

2023-08-14 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. In D154503#4576481 , @aaron.ballman wrote: > In D154503#4576475 , @kadircet > wrote: > >> In D154503#4576442 , >> @aaron.ballman wrote: >> >>>

[PATCH] D157868: [clang] Use RecoveryExprs for broken defaultargs, instead of OpaqueValueExprs

2023-08-14 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added reviewers: sammccall, aaron.ballman. Herald added a subscriber: arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added projects: clang, clang-tools-extra. Herald added a subscriber: cfe-commits. This makes sure

[PATCH] D157868: [clang] Use RecoveryExprs for broken defaultargs, instead of OpaqueValueExprs

2023-08-14 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 549939. kadircet marked 2 inline comments as done. kadircet added a comment. - Use Expr* instead of ExprResult - Add dump test to demonstrate new RecoveryExpr Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D15786

[PATCH] D157868: [clang] Use RecoveryExprs for broken defaultargs, instead of OpaqueValueExprs

2023-08-14 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang/include/clang/Sema/Sema.h:3026 + void ActOnParamDefaultArgumentError(Decl *param, SourceLocation EqualLoc, + ExprResult DefaultArg); ExprResult ConvertParamDefaultArgument(ParmVarDecl *Para

[PATCH] D157610: [include-cleaner][clangd][clang-tidy] Ignore resource dir during include-cleaner analysis.

2023-08-14 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp:122 llvm::DenseSet SeenSymbols; + std::string ResourceDir = HS->getHeaderSearchOpts().ResourceDir; // FIXME: Find a way to have less code duplication between include-clean

[PATCH] D157905: [include-cleaner] Filter references to identity macros

2023-08-14 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: sammccall. Herald added a project: All. kadircet requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Despite being true positives, these results just confuse users. So filte

[PATCH] D157207: [clangd] Fix typo in comment

2023-08-14 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG09a622baa2d8: [clangd] Fix typo in comment (authored by Eymay, committed by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157207/new/ https://rev

[PATCH] D157868: [clang] Use RecoveryExprs for broken defaultargs, instead of OpaqueValueExprs

2023-08-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. kadircet marked 3 inline comments as done. Closed by commit rG373fcd5d73a3: [clang] Use RecoveryExprs for broken defaultargs, instead of OpaqueValueExprs (authored by k

[PATCH] D157905: [include-cleaner] Filter references to identity macros

2023-08-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 550662. kadircet marked an inline comment as done. kadircet added a comment. - Rename helper, update comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157905/new/ https://reviews.llvm.org/D157905 Files:

[PATCH] D157905: [include-cleaner] Filter references to identity macros

2023-08-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet marked an inline comment as done. kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/Analysis.cpp:43 + // This results in surprising behavior from users point of view (we + // generate a usage of stdio.h, in places unrelated to st

[PATCH] D158093: [Sema] Clean up ActionResult type a little. NFCI

2023-08-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks a lot for doing this, i think this makes it a lot more obvious that this is a tristate struct rather than invalid just being a bit that can bit set for both null and non-null pointe

[PATCH] D157905: [include-cleaner] Filter references to identity macros

2023-08-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet marked 2 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/Analysis.cpp:43 + // This results in surprising behavior from users point of view (we + // generate a usage of stdio.h, in places unrelated to st

[PATCH] D157905: [include-cleaner] Filter references to identity macros

2023-08-16 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 550790. kadircet added a comment. Herald added subscribers: PiotrZSL, carlosgalvezp, arphaman. Herald added a reviewer: njames93. - Apply filtering only to macros that expand to themselves Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[PATCH] D158269: [clang] Prevent possible use-after-free

2023-08-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added reviewers: sammccall, ilya-biryukov. Herald added a project: All. kadircet requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This prevents further parsing of tokens (that'll be freed) inside meth

[PATCH] D157905: [include-cleaner] Filter references to identity macros

2023-08-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. kadircet marked an inline comment as done. Closed by commit rG90ecadde62f3: [include-cleaner] Filter references to identity macros (authored by kadircet). Changed prior to commit: https://reviews.llvm.org/D157905?vs=55079

[PATCH] D158269: [clang] Prevent possible use-after-free

2023-08-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. kadircet marked an inline comment as done. Closed by commit rG851c248dfcdb: [clang] Prevent possible use-after-free (authored by kadircet). Changed prior to commit:

[PATCH] D158426: [clangd] Bump timeouts for LSPServerTests

2023-08-21 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added reviewers: sammccall, nridge. Herald added a subscriber: arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. We se

[PATCH] D158426: [clangd] Bump timeouts for LSPServerTests

2023-08-22 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 552284. kadircet marked 2 inline comments as done. kadircet added a comment. - Just bump the timeouts Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158426/new/ https://reviews.llvm.org/D158426 Files: clang-

[PATCH] D158515: [include-cleaner] Make handling of enum constants similar to members

2023-08-22 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: sammccall. Herald added a project: All. kadircet requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. We were treating enum constants more like regular decls, which results i

[PATCH] D158426: [clangd] Bump timeouts for LSPServerTests

2023-08-22 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG8c21544286a0: [clangd] Bump timeouts for LSPServerTests (authored by kadircet). Changed prior to commit: https://reviews.llvm.org/D158426?vs=552284&id=552354#toc Repository: rG LLVM Github Monorepo

[PATCH] D158515: [include-cleaner] Make handling of enum constants similar to members

2023-08-22 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 552391. kadircet added a comment. - Add test for using enums - Drop implicit references from qualified names to their containers, as we should already have explicit references from the spelling of the qualifier. Repository: rG LLVM Github Monorepo CHANG

[PATCH] D158515: [include-cleaner] Make handling of enum constants similar to members

2023-08-22 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 552392. kadircet added a comment. - Change to c++20 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158515/new/ https://reviews.llvm.org/D158515 Files: clang-tools-extra/include-cleaner/lib/WalkAST.cpp clan

[PATCH] D150124: [index][clangd] Consider labels when indexing function bodies

2023-07-28 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. sorry for missing this review. Nathan's explanation around `targetDecl` vs `findRefs` is absolutely right (btw, thanks a lot Nathan for taking good care of clangd, I don't think I say that enough). hopefully one day we can switch clangd's usage of libindex to our inter

[PATCH] D156650: [clangd] Respect IWYU keep pragma for standard headers.

2023-07-31 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:85-86 if (PI) { if (PI->shouldKeep(Inc.HashLine + 1)) return false; // Check if main f

[PATCH] D156648: [Tooling/Inclusion] Add std::range symbols in the mapping.

2023-07-31 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. can you also create a cherry-pick request for this patch once it lands? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156648/new/ https://re

[PATCH] D156650: [clangd] Respect IWYU keep pragma for standard headers.

2023-07-31 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. also we should get this cherry-picked too. `keep` pragmas on includes are not common, but people do have export pragmas often enough to cause some annoyance here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156650/new/

[PATCH] D156712: [include-cleaner] Handle StdInitializerListExprs

2023-07-31 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision. kadircet added a reviewer: hokein. Herald added a project: All. kadircet requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Per C++ standard, programs omitting the definition for initializer_list is

[PATCH] D156712: [include-cleaner] Handle StdInitializerListExprs

2023-08-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG3736eaa6a0b8: [include-cleaner] Handle StdInitializerListExprs (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156712/new/ https://rev

[PATCH] D156122: [include-cleaner] Introduce support for always_keep pragma

2023-08-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet marked an inline comment as done. kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h:63 + bool shouldKeep(unsigned HashLineNumber) const; + bool shouldKeep(const FileEntry *FE) const;

[PATCH] D156122: [include-cleaner] Introduce support for always_keep pragma

2023-08-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 546001. kadircet marked an inline comment as done. kadircet added a comment. - Rebase - Properly check for null-ness of FileEntry. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156122/new/ https://reviews.llvm

[PATCH] D156123: [include-cleaner] Unify always_keep with rest of the keep logic

2023-08-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 546070. kadircet added a comment. - Add tests for pragmas on stdlib headers Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156123/new/ https://reviews.llvm.org/D156123 Files: clang-tools-extra/clang-tidy/mis

[PATCH] D156123: [include-cleaner] Unify always_keep with rest of the keep logic

2023-08-01 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/Record.cpp:248 + if (Top.SeenAtFile == SM.getMainFileID() && IncludedFile) +Out->ShouldKeep.insert(IncludedFile->getUniqueID()); } VitaNuo wrote: > If I understand

[PATCH] D156122: [include-cleaner] Introduce support for always_keep pragma

2023-08-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG778a5e9bc633: [include-cleaner] Introduce support for always_keep pragma (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156122/new/ h

[PATCH] D156123: [include-cleaner] Unify always_keep with rest of the keep logic

2023-08-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG43974333dd09: [include-cleaner] Unify always_keep with rest of the keep logic (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES

<    23   24   25   26   27   28   29   30   31   32   >