[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

[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

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

[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

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

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

[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

[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

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

[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

[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

[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

[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

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

[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

[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

[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

[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

[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

[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

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

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

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

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

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

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

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

[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 ) { if (Language.CPlusPlus) { +static const auto *Symbols = []() { what about getting

[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] 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()) { +

[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) { +

[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

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

[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

[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

[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

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

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

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

[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

[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

[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

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

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

[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

[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

[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

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

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

[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

[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

[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

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

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

kadircet requested changes to this revision. kadircet added a comment. This revision now requires changes to proceed. sorry i'll need to take a look at this again. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141757/new/

[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: > Conceptually, this (and other `atomic_*` symbols) doesn't feel

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

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

[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"), - } this is actually checking for something else (sorry for the confusing

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

kadircet added inline comments. Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h:133 + + Header(std::in_place_t, decltype(Storage) Sentinel) + : Storage(std::move(Sentinel)) {} sammccall wrote: > this is a bit

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

kadircet updated this revision to Diff 491286. kadircet marked 23 inline comments as done. kadircet added a comment. - Address comments & rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139921/new/ https://reviews.llvm.org/D139921 Files:

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

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. branch cut is upon us, so it'd be great if you can land this soon (or let us know if you don't have commit access) Comment at:

[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

[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

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

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

[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

[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 {}; >

[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] 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 ? >CCSema->getASTContext() : nullptr, -Item, SemaCCS, QueryScopes, *Inserter, FileName, +Recorder ?

[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

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

[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

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

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

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

[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

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

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

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

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

[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] D141495: [clangd] Suppress clang-tidy warnings for code spelled in system macros

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

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

kadircet created this revision. kadircet added a reviewer: tra. Herald added subscribers: mattd, yaxunl. Herald added a project: All. kadircet requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. LibPath discovered during

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