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

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] D153769: [clangd] Implement the 'Organize Imports' source action. Fix include-cleaner findings in batch.

kadircet added a comment. FYI, i don't think you uploaded the new version of the patch Comment at: clang-tools-extra/clangd/ClangdServer.cpp:752 auto Action = [File = File.str(), Sel, TweakID = TweakID.str(), - CB = std::move(CB), + CB =

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

kadircet added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:752 auto Action = [File = File.str(), Sel, TweakID = TweakID.str(), - CB = std::move(CB), + CB = std::move(CB), ActionKinds, this](Expected

[PATCH] D156659: [clangd] Rollforward include-cleaner library usage in symbol collector.

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

[PATCH] D156659: [clangd] Rollforward include-cleaner library usage in symbol collector.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:828 - tooling::stdlib::Lang Lang = tooling::stdlib::Lang::CXX; -if (LangOpts.C11) - Lang = tooling::stdlib::Lang::C; sorry i got confused, this also works

[PATCH] D156659: [clangd] Rollforward include-cleaner library usage in symbol collector.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:912 +if (Directives & Symbol::Import) { + if (auto IncludeHeader = HeaderFileURIs->getIncludeHeader(FID); + !IncludeHeader.empty()) { VitaNuo wrote:

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

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Comment at: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp:587 + TU.ExtraArgs.push_back(testPath("resources")); +

[PATCH] D158566: Add CLANGD_INCLUDE_TESTS as a separate flag to control clangd tests

kadircet added a comment. hi @hiraditya , i believe your issues should disappear starting with 9a26d2c6d35f574d7a4b06a5a22f8a1c063cb664 . LMK if you're still facing problems and want to move forward with such a patch

[PATCH] D159441: [include-cleaner] Weaken signal for boosting preferred headers

This revision was automatically updated to reflect the committed changes. Closed by commit rG73b2c86d95dc: [include-cleaner] Weaken signal for boosting preferred headers (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D159441: [include-cleaner] Weaken signal for boosting preferred headers

kadircet added a comment. see also https://github.com/llvm/llvm-project/issues/62172 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159441/new/ https://reviews.llvm.org/D159441 ___ cfe-commits mailing

[PATCH] D159263: [clang-tidy] misc-include-cleaner: avoid duplicated fixes

kadircet added a comment. In D159263#4638199 , @danix800 wrote: > If we have further plans from upstream on these checker/clang-format related > logic, I can wait and abandon this revision. Not at the moment, so please don't! > Or if such issue does

[PATCH] D159263: [clang-tidy] misc-include-cleaner: avoid duplicated fixes

kadircet added a comment. Thanks a lot for the patch @danix800 ! I initially was rather focused on behavior of this check in workflows that require seeing "self-contained-diags", but also I rather see the bulk-apply use cases as always requiring clang-format afterwards. Hence didn't really try

[PATCH] D159441: [include-cleaner] Weaken signal for boosting preferred headers

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. Putting preferred header signal above completeness implied we would

[PATCH] D156659: [clangd] Rollforward include-cleaner library usage in symbol collector.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:840 -if (S->Scope == "std::" && S->Name == "move") { - if (!S->Signature.contains(',')) -return ""; - return ""; -} - -if (auto StdSym =

[PATCH] D159363: [clangd] SIGSEGV at clangd: DiagnosticConsumer Is Used After Free

kadircet added a comment. thanks, the fix LGTM as well. but i wonder how this surfaces, to make sure we're taking necessary precautions in the future. we definitely have a dangling reference, which isn't great. but it's surprising that we access diags consumer during indexing. I assume it's

[PATCH] D159363: [clangd] SIGSEGV at clangd: DiagnosticConsumer Is Used After Free

kadircet added a comment. thanks, the fix LGTM as well. but i wonder how this surfaces, to make sure we're taking necessary precautions in the future. we definitely have a dangling reference, which isn't great. but it's surprising that we access diags consumer during indexing. I assume it's

[PATCH] D158566: Add CLANGD_INCLUDE_TESTS as a separate flag to control clangd tests

kadircet added a comment. In D158566#4616417 , @ilya-biryukov wrote: > Open question: I also feel like the best option here is to fix the tests, but > I'm not sure how hard that would be. @sammccall any thoughts? > I suspect the particular tests are

[PATCH] D159338: [clangd][tests] Bump timeouts in TUSchedulerTests to 60 secs

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG16b8b3e59f7e: [clangd][tests] Bump timeouts in TUSchedulerTests to 60 secs (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D159337: [clangd][tests] Assert for idleness of scheduler

This revision was automatically updated to reflect the committed changes. Closed by commit rG69feef0e8277: [clangd][tests] Assert for idleness of scheduler (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159337/new/

[PATCH] D159338: [clangd][tests] Bump timeouts in TUSchedulerTests to 60 secs

kadircet created this revision. kadircet added reviewers: sammccall, nridge. Herald added subscribers: arphaman, javed.absar. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project:

[PATCH] D159337: [clangd][tests] Assert for idleness of scheduler

kadircet created this revision. kadircet added reviewers: sammccall, nridge. Herald added subscribers: arphaman, javed.absar. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project:

[PATCH] D157963: [clang-format] Annotate constructor/destructor names

kadircet added a comment. In D157963#4621206 , @HazardyKnusperkeks wrote: > @kadircet shouldn't you at least say why it got reverted? Even better state > your problem and give a chance to fix before you revert? Hi,

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

This revision was automatically updated to reflect the committed changes. kadircet marked 2 inline comments as done. Closed by commit rGab090e9e49ff: [include-cleaner] Make handling of enum constants similar to members (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE

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

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

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

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

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

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=552354#toc Repository: rG LLVM Github Monorepo

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

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

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

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:

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

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

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

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] D157905: [include-cleaner] Filter references to identity macros

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:

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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/

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

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

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

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

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

kadircet added inline comments. Comment at: clang/include/clang/Sema/Sema.h:3026 + void ActOnParamDefaultArgumentError(Decl *param, SourceLocation EqualLoc, + ExprResult DefaultArg); ExprResult ConvertParamDefaultArgument(ParmVarDecl

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

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

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

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

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

kadircet added a comment. In D154503#4576481 , @aaron.ballman wrote: > In D154503#4576475 , @kadircet > wrote: > >> In D154503#4576442 , >> @aaron.ballman wrote: >>

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

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

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

kadircet added a comment. In D154503#4576442 , @aaron.ballman wrote: > I'm not opposed to a revert if this is causing problems for your downstream, > but be sure to also remove the changes from the 17.x branch if you go this > route rather than fixing

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

kadircet added a comment. Hi! After this patch clang seems to be crashing on: class a { static c a; void b(){a}; }; with stack trace and assertion error: $ ~/repos/llvm/build/bin/clang -xc++ repro.cc repro.cc:2:10: error: unknown type name 'c' 2 | static c a; |

[PATCH] D157395: [include-cleaner] Follow `IWYU pragma: export` links transitively.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. > Order of exporters is from outermost to innermost. FWIW, that's mostly a coincidence. all 3 exporters are private headers, hence they're penalized for it. Afterwards "export1.h" is

[PATCH] D156704: [clang][HeaderSearch] Treat framework headers as System for suggestPath

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

[PATCH] D157395: [include-cleaner] Follow `IWYU pragma: export` links transitively.

kadircet added a comment. i think we should also have some unittests in FindHeadersTest, some sample scenarios: foo.h #pragma once void foo(); export1.h #pragma once // IWYU pragma: private, include "public1.h" #include "foo.h" // IWYU pragma: export export2.h #pragma once //

[PATCH] D157400: [include-cleaner] Dont boost private headers beyond public ones

This revision was automatically updated to reflect the committed changes. Closed by commit rG0a315be2a46f: [include-cleaner] Dont boost private headers beyond public ones (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D157400: [include-cleaner] Dont boost private headers beyond public ones

kadircet created this revision. kadircet added a reviewer: VitaNuo. Herald added a project: All. kadircet requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Private headers should be the last resort, even if they match the name of

[PATCH] D157390: [clang-tidy][include-cleaner] Add option to control deduplication of findings per symbol

kadircet marked 2 inline comments as done. kadircet added a comment. hi @PiotrZSL, addressed those in 724b40a1379f7c116473a02a3cec4d341c475b46 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D157390: [clang-tidy][include-cleaner] Add option to control deduplication of findings per symbol

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG89d0a76be68b: [clang-tidy][include-cleaner] Add option to control deduplication of findings… (authored by kadircet). Repository: rG LLVM Github

[PATCH] D157390: [clang-tidy][include-cleaner] Add option to control deduplication of findings per symbol

kadircet updated this revision to Diff 548199. kadircet added a comment. - Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157390/new/ https://reviews.llvm.org/D157390 Files: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp

[PATCH] D157390: [clang-tidy][include-cleaner] Add option to control deduplication of findings per symbol

kadircet updated this revision to Diff 548190. kadircet marked 2 inline comments as done. kadircet added a comment. Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157390/new/ https://reviews.llvm.org/D157390 Files:

[PATCH] D157390: [clang-tidy][include-cleaner] Add option to control deduplication of findings per symbol

kadircet created this revision. kadircet added a reviewer: VitaNuo. Herald added subscribers: PiotrZSL, carlosgalvezp, xazax.hun. 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

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

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. Thanks! LMK if i should land this for you (and an email address to set as commit author) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D157078: [include-cleaner] Handle files with unnamed buffers

This revision was automatically updated to reflect the committed changes. Closed by commit rG31873d3fca38: [include-cleaner] Handle files with unnamed buffers (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157078/new/

[PATCH] D157071: [clangd] Dont assert on specific uris for diagnostics docs

This revision was automatically updated to reflect the committed changes. Closed by commit rG734da23e21eb: [clangd] Dont assert on specific uris for diagnostics docs (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157071/new/

[PATCH] D157078: [include-cleaner] Handle files with unnamed buffers

kadircet created this revision. kadircet added a reviewer: hokein. Herald added a subscriber: ChuanqiXu. Herald added a project: All. kadircet requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Some tools can register virtual

[PATCH] D157071: [clangd] Dont assert on specific uris for diagnostics docs

kadircet created this revision. kadircet added a reviewer: hokein. 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. To enable

[PATCH] D156704: [clang][HeaderSearch] Treat framework headers as System for suggestPath

kadircet added inline comments. Comment at: clang/lib/Lex/HeaderSearch.cpp:1939 std::string HeaderSearch::suggestPathToFileForDiagnostics( llvm::StringRef File, llvm::StringRef WorkingDir, llvm::StringRef MainFile, sorry I guess my suggestion get

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

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

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

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/

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

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

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

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:

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

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/

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

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] D156712: [include-cleaner] Handle StdInitializerListExprs

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/

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

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] D156650: [clangd] Respect IWYU keep pragma for standard headers.

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] D156648: [Tooling/Inclusion] Add std::range symbols in the mapping.

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/

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

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

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

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

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

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); -

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

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

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

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

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

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] D155878: [clangd] Loose include-cleaner matching for verbatim headers

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 = AST.getSourceManager(); - const auto = - convertIncludes(SM,

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

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 : nit: early exit

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

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

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

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/

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

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/

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

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

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

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

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

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

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

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] D133843: [clangd] Prefer definitions for gototype and implementation

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=542874#toc Repository: rG LLVM

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

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] D153769: [clangd] Implement the 'Organize Imports' source action. Fix include-cleaner findings in batch.

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

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

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,

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

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] D155385: [clangd] enable unused-include warnings for standard library headers

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] D155392: [clangd] add a config knob to disable modules

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] D78038: [clangd] WIP: fix several bugs relating to include insertion

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 =

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

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

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

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

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

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/

  1   2   3   4   5   6   7   8   9   10   >