[PATCH] D137319: [include-cleaner] Add export IWYU pragma support.

kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/Record.cpp:69 + // main-file #include with export pragma should never be removed. + if (Top.Exporter == SM.getMainFileID()) +Out->ShouldKeep.insert( hokein wrote:

[PATCH] D137650: [clangd] Implement hover for string literals

kadircet added a comment. sure i am happy to land, @tom-anders do you have anything else to add ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137650/new/ https://reviews.llvm.org/D137650 ___ cfe-commit

[PATCH] D137650: [clangd] Implement hover for string literals

This revision was automatically updated to reflect the committed changes. Closed by commit rG92297bde5ce1: [clangd] Implement hover for string literals (authored by v1nh1shungry, committed by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

[PATCH] D136723: [include-cleaner] Record main-file macro occurences and includes

kadircet added a comment. In D136723#3915898 , @sammccall wrote: > (and sorry, it seems I was sitting on a pile of comments I thought I'd sent, > LMK if I should follow up on them) no looks good. the only meaty discussion was fileentry to multiple incl

[PATCH] D137319: [include-cleaner] Add export IWYU pragma support.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks for bearing with me, LG! Comment at: clang-tools-extra/include-cleaner/lib/Record.cpp:72 + void checkForExport(FileID FileSeenHash, int HashLine, +

[PATCH] D137320: [include-cleaner] Initial version for the "Location=>Header" step

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! Comment at: clang-tools-extra/include-cleaner/lib/Analysis.cpp:58 +Results = {{FE}}; +// FIXME: compute a closure of exporter headers. +for (con

[PATCH] D137697: Move the isSelfContained function from clangd to libtooling.

kadircet added inline comments. Comment at: clang/include/clang/Tooling/Inclusions/Header.h:1 +//===--- Header.h *- C++-*-===// +// rather than calling this `Header` can we call this `HeaderAnalysis`? ===

[PATCH] D137698: [include-cleaner] Add self-contained file support for PragmaIncludes.

kadircet added a comment. i guess this patch needs a rebase for other pieces as well? but LG in general Comment at: clang-tools-extra/include-cleaner/lib/Analysis.cpp:47 + const PragmaIncludes &PI) { + const FileEntry *FE = SM.getFil

[PATCH] D137697: Move the isSelfContained function from clangd to libtooling.

kadircet added inline comments. Comment at: clang/include/clang/Tooling/Inclusions/Header.h:21 +/// +/// A header is considered self-contained if either it has a proper header guard +/// or it doesn't has dont-include-me-similar pattern. kadircet wrote: > let's

[PATCH] D137825: [clang-include-cleaner] make SymbolLocation a real class, move FindHeaders

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

[PATCH] D137697: Move the isSelfContained function from clangd to libtooling.

kadircet added inline comments. Comment at: clang/lib/Tooling/Inclusions/Header.cpp:64 + // particular preprocessor state, usually set up by another header. + return !isDontIncludeMeHeader(SM.getBufferData(SM.translateFile(FE))); +} sammccall wrote: > kadircet

[PATCH] D137697: Move the isSelfContained function from clangd to libtooling.

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

[PATCH] D137697: Move the isSelfContained function from clangd to libtooling.

kadircet added inline comments. Comment at: clang/include/clang/Tooling/Inclusions/HeaderAnalysis.h:28 +/// dont-include-me pattern heuristically. +bool isSelfContainedHeader(FileID FID, const SourceManager &SM, + HeaderSearch &HeaderInfo); -

[PATCH] D137697: Move the isSelfContained function from clangd to libtooling.

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

[PATCH] D137962: [clang][Tooling] Make the filename behaviour consistent

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, ilya-biryukov. Herald added a project: clang. Dotdots were removed only when converting a relative path to absolute

[PATCH] D137962: [clang][Tooling] Make the filename behaviour consistent

kadircet updated this revision to Diff 475362. kadircet added a comment. - Drop documentation - Call remove_dots on the common path Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137962/new/ https://reviews.llvm.org/D137962 Files: clang/lib/Tooli

[PATCH] D137962: [clang][Tooling] Make the filename behaviour consistent

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGae59131d3ef3: [clang][Tooling] Make the filename behaviour consistent (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE L

[PATCH] D137698: [include-cleaner] Add self-contained file support for PragmaIncludes.

kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h:100 + /// Contains all non self-contained files during the parsing. + llvm::DenseSet NonSelfContainedFiles; s/during/detected during/ =

[PATCH] D138047: Fix use of dangling stack allocated string in IncludeFixer

kadircet added a comment. yeah unfortunately testing this is hard, but bug&fix are obvious so it's fine to land without a test. but i think we should rather fix the broken call site at clang-tools-extra/clangd/ParsedAST.cpp and move `BuildDir` to outer scope where it'll outlive IncludeFixer (~

[PATCH] D137817: [clangd] Improve action `RemoveUsingNamespace` for user-defined literals

kadircet added a comment. thanks for pinging @tom-anders , i've added some concerns about the behaviour in general. sorry if these are discussed somewhere else but i've missed. Comment at: clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp:294 +} +using n

[PATCH] D137919: [clangd] use fine-grained code action kinds

kadircet added a comment. as discussed in https://github.com/clangd/clangd/issues/1326#issuecomment-1313502572, we've decided to not move forward with the change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137919/new/ https://reviews.llvm.org/

[PATCH] D137817: [clangd] Improve action `RemoveUsingNamespace` for user-defined literals

kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp:294 +} +using namespace ns1::ns2; +using namespace ns1::ns3; v1nh1shungry wrote: > kadircet wrote: > > sorry i am having trouble understanding why

[PATCH] D138134: [include-cleaner] Defer decl->stdlib conversion into decl->location conversion

kadircet created this revision. kadircet added reviewers: hokein, 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 preserve decls for stdlib symbols after this patch in symbol.

[PATCH] D138134: [include-cleaner] Defer decl->stdlib conversion into decl->location conversion

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe5739eb4f813: [include-cleaner] Defer decl->stdlib conversion into decl->location conversion (authored by kadircet). Repository: rG LLVM Github Mo

[PATCH] D138047: Fix use of dangling stack allocated string in IncludeFixer

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

[PATCH] D138200: [include-cleaner] Make use of locateSymbol in WalkUsed

kadircet created this revision. kadircet added reviewers: hokein, 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. Depens on D135953 Repository

[PATCH] D135953: [IncludeCleaner] Introduce decl to location mapping

kadircet updated this revision to Diff 476091. kadircet added a comment. - Rebase - Move to new file - Handle macros - Change from boolean output to enum for signalling Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135953/new/ https://reviews.llvm.

[PATCH] D136082: [clangd] Extend --check to time clang-tidy checks, so we can block slow ones

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. Herald added a reviewer: njames93. thanks, lgtm! Comment at: clang-tools-extra/clangd/tool/Check.cpp:66 +llvm::cl::desc("Print the overhead of checks matching this gl

[PATCH] D128457: [clangd] Add new IncludeType to IncludeHeaderWithReferences

kadircet added inline comments. Comment at: clang-tools-extra/clangd/SourceCode.cpp:1241 + !HeaderInfo.hasFileBeenImported(FE) && + (FID != SM.getMainFileID() || !fileContainsImports(FID, SM))) return false; can you put a comment here saying `any h

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

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: > VitaNuo wrote: > > VitaNuo wrote: > > > kadircet wrote: > > > > hokei

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

kadircet added inline comments. Comment at: clang/tools/include-mapping/cppreference_parser.py:174 - # std::remove<> has variant algorithm. - "std::remove": ("algorithm"), - } VitaNuo wrote: > VitaNuo wrote: > > kadircet wrote: > > > VitaNuo wrote: > >

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:994 HI.CalleeArgInfo.emplace(toHoverInfoParam(PVD, PP)); + PassType.PassBy = getPassMode(PVD->getType()); +} sorry for showing up late to the party but instead of chang

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:994 HI.CalleeArgInfo.emplace(toHoverInfoParam(PVD, PP)); + PassType.PassBy = getPassMode(PVD->getType()); +} v1nh1shungry wrote: > kadircet wrote: > > sorry for showing

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:994 HI.CalleeArgInfo.emplace(toHoverInfoParam(PVD, PP)); + PassType.PassBy = getPassMode(PVD->getType()); +} v1nh1shungry wrote: > kadircet wrote: > > v1nh1shungry wrot

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

kadircet added inline comments. Comment at: clang/include/clang/Tooling/Inclusions/StdSymbolMap.inc:205 SYMBOL(basic_syncbuf, std::, ) SYMBOL(begin, std::, ) SYMBOL(bernoulli_distribution, std::, ) i think we should have other providers here, https://en.cppre

[PATCH] D142228: [clangd] Disable tests that are incompatible with Windows

kadircet added a comment. thanks this makes sense! we were filtering on `target_triple` before, which is not right, especially when host triple is not the default target. as a follow up i'll drop all the `target={{...}}` conditions as we should actually be able to run these tests even when they

[PATCH] D142440: [clangd] Don't show 'auto' type hint when type deduction fails

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Comment at: clang-tools-extra/clangd/InlayHints.cpp:307 +if (auto *AT = D->getType()->getContainedAutoType()) { + if (!AT->getDeducedType().isNull() && !

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

kadircet added inline comments. Comment at: clang/tools/include-mapping/cppreference_parser.py:196 + "std::remove$": "algorithm", + "std::atomic.*": "atomic", + there's no variant of "std::atomic.*" called "atomic", in https://en.cppreference.com/w/cpp

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

kadircet added a subscriber: VitaNuo. kadircet added a comment. thanks a lot for the patch, we're already having some efforts right now to improve our cppreference parsing and get to a more complete set of symbols (@VitaNuo for visibility, who drives these efforts). Regarding the experimental s

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

kadircet added a comment. And to follow up, I definitely see the problem you're facing and it's something we'd like to address as well, just not in the way you proposed. This falls under the bucket of "we might have symbols missing from our stdlib mappings and should try to compensate for that"

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

kadircet created this revision. kadircet added reviewers: hokein, 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. Repos

[PATCH] D142892: [clangd] Publish diagnostics from stale preambles

kadircet created this revision. kadircet added reviewers: sammccall, 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. Diagn

[PATCH] D142892: [clangd] Publish diagnostics from stale preambles

kadircet updated this revision to Diff 493382. kadircet added a comment. Well, turns out presumed location patching was not handled in clangd at all for diagnostics. So add that to translate ranges, at least when we have ranges or locations that point to preamble patch. Repository: rG LLVM Git

[PATCH] D142871: [clangd] Semantic highlighting for constrained-parameter

kadircet added a subscriber: usaxena95. kadircet added inline comments. Comment at: clang-tools-extra/clangd/FindTarget.cpp:720 + // Pick up the name via VisitNamedDecl + Base::VisitTemplateTypeParmDecl(D); +} nridge wrote: > Am I using the API corr

[PATCH] D139926: [clangd] Add semantic token for angle brackets

kadircet added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:382 + +// For the inner element of a nested template instantiation with no space +// between the '>' characters, TemplateSpecializationLocInfo::RAngleLoc has i a

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:994 HI.CalleeArgInfo.emplace(toHoverInfoParam(PVD, PP)); + PassType.PassBy = getPassMode(PVD->getType()); +} v1nh1shungry wrote: > kadircet wrote: > > v1nh1shungry wrot

[PATCH] D139926: [clangd] Add semantic token for angle brackets

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. In D139926#4091990 , @nridge wrote: > but it sounds like you've looked at that well i did now :P --- thanks a lot to you both, let's ship it!

[PATCH] D140915: [clangd] Fix getQueryScopes for using-directive with inline namespace

kadircet added inline comments. Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1709 + +// The enclosing namespace must be first, it gets a quality boost. +if (auto Enclosing = SpecifiedScopes.EnclosingNamespace) { i was actually suggesting to put t

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

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

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

kadircet updated this revision to Diff 494003. kadircet added a comment. Wire up the feature and add some test cases 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/Confi

[PATCH] D143093: [clangd] #undef macros inside 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. That way we

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

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. Depends on D

[PATCH] D143096: [clangd] Provide patched diagnostics with 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. This patch t

[PATCH] D142892: [clangd] Publish diagnostics from stale preambles

kadircet abandoned this revision. kadircet added a comment. split into D143093 , D143095 and D143096 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

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

kadircet added inline comments. Comment at: clang-tools-extra/clangd/ConfigFragment.h:243 +/// - Strict +std::optional> Mode; + sammccall wrote: > I think "Diagnostics.Mode" is too vague a name. > > I expect this to be a rollout flag that we remove in th

[PATCH] D143197: [clangd] Patch includes even without any changes

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. Repository:

[PATCH] D143197: [clangd] Patch includes even without any changes

kadircet updated this revision to Diff 494347. kadircet added a comment. - Also populate additional fields Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143197/new/ https://reviews.llvm.org/D143197 Files: clang-tools-extra/clangd/Preamble.cpp

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

kadircet added a comment. In D143093#4099623 , @sammccall wrote: > I can't understand from the description, code, or testcases what problem this > is fixing. Can you clarify, ideally by improving the testcases? Yeah should've elaborated on this one. As

[PATCH] D143160: [include-mapping] Introduce a human-edit CXXSymbolMapping file

kadircet added inline comments. Comment at: clang/include/clang/Tooling/Inclusions/CXXSymbolMap.inc:12 +SYMBOL(consume_header, std::, ) // declared +SYMBOL(consume_header, std::, ) // defined +SYMBOL(generate_header, std::, ) i feel like cppreference is wrong he

[PATCH] D142871: [clangd] Semantic highlighting for constrained-parameter

kadircet accepted this revision. kadircet added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/FindTarget.cpp:1047 + bool TraverseTypeConstraint(const TypeConstraint *TC) { +Out(ReferenceLoc{TC->getNestedNameSpecifier

[PATCH] D140915: [clangd] Fix getQueryScopes for using-directive with inline namespace

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. LGTM, thanks! Comment at: clang-tools-extra/clangd/CodeComplete.cpp:675 +std::vector EnclosingAtFront; +if (EnclosingNamespace.has_value()) { + EnclosingAtFr

[PATCH] D143160: [include-mapping] Introduce a human-edit CXXSymbolMapping file

kadircet added inline comments. Comment at: clang/include/clang/Tooling/Inclusions/CXXSymbolMap.inc:1 +//===-- CXXSymbolMap.inc *- C++ -*-===// +// also maybe rename this to, `AlternativeHeaderMap.inc` ?

[PATCH] D143274: [clangd] Remove the direct use of StdSymbolMapping.inc usage.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:736 void CanonicalIncludes::addSystemHeadersMapping(const LangOptions &Language) { if (Language.CPlusPlus) { +static const auto *Symbols = []() { what about g

[PATCH] D143280: [include-mapping] Better #includes support for std input/output symbols

kadircet added inline comments. Comment at: clang/tools/include-mapping/gen_std.py:160 + headers = [sym_header] + if symbol.name in iosfwd_symbols: +headers.append("") i think putting `iostream` before `iosfwd` in the alternative list makes more sense. WDY

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

kadircet added inline comments. Comment at: clang/tools/include-mapping/gen_std.py:107-108 symbols = cppreference_parser.GetSymbols(parse_pages) - + # C++ form of the C standard headers. + c_style_headers = { +"", Comment at: clang/

[PATCH] D143160: [include-mapping] Introduce a human-edit CXXSymbolMapping file

kadircet added inline comments. Comment at: clang/include/clang/Tooling/Inclusions/StdAlternativeHeaderMap.inc:3 +// +// This is a hand-curated list for C++ symbols (e.g. provided by multiple +// headers), to address the short comings of cppreference or automated

[PATCH] D143274: [clangd] Remove the direct use of StdSymbolMapping.inc usage.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:711 +return ""; + // There are multiple headers for size_t, pick one. + if (QName == "std::size_t") i think the comment is misleading. as if we had some alterna

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

kadircet added inline comments. Comment at: clang/tools/include-mapping/gen_std.py:123 +c_header = "<" + header[2:-1] + ".h>" +if not any(re.fullmatch(x, symbol.name) for x in exception_symbols): + if symbol.namespace != None: nit: early exits ```

[PATCH] D143274: [clangd] Remove the direct use of StdSymbolMapping.inc usage.

kadircet added a comment. thanks a lot. since this is the last (and only) upstream user of the raw mappings. can you also move them into `clang/lib/Tooling/Inclusions/Stdlib/` as part of this patch? Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.h:43 + /// Retu

[PATCH] D143280: [include-mapping] Better #includes support for std input/output symbols

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, lgtm! as discussed offline, let's first move the raw header mappings to a private location first though and make sure all users are going through stdlib apis to ensure we don't see

[PATCH] D143160: [include-mapping] Introduce a human-edit CXXSymbolMapping file

kadircet added inline comments. Comment at: clang/include/clang/Tooling/Inclusions/StdAlternativeHeaderMap.inc:3 +// +// This is a hand-curated list for C++ symbols (e.g. provided by multiple +// headers), to address the short comings of cppreference or automated

[PATCH] D143274: [clangd] Remove the direct use of StdSymbolMapping.inc usage.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.h:43 + /// Returns the overridden include for a qualified symbol with, or "". + /// \p Scope and \p Name concatenation forms the full qualified name. + /// \p Scope is the qualifier wi

[PATCH] D143160: [include-mapping] Introduce a human-edit CXXSymbolMapping file

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

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

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

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

kadircet marked 5 inline comments as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp:555 + EXPECT_TRUE(compileAndApply()); + // Defaults to Strict. + EXPECT_EQ(Conf.Diagnostics.Mode, Config::DiagnosticsMode::Fast); --

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

kadircet added inline comments. Comment at: clang-tools-extra/clangd/Preamble.cpp:721 + // re-definition warnings. + if (TD.Directive == tok::pp_define) +Patch << "#undef " << TD.MacroName << '\n'; i know we've discussed emitting all the `#undef

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

kadircet updated this revision to Diff 495185. kadircet added a comment. - Update tests 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/c

[PATCH] D141467: [clang][Driver][CUDA] Get rid of unused LibPath

kadircet added a comment. In D141467#4045190 , @tra wrote: > Given that it's indeed unused, I'm fine with removing it. > > That said, it's somewhat odd that in your setup clang was able to find > everything but the library directory. You generally would

[PATCH] D141467: [clang][Driver][CUDA] Get rid of unused LibPath

This revision was automatically updated to reflect the committed changes. Closed by commit rG4921b0a285ac: [clang][Driver][CUDA] Get rid of unused LibPath (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141467/new/ https://revi

[PATCH] D141509: [include-mapping] Fix parsing of html_book_20190607.zip (https://en.cppreference.com/w/File:html_book_20190607.zip). Skip entries that have been added to the index (C++20 symbols), bu

kadircet added inline comments. Comment at: clang/tools/include-mapping/cppreference_parser.py:145 path = os.path.join(root_dir, symbol_page_path) - results.append((symbol_name, + if os.path.isfile(path): +results.append((symbol_name, sor

[PATCH] D141611: [include-mapping] Print an error message in case the symbol index points to a non-existent page.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Comment at: clang/tools/include-mapping/cppreference_parser.py:149 + else: +sys.stderr.write("Could not parse page: %s. Page does not exist.\n" % pa

[PATCH] D141047: build: with -DCLANGD_ENABLE_REMOTE=ON, search for grpc++ dependencies too

kadircet added inline comments. Comment at: clang/cmake/modules/AddGRPC.cmake:7 generate_proto_sources(ProtoSource ${ProtoFile} ${PROTO_UNPARSED_ARGUMENTS}) + set(LINKED_GRPC_LIBRARIES protobuf gpr grpc grpc++) Hi @sylvestre.ledru this seem to have broken c

[PATCH] D141608: [include-cleaner] Don't consider the underlying type of Decltype MemberProvider as a use

kadircet added a comment. I am a little skeptical about this one. I think it's somewhat an obscure pattern, and probably warrants a use of the type when it can be deduced and the user code is accessing members (relying on the type more than an opaque manner). So what about waiting on this patch

[PATCH] D141670: [include-cleaner] FindHeaders respects IWYU export pragma for standard headers.

kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/lib/Record.cpp:203 + const FileEntry *IncludedHeader, + std::optional StandardHeader) { if (ExportStack.empty()) just saying `StandardHead

[PATCH] D141671: Move around structs and definitions to prevent incomplete types.

kadircet added a comment. In D141671#4050928 , @ilya-biryukov wrote: > There is potentially a way to move less code by keeping all declarations at > place and only moving bodies of definitions of constructors and destructors > to the `.cpp` file. > Not

[PATCH] D141671: Move definitions to prevent incomplete types.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks, LGTM! (btw, i know it's too late already, but in theory clangd has a code action called move definition out-of-line, could help with such refactorings in the future, if you didn't

[PATCH] D141670: [include-cleaner] FindHeaders respects IWYU export 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/include-cleaner/lib/Record.cpp:192 int HashLine = SM.getLineNumber(HashFID, SM.getFileOffset(HashLoc)); -checkForExport(Has

[PATCH] D139926: [clangd] Add semantic tokens for angle brackets

kadircet added a comment. Sorry for missing this one, apparently I've already started writing an answer but got context switching and forgot about it. > This is needed for clients that would like to visualize matching > opening and closing angle brackets, which can be valuable in non-trivial > t

[PATCH] D139926: [clangd] Add semantic tokens for angle brackets

kadircet added a comment. > Note that we use this information to *animate* the matching tokens, i.e. when > the cursor is on one of them, both it and its counterpart get a special > highlighting. That's why it's so important that the language server > guarantees they always come in pairs. Oh I

[PATCH] D140875: [clangd] prototype: Implement unused include warnings with include-cleaner library.

kadircet added a comment. regarding testing, i think updating all tests that we have in `IncludeCleanerTests` that call `computeUnusedIncludes` to also call the new function that'll use include-cleaner library should be enough. Comment at: clang-tools-extra/clangd/CMakeLists.

[PATCH] D140915: [clangd] Fix getQueryScopes for using-directive with inline namespace

kadircet added inline comments. Comment at: clang-tools-extra/clangd/CodeComplete.cpp:646 // "" for global namespace, "ns::" for normal namespace. + std::vector QueryScopes; + // Like QueryScopes, but also contains inline namespaces. could you actually chang

[PATCH] D141800: [clangd] Fix qualifier not being dropped for using declaration referring to scoped enum

kadircet added inline comments. Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1993 Builder.emplace(Recorder ? &Recorder->CCSema->getASTContext() : nullptr, -Item, SemaCCS, QueryScopes, *Inserter, FileName, +Recorde

[PATCH] D140875: [clangd] prototype: Implement unused include warnings with include-cleaner library.

kadircet added inline comments. Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:479 +} +// FIXME: !!this is a hacky way to collect macro references. +std::vector Macros; hokein wrote: > kadircet wrote: > > this might behave slightly different

[PATCH] D139926: [clangd] Add semantic token for angle brackets

kadircet added a comment. > We can easily check the actual character at the given position in the client, > so I could just merge the two highlighting kinds. Thanks! Note that it might not be as easy at the face of templates, eg: #define LESS < template LESS typename T > class A {}; > No

[PATCH] D140875: [clangd] prototype: Implement unused include warnings with include-cleaner library.

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. thanks! Comment at: clang-tools-extra/clangd/ParsedAST.h:111 + const include_cleaner::PragmaIncludes *getPragmaIncludes() const; + can you add a comme

[PATCH] D140875: [clangd] prototype: Implement unused include warnings with include-cleaner library.

kadircet added a comment. sorry for the hassle 2133e8b9f942f91ec54e28c580fccf6d6b26c62e should fix Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140875/new/ https://reviews.llvm

[PATCH] D142187: [clang] Fix typos in member initializers

kadircet created this revision. kadircet added reviewers: hokein, cor3ntin, aaron.ballman. Herald added a project: All. kadircet requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This was regressed in ca619613801233ef2def8c3cc7d311d5ed0033cb

[PATCH] D142187: [clang] Fix typos in member initializers

kadircet marked 3 inline comments as done. kadircet added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:4101 - ExprResult Init = InitExpr; - if (!FD->getType()->isDependentType() && !InitExpr->isTypeDependent()) { -Init = ConvertMemberDefaultInitExpression(F

[PATCH] D142187: [clang] Fix typos in member initializers

kadircet updated this revision to Diff 490793. kadircet marked 3 inline comments as done. kadircet added a comment. assert on usability of initializer Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142187/new/ https://reviews.llvm.org/D142187 Files

[PATCH] D142187: [clang] Fix typos in member initializers

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGebd9a2477e69: [clang] Fix typos in member initializers (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

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