[PATCH] D147139: [clangd] Map references from include'd files to directives

This revision was automatically updated to reflect the committed changes. Closed by commit rGf3a815aa776f: [clangd] Map references from include'd files to directives (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147139/new/ h

[PATCH] D147044: [clangd] Implement cross reference request for #include lines.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:1338 + +auto ReferencedInclude = convertIncludes(SM, Inc); +include_cleaner::walkUsed( can we put the rest into a separate function (I know this function is already quite long

[PATCH] D147044: [clangd] Implement cross reference request for #include lines.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:443 + // No match for this provider in the includes list. + return {}; +} `return std::nullopt` Comment at: clang-tools-extra/clangd/IncludeCleaner.h:87 +

[PATCH] D147144: [include-cleaner] Report references to operator calls as implicit

kadircet updated this revision to Diff 510474. kadircet marked an inline comment as done. kadircet added a comment. - Update to treat operator calls to members as "explicit" fater offline discussions. This better aligns with what we do for regular member exprs. Repository: rG LLVM Github Mono

[PATCH] D147144: [include-cleaner] Report references to operator calls as implicit

This revision was automatically updated to reflect the committed changes. Closed by commit rG26ff268b80c5: [include-cleaner] Report references to operator calls as implicit (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147144/

[PATCH] D147449: [include-cleaner] Only ignore builtins without a header

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. Certain standard library functions (e.g. std::move) are also implemented as

[PATCH] D147449: [include-cleaner] Only ignore builtins without a header

This revision was automatically updated to reflect the committed changes. Closed by commit rG3402b77db3ee: [include-cleaner] Only ignore builtins without a header (authored by kadircet). Changed prior to commit: https://reviews.llvm.org/D147449?vs=510516&id=510746#toc Repository: rG LLVM Git

[PATCH] D147686: [clangd] Fix a nullptr-dereference crash in computeIncludeCleanerFindings.

kadircet added a comment. could you give some more details about the crash, so that we actually now where to fix it? e.g. is it token buffers missing the spelled token at that offset somehow ? or is it AST reporting a weird source locations for user-defined literals and we need to do some sort

[PATCH] D148112: [include-cleaner] Improve handling for templates

kadircet created this revision. Herald added a subscriber: mgrang. Herald added a project: All. kadircet requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Principal here is: - Making sure each template instantiation implies use o

[PATCH] D148112: [include-cleaner] Improve handling for templates

kadircet updated this revision to Diff 512775. kadircet edited the summary of this revision. kadircet added a comment. - Use template instantion pattern helper instead of dealing with partial specializations Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D148112: [include-cleaner] Improve handling for templates

kadircet updated this revision to Diff 512851. kadircet marked an inline comment as done. kadircet added a comment. Herald added a subscriber: ChuanqiXu. - Ignore explicit instantiations - Update tests to use decls Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[PATCH] D148112: [include-cleaner] Improve handling for templates

kadircet marked 3 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp:109 ADD_FAILURE() << DiagBuf << "\nfrom code:\n" << ReferencingCode; + llvm::sort(TargetDeclKinds); + TargetDeclKinds.erase(ll

[PATCH] D148112: [include-cleaner] Improve handling for templates

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG34f5774920f5: [include-cleaner] Improve handling for templates (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D148143: [clangd] Treat preamble patch as main file for include-cleaner analysis

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. Since we red

[PATCH] D148143: [clangd] Treat preamble patch as main file for include-cleaner analysis

kadircet updated this revision to Diff 512898. kadircet added a comment. - Expose helper to get preamble patch entry Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148143/new/ https://reviews.llvm.org/D148143 Files: clang-tools-extra/clangd/Inclu

[PATCH] D148112: [include-cleaner] Improve handling for templates

kadircet added a comment. caea93cc4478cca28321cba4fa2871d5e6090299 should fix the issue Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148112/new/ https://reviews.llvm.org/D1481

[PATCH] D148143: [clangd] Treat preamble patch as main file for include-cleaner analysis

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 rG144562e678d9: [clangd] Treat preamble patch as main file for include-cleaner analysis (authored by kadirce

[PATCH] D148158: [include-cleaner] Handle incomplete template specializations

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. Instantiation pattern is null for incomplete template types and using sp

[PATCH] D146941: [clangd] Use all inputs to SystemIncludeExtractor in cache key

kadircet marked 3 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/SystemIncludeExtractor.cpp:80 + std::string Driver; + std::string Directory; + // Whether certain includes should be part of query. nridge wrote: >

[PATCH] D146941: [clangd] Use all inputs to SystemIncludeExtractor in cache key

kadircet updated this revision to Diff 513070. 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/D146941/new/ https://reviews.llvm.org/D146941 Files: clang-tools-

[PATCH] D147686: [clangd] Fix a nullptr-dereference crash in computeIncludeCleanerFindings.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp:450 +void test() { + 1s; +} hokein wrote: > here is the `UserDefinedLiteral` AST node: > > ``` > `-UserDefinedLiteral 0x5556682e4500 'int' >

[PATCH] D147905: [clangd] Avoid passing -xobjective-c++-header to the system include extractor

kadircet added inline comments. Comment at: clang-tools-extra/clangd/SystemIncludeExtractor.cpp:340 +// is not installed. +if (Lang == "objective-c++-header") { + Lang = "c++-header"; this feels like too much of a layering violation and might (will?)

[PATCH] D146941: [clangd] Use all inputs to SystemIncludeExtractor in cache key

kadircet updated this revision to Diff 513098. kadircet marked an inline comment as done. kadircet added a comment. Fix typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146941/new/ https://reviews.llvm.org/D146941 Files: clang-tools-extra/clan

[PATCH] D148158: [include-cleaner] Handle incomplete template specializations

kadircet updated this revision to Diff 513173. kadircet marked 2 inline comments as done. kadircet added a comment. - Update comments & test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148158/new/ https://reviews.llvm.org/D148158 Files: clang-

[PATCH] D148158: [include-cleaner] Handle incomplete template specializations

kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp:163 ElementsAre(Decl::ClassTemplatePartialSpecialization)); + // Incomplete templates don't have a specific specialization associated. + EXPECT_THAT(testWalk(R

[PATCH] D148158: [include-cleaner] Handle incomplete template specializations

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3d6d2ae6f490: [include-cleaner] Handle incomplete template specializations (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D148213: [clangd] Use FileEntryRef for canonicalizing filepaths.

kadircet added a comment. we've got one more reference to `getCanonicalPath` in `clang-tools-extra/clangd/IncludeCleaner.cpp:320`, i guess the best way is just calling `getLastRef` on the FileEntry* Comment at: clang-tools-extra/clangd/SourceCode.cpp:518

[PATCH] D148213: [clangd] Use FileEntryRef for canonicalizing filepaths.

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

[PATCH] D147905: [clangd] Avoid passing -xobjective-c++-header to the system include extractor

kadircet added inline comments. Comment at: clang-tools-extra/clangd/SystemIncludeExtractor.cpp:340 +// is not installed. +if (Lang == "objective-c++-header") { + Lang = "c++-header"; nridge wrote: > kadircet wrote: > > this feels like too much of a

[PATCH] D143260: [clangd] Add semantic token for labels

kadircet added a comment. sorry for long silence. > But access specifiers are a completely different thing semantically, that's > the point: The user does not tell the client: "I want everything that is > followed by a single colon in this color"; that would be silly. They say "I > want goto

[PATCH] D147802: [clangd] Handle destructors in DefineOutline tweak

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:186-194 + if (const auto *Destructor = llvm::dyn_cast(FD)) { +if (auto Err = Declarati

[PATCH] D147808: [clangd] Support defaulted destructors in Define Outline tweak

kadircet added a comment. there's actually a slight difference between an inline defaulted special member function and an out-of-line defaulted one. the latter makes the special member "user-defined" which might cause various headaches (e.g. type is no longer "trivial"). i don't think we should

[PATCH] D146941: [clangd] Use all inputs to SystemIncludeExtractor in cache key

This revision was automatically updated to reflect the committed changes. Closed by commit rGf099f2fefbab: [clangd] Use all inputs to SystemIncludeExtractor in cache key (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146941/new

[PATCH] D147684: [clangd] Add batch fixes for include-cleaner diagnostics

kadircet added inline comments. Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:418 +Fix removeAllUnusedIncludes(llvm::ArrayRef UnusedIncludes) { + Fix RemoveAll; can we also derive these from an `llvm::ArrayRef` ? to make sure there can't be a discre

[PATCH] D148088: [RFC][clangd] Move preamble index out of document open critical path

kadircet added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:90 +if (Tasks) { + std::unique_lock Lock(*Barrier, std::try_to_lock); + Tasks->runAsync("task:" + Path + Version, std::move(Task)); DmitryPolukhin wrote: > kadirce

[PATCH] D147808: [clangd] Support non-trivial defaulted special member functions in Define Outline tweak

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks a lot, and sorry for the delay! Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:333 +[](const syntax::Token &Tok) { return Tok.kind(

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

kadircet added a comment. In D150997#4357589 , @nikic wrote: > It looks like you forgot to `git add` the new file maybe? > > This was added in https://reviews.llvm.org/D133200, maybe @kadircet can > comment on what this was intended for. It was already

[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

kadircet added a comment. In D150997#4357589 , @nikic wrote: > It looks like you forgot to `git add` the new file maybe? > > This was added in https://reviews.llvm.org/D133200, maybe @kadircet can > comment on what this was intended for. It was already

[PATCH] D151073: [clang] Fix label (de-)serialization in ASM statements.

kadircet added inline comments. Comment at: clang/test/PCH/asm-label.cpp:5 +#define HEADER_H +#pragma once +void MyMethod() { you can drop `#pragma once` know, as it won't be parsed again anyways Comment at: clang/test/PCH/asm-label.cpp:6 +#pra

[PATCH] D151185: [clangd] Store paths as requested in PreambleStatCache

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. Underlying FS c

[PATCH] D151185: [clangd] Store paths as requested in PreambleStatCache

This revision was automatically updated to reflect the committed changes. Closed by commit rG35ce741ef3e3: [clangd] Store paths as requested in PreambleStatCache (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151185/new/ https

[PATCH] D149948: [include-cleaner] Treat references to nested types implicit

This revision was automatically updated to reflect the committed changes. kadircet marked an inline comment as done. Closed by commit rGcce7b816a1ee: [include-cleaner] Treat references to nested types implicit (authored by kadircet). Changed prior to commit: https://reviews.llvm.org/D149948?vs=

[PATCH] D151303: [clangd] Fix add-using tweak on declrefs with template arguments

kadircet created this revision. kadircet added a reviewer: VitaNuo. 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. Repository:

[PATCH] D151303: [clangd] Fix add-using tweak on declrefs with template arguments

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG51df1a9ac96f: [clangd] Fix add-using tweak on declrefs with template arguments (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D151321: [clangd] Dont run raw-lexer for OOB source locations

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. We can get stal

[PATCH] D148793: [WIP][clang-tidy] Implement an include-cleaner check.

kadircet added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp:61 +void IncludeCleanerCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "Suppress", ""); +} after storing them in the class state

[PATCH] D151321: [clangd] Dont run raw-lexer for OOB source locations

This revision was automatically updated to reflect the committed changes. Closed by commit rGa2f7352f3e80: [clangd] Dont run raw-lexer for OOB source locations (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151321/new/ https:/

[PATCH] D151294: [clangd] Remove inline Specifier for DefineOutline Tweak

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, LGTM! Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:136 +// Removes matching instances of given token preceeding the function defition. +

[PATCH] D151294: [clangd] Remove inline Specifier for DefineOutline Tweak

This revision was automatically updated to reflect the committed changes. Closed by commit rGfe7afcf70c93: [clangd] Remove inline Specifier for DefineOutline Tweak (authored by bgluzman, committed by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D148793: [WIP][clang-tidy] Implement an include-cleaner check.

kadircet added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp:42 + +struct MissingIncludeInfo { + SourceLocation SymRefLocation; let's put this struct into anon namespace Comment at: clang-tools-extra/cl

[PATCH] D150185: [include-cleaner] Allow multiple strategies for spelling includes.

kadircet added a comment. thanks a lot! outline seems good. mostly some comments on implementation details. Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h:92 + + virtual std::string operator()(llvm::StringRef HeaderPhysicalPath) const

[PATCH] D142890: [clangd] Add config option for fast diagnostics mode

kadircet updated this revision to Diff 495407. kadircet added a comment. - Update tests after discussions in D143096 to be line-oriented, rather than being directive-oriented. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D143093: [clangd] #undef macros inside preamble patch

kadircet updated this revision to Diff 495409. kadircet added a comment. - Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143093/new/ https://reviews.llvm.org/D143093 Files: clang-tools-extra/clangd/Preamble.cpp clang-tools-extra/clangd/

[PATCH] D143095: [clangd] Respect preamble-patch when handling diags

kadircet updated this revision to Diff 495410. kadircet added a comment. - Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143095/new/ https://reviews.llvm.org/D143095 Files: clang-tools-extra/clangd/Diagnostics.cpp clang-tools-extra/clan

[PATCH] D143096: [clangd] Provide patched diagnostics with preamble patch

kadircet marked an inline comment as done. kadircet added a comment. In D143096#4099662 , @sammccall wrote: > It looks like this fixes up the location only of diagnostics attached to > particular directives (`#include`) based on some "deep" idea about th

[PATCH] D143096: [clangd] Provide patched diagnostics with preamble patch

kadircet updated this revision to Diff 495411. kadircet added a comment. - Change to a line based translation logic Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143096/new/ https://reviews.llvm.org/D143096 Files: clang-tools-extra/clangd/Parsed

[PATCH] D143486: [clangd] Fix a crash in semantic highlighting.

kadircet accepted this revision. kadircet added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:524 const auto *Tok = TB.spelledTokenAt(Loc); -assert(Tok); - +if (!Tok) + return std:

[PATCH] D143197: [clangd] Fix bugs in main-file include patching for stale preambles

kadircet updated this revision to Diff 495790. kadircet marked 4 inline comments as done. kadircet retitled this revision from "[clangd] Patch includes even without any changes" to "[clangd] Fix bugs in main-file include patching for stale preambles". kadircet edited the summary of this revision.

[PATCH] D143197: [clangd] Fix bugs in main-file include patching for stale preambles

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Preamble.cpp:690 +// it's coming from baseline preamble. +if (It->second) { + Inc.Resolved = It->second->Resolved; sammccall wrote: > why the null check? as discussed of

[PATCH] D143214: [include-mapping] Add C-compatibility symbol entries.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/StdLibTests.cpp:37 EXPECT_THAT(CXX, HasSubstr("#include ")); - EXPECT_THAT(CXX, Not(HasSubstr("#include "))); + EXPECT_THAT(CXX, HasSubstr("#include ")); hokein wrote: > This is

[PATCH] D143569: [Tooling/Inclusions] Add more multi-header symbols to StdSpecialSymbolMap.inc

kadircet accepted this revision. kadircet added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Tooling/Inclusions/Stdlib/StdSpecialSymbolMap.inc:37 +SYMBOL(swap, std::, ) // since C++17 +SYMBOL(swap, std::, ) // until C++11 +// C++ [tuple.

[PATCH] D143559: [Tooling/Inclusion] Use the StdSpecialSymbolMap.inc in the stdlib

kadircet accepted this revision. kadircet added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/Tooling/StandardLibraryTest.cpp:65 stdlib::Header::named(""))); + EXPECT_THAT(stdlib::Symbol::named("std::", "

[PATCH] D142890: [clangd] Add config option for fast diagnostics mode

kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/PreambleTests.cpp:624 + + llvm::StringLiteral BaselinePreamble = "#define FOO\n"; + { sammccall wrote: > nit: "preamble" vs "code" is a confusing distinction when we're using both as

[PATCH] D142890: [clangd] Add config option for fast diagnostics mode

kadircet updated this revision to Diff 495879. kadircet marked 10 inline comments as done. kadircet added a comment. - Use raw string literals - Make tests more expressive by mentioning diagnostic names Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D143197: [clangd] Fix bugs in main-file include patching for stale preambles

kadircet updated this revision to Diff 495884. kadircet marked 2 inline comments as done. kadircet added a comment. - use rawstrings in test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143197/new/ https://reviews.llvm.org/D143197 Files: clang-

[PATCH] D143597: [clangd] Drop includes from disabled PP regions in preamble patch

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. In rest of t

[PATCH] D143197: [clangd] Fix bugs in main-file include patching for stale preambles

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Preamble.cpp:691 +// it's coming from baseline preamble. +if (It->second) + PatchedInc = *It->second; sammccall wrote: > if It->second is null, then all the `#includes`

[PATCH] D142890: [clangd] Add config option for fast diagnostics mode

kadircet updated this revision to Diff 495889. kadircet added a comment. - Insert missing include Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142890/new/ https://reviews.llvm.org/D142890 Files: clang-tools-extra/clangd/Config.h clang-tools-e

[PATCH] D143093: [clangd] #undef macros inside preamble patch

kadircet updated this revision to Diff 495891. kadircet marked 3 inline comments as done. kadircet added a comment. - Use raw strings - Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143093/new/ https://reviews.llvm.org/D143093 Files: clan

[PATCH] D143095: [clangd] Respect preamble-patch when handling diags

kadircet updated this revision to Diff 495893. kadircet marked 3 inline comments as done. kadircet added a comment. - Drop FIXME for respecting all presumed locations - Use PatchLoc instead of PLoc - Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D143509: Move the BySpelling map to IncludeStructure.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Headers.h:164 + llvm::StringMap> + buildMainFileIncludesBySpelling() const { +llvm::StringMap> BySpelling; instead of building this on-demand, what about building it as we're collecting d

[PATCH] D143496: [clangd] Add support for missing includes analysis.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Config.h:91 + enum class MissingIncludesPolicy { +/// Diagnose missing includes. rather than duplicating, what about renaming `UnusedIncludesPolicy` to `IncludesPolicy` and use it for bo

[PATCH] D143214: [include-mapping] Add C-compatibility symbol entries.

kadircet added inline comments. Comment at: clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp:218 + if (!D) // global scope. +return getMappingPerLang(L)->NamespaceSymbols->lookup(""); auto It = NamespaceCache.find(D); oh i actually missed the behav

[PATCH] D143319: [clangd] Support iwyu pragma: no_include

kadircet added a comment. thanks a lot for the patch! we're migrating IWYU related functionality in clangd to make use of include-cleaner library nowadays. so can you actually move the IWYU no_include pragma handling logic into https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/i

[PATCH] D143214: [include-mapping] Add C-compatibility symbol entries.

kadircet accepted this revision. kadircet added a comment. thanks, lgtm again Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143214/new/ https://reviews.llvm.org/D143214 ___ cfe-commits mailing list cfe-c

[PATCH] D143638: [clangd] Move function body to out-of-line: unnamed class method incorrect moving

kadircet added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:395 // Bail out in templated classes, as it is hard to spell the class name, i.e // if the template parameter is unnamed. could you move this comme

[PATCH] D143638: [clangd] Move function body to out-of-line: unnamed class method incorrect moving

kadircet accepted this revision. kadircet added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/unittests/tweaks/DefineOutlineTests.cpp:89 + // Not available on methods of unnamed classes. + EXPECT_UNAVAILABLE(R"cpp( +s

[PATCH] D143509: Move the BySpelling map to IncludeStructure.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Headers.cpp:76 +Out->MainFileIncludesBySpelling.try_emplace(Inc.Written) +.first->second.push_back(static_cast(*Inc.HeaderID)); } right now we're only storing "resolve

[PATCH] D143509: Move the BySpelling map to IncludeStructure.

kadircet added a comment. oh forgot to mention, could you also please add some tests into llvm-project/clang-tools-extra/clangd/unittests/HeadersTests.cpp ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143509/new/ https://reviews.llvm.org/D143509

[PATCH] D143640: [Tooling/Inclusion] Support overload symbols in the stdlib.

kadircet added a comment. All this complexity for handling only 4 symbols feels wrong. is this the whole set? are there any other symbols that fall under this class? can we disambiguate all of them solely based on number of parameters? Downsides: - We are relying heavily on number of parameters

[PATCH] D143197: [clangd] Fix bugs in main-file include patching for stale preambles

This revision was automatically updated to reflect the committed changes. Closed by commit rGfae01d175a29: [clangd] Fix bugs in main-file include patching for stale preambles (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D14319

[PATCH] D143597: [clangd] Drop includes from disabled PP regions in preamble patch

This revision was automatically updated to reflect the committed changes. Closed by commit rG19659b5f0dd1: [clangd] Drop includes from disabled PP regions in preamble patch (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143597/

[PATCH] D143096: [clangd] Provide patched diagnostics with preamble patch

kadircet updated this revision to Diff 496900. kadircet marked 12 inline comments as done. kadircet added a comment. - Change patching logic to iterate over diags instead of preamble lines - Change translation logic to: - Preserve a diagnostic if its range can be translated. - Preserve all the

[PATCH] D143096: [clangd] Provide patched diagnostics with preamble patch

kadircet updated this revision to Diff 496905. kadircet added a comment. - Reflow comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143096/new/ https://reviews.llvm.org/D143096 Files: clang-tools-extra/clangd/ParsedAST.cpp clang-tools-ext

[PATCH] D143096: [clangd] Provide patched diagnostics with preamble patch

kadircet updated this revision to Diff 496907. kadircet edited the summary of this revision. kadircet added a comment. - Update commit message Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143096/new/ https://reviews.llvm.org/D143096 Files: clan

[PATCH] D142836: [clangd] Add symbol mappings for `std::experimental::filesystem`

kadircet added a comment. Thanks for updating this, as I mentioned in https://reviews.llvm.org/D143319#4115186, we should put these symbols into their own symbol map. ATM `StdSymbolMap.inc` is still generated by an automated tool and shouldn't be modified by hand. So can you rather put these sy

[PATCH] D143260: [clangd] Add semantic token for labels

kadircet added a comment. thanks for the patch, but could you elaborate a little bit on "why this is useful for clients". in theory semantic highlighting tries to provide annotations for tokens that are hard to disambiguate by a syntax highlighter due to need for context. at hindsight i can't se

[PATCH] D143436: [clangd] Apply standard adaptors to CDBs pushed from LSP

kadircet added a comment. > There is a discrepancy between how clangd processes CDB loaded from JSON file > on disk and pushed via LSP. That's actually because we model compile commands pushed via LSP as a "fixed compilation database" rather than a json compilation database (you can check the

[PATCH] D143640: [Tooling/Inclusion] Support overload symbols in the stdlib.

kadircet added a comment. In D143640#4122603 , @hokein wrote: > Thanks for the comment, putting more thoughts. > > In D143640#4121998 , @kadircet > wrote: > >> All this complexity for handling only 4 symbols feel

[PATCH] D142836: [clangd] Add symbol mappings for `std::experimental::filesystem`

kadircet added a comment. thanks a lot! mostly LG couple more nits Comment at: clang/lib/Tooling/Inclusions/Stdlib/StdTsSymbolMap.inc:1 +// These symbols are exported from N4100[fs.filesystem.synopsis], the final +// draft for experimental filesystem. Note that not all of these

[PATCH] D143112: [clang] Support parsing comments without ASTContext

kadircet added a comment. Thanks, this looks like a good start. I left some comments about the details of implementation, as for placing and APIs in general, i think it's useful to see how things will be built on top inside clangd to make the right call here. Comment at: clan

[PATCH] D142836: [clangd] Add symbol mappings for `std::experimental::filesystem`

kadircet added inline comments. Comment at: clang/lib/Tooling/Inclusions/Stdlib/StdTsSymbolMap.inc:6 +// whatever. +// clang-format off +SYMBOL(absolute, std::experimental::filesystem::, ) zyounan wrote: > kadircet wrote: > > can you strip clang-format pragmas to

[PATCH] D143906: [include-cleaner] Better support ambiguous std symbols

kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp:104 + } + if (FName == "remove") { +if (FD->getNumParams() == 1) nit: `else if` Comment at: clang-tools-extra/include-cleaner/lib/FindHeader

[PATCH] D142836: [clangd] Add symbol mappings for `std::experimental::filesystem`

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Comment at: clang/lib/Tooling/Inclusions/Stdlib/StdTsSymbolMap.inc:6 +// whatever. +// clang-format off +SYMBOL(absolute, std::experimental::filesystem::, ) -

[PATCH] D143906: [include-cleaner] Better support ambiguous std symbols

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Comment at: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp:85 +Hints isPublicHeader(const FileEntry *FE, const PragmaIncludes *PI) { + return (PI->isPri

[PATCH] D143496: [clangd] Add support for missing includes analysis.

kadircet added a comment. Thanks! Details mostly looks good, but i think we had different ideas about how the final diagnostics should look like. Let's try to clear that up a bit. My suggested proposal is: - Having a main diagnostic for each symbol that doesn't have a satisfying include in the

[PATCH] D143509: Move the BySpelling map to IncludeStructure.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Headers.h:167 + // Spelling should include brackets or quotes, e.g. . + llvm::SmallVector + mainFileIncludesWithSpelling(llvm::StringRef Spelling) const { we're still returning just the `Head

[PATCH] D143096: [clangd] Provide patched diagnostics with preamble patch

kadircet updated this revision to Diff 498308. kadircet marked 6 inline comments as done. kadircet added a comment. - Move patch translation logic to a separate class - Perform eager copies and use `bool translateX(X&)` type of APIs instead of returning optionals. - Perform multi line content che

[PATCH] D143096: [clangd] Provide patched diagnostics with preamble patch

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Preamble.cpp:625 +// same spelling. +static std::vector patchDiags(llvm::ArrayRef BaselineDiags, +const ScannedPreamble &BaselineScan, sammccall wrote: > I th

[PATCH] D143096: [clangd] Provide patched diagnostics with preamble patch

kadircet updated this revision to Diff 498351. kadircet marked 4 inline comments as done. kadircet added a comment. - Use direct comparison of ArrayRefs - optional for closest - loop over lookup results rather than through iterator Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

<    30   31   32   33   34   35   36   37   38   39   >