[PATCH] D124606: Use `-text` git attribute instead of `text eol=...'

ilya-biryukov added a comment. @labath, ah, sorry, you beat me to it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124606/new/ https://reviews.llvm.org/D124606 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D124606: Use `-text` git attribute instead of `text eol=...'

ilya-biryukov added a comment. In D124606#3484740 , @Meinersbur wrote: > In D124606#3479793 , @ilya-biryukov > wrote: > >> LGTM > > This and the commit (@MForster ) was too hasty. There should have been time >

[PATCH] D124923: [Sema] Simplify CheckConstraintSatisfaction. NFC

ilya-biryukov created this revision. ilya-biryukov added reviewers: sammccall, saar.raz. Herald added a project: All. ilya-biryukov requested review of this revision. Herald added a project: clang. - Exit early when constraint caching is disabled. - Use unique_ptr to manage temporary lifetime. - F

[PATCH] D124923: [Sema] Simplify CheckConstraintSatisfaction. NFC

ilya-biryukov added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:335 + auto Satisfaction = + std::make_unique(Template, TemplateArgs); if (::CheckConstraintSatisfaction(*this, Template, ConstraintExprs, I also wonder if this could be alloc

[PATCH] D124923: [Sema] Simplify CheckConstraintSatisfaction. NFC

ilya-biryukov updated this revision to Diff 427018. ilya-biryukov added a comment. - add FIXME for a memory leak Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124923/new/ https://reviews.llvm.org/D124923 Files: clang/lib/Sema/SemaConcept.cpp I

[PATCH] D124923: [Sema] Simplify CheckConstraintSatisfaction. NFC

ilya-biryukov added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:321 + bool ShouldCache = LangOpts.ConceptSatisfactionCaching && Template; + if (!ShouldCache) { sammccall wrote: > Another loose end that could be cleaned up sometime! > > `Conce

[PATCH] D124923: [Sema] Simplify CheckConstraintSatisfaction. NFC

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG726d7b07fcde: [Sema] Simplify CheckConstraintSatisfaction. NFC (authored by ilya-biryukov). Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[PATCH] D124923: [Sema] Simplify CheckConstraintSatisfaction. NFC

ilya-biryukov added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:335 + auto Satisfaction = + std::make_unique(Template, TemplateArgs); if (::CheckConstraintSatisfaction(*this, Template, ConstraintExprs, ilya-biryukov wrote: > sammccall wro

[PATCH] D125014: [Driver] Remove -fno-concept-satisfaction-caching

ilya-biryukov created this revision. ilya-biryukov added a reviewer: kadircet. Herald added a project: All. ilya-biryukov requested review of this revision. Herald added a project: clang. The flag was added when the C++20 draft did not allow for concept caching. The final C++20 standard permits th

[PATCH] D125014: [Driver] Remove -fno-concept-satisfaction-caching

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe13c28ec595d: [Driver] Remove -fno-concept-satisfaction-caching (authored by ilya-biryukov). Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D124923: [Sema] Simplify CheckConstraintSatisfaction. NFC

ilya-biryukov added a comment. @erichkeane sure, will do. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124923/new/ https://reviews.llvm.org/D124923 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D125103: [clangd] Speed up a slow sleeping testcase.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. Many thanks for improving the running time, this slow test has bugged me since the day I first ran it. I'd even be happy to remove it completely. It's slow, possibly flaky, so IMO

[PATCH] D129488: [Sema] Fix evaluation context of immediate functions in default arguments

ilya-biryukov created this revision. ilya-biryukov added a reviewer: jyknight. Herald added a project: All. ilya-biryukov requested review of this revision. Herald added a project: clang. They must be evaluated in the context where default argument is actually used during a call, not in a paramete

[PATCH] D129068: [AST] Accept identical TypeConstraint referring to other template parameters.

ilya-biryukov added a comment. A few NITs from my side, hopefully they can improve things a bit. Comment at: clang/include/clang/AST/ASTContext.h:2676 + /// Determine whether two 'requires' expressions are similar enough. + /// Use of 'requires' isn't mandatory, works with

[PATCH] D129499: [clang] Do not crash on "requires" after a fatal error occurred.

ilya-biryukov added a comment. Thanks! The fix LG, just a small nitpick for the test from my side Comment at: clang/test/SemaCXX/concept-fatal-error.cpp:8 + // We test that we do not crash in such cases (#55401) + int i = requires { { i } f } // expected-error {{expected ';'

[PATCH] D128621: [clangd] Do not try to use $0 as a placeholder in completion snippets

ilya-biryukov added a comment. Another alternative that I think should give the best UX is to replace `${0:named}` with `$0`. The items will look different, but will behave identically to the old behavior before VSCode change, i.e. won't "eat" an extra tab press at the end of completion session

[PATCH] D129068: [AST] Accept identical TypeConstraint referring to other template parameters.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. LGTM, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129068/new/ https://reviews.llvm.org/D129068 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://list

[PATCH] D129499: [clang] Do not crash on "requires" after a fatal error occurred.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. lgtm Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129499/new/ https://reviews.llvm.org/D129499 _

[PATCH] D130228: [clangd] Mention whether compile flags were inferred in check mode

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM. Please note the NIT that arguably improves readability. Comment at: clang-tools-extra/clangd/tool/Check.cpp:114 Cmd = std::move(*TrueCmd); - l

[PATCH] D130261: [clangd] Refactor forwarding call detection logic

ilya-biryukov requested changes to this revision. ilya-biryukov added a comment. This revision now requires changes to proceed. The new test fails in the pre merge checks. Maybe the diff is missing some more changes? Comment at: clang-tools-extra/clangd/AST.cpp:794 }

[PATCH] D130261: [clangd] Refactor forwarding call detection logic

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130261/new/ https://reviews.llvm.org/D130261 _

[PATCH] D129488: [Sema] Fix evaluation context of immediate functions in default arguments

ilya-biryukov added a comment. TODO: add example from cpprefence to the tests: consteval int f() { return 42; } consteval auto g() { return &f; } consteval int h(int (*p)() = g()) { return p(); } constexpr int r = h(); // OK constexpr auto e = g(); // ill-formed: a pointer to an immedi

[PATCH] D129488: [Sema] Fix evaluation context of immediate functions in default arguments

ilya-biryukov added a comment. another example that actually breaks: constexpr int foo(int i) { if (i == 1) return i/0; } consteval int bar(int i) { if (i == 1) return i/0; return i; } void baz(int i = foo(1), int j = bar(1)) { } void xxx() { baz();

[PATCH] D130260: [clangd] Make forwarding parameter detection logic resilient

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/AST.cpp:833 +// Check that this expands all the way until the last parameter. +auto ParamEnd = It + Parameters.size() - 1; +assert(std::distance(Args.begin(), ParamEnd) < ---

[PATCH] D130260: [clangd] Make forwarding parameter detection logic resilient

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/AST.cpp:828 // find the argument directly referring to the first parameter -for (auto It = Args.begin(); It != Args.end(); ++It) { - const Expr *Arg = *It; - if (const auto *RefArg = unwr

[PATCH] D130260: [clangd] Make forwarding parameter detection logic resilient

ilya-biryukov accepted this revision. ilya-biryukov added a comment. LGTM Comment at: clang-tools-extra/clangd/AST.cpp:830 +assert(Parameters.size() <= + static_cast(std::distance(Args.begin(), Args.end(; +for (auto Begin = Args.begin(), End = Args.end() -

[PATCH] D128921: [Sema] Merge C++20 concept definitions from different modules in same TU

ilya-biryukov updated this revision to Diff 446883. ilya-biryukov added a comment. - Update diagnostics per Richard's suggestions, add tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128921/new/ https://reviews.llvm.org/D128921 Files: clang/

[PATCH] D128921: [Sema] Merge C++20 concept definitions from different modules in same TU

ilya-biryukov marked 3 inline comments as done. ilya-biryukov added a comment. Sorry, was out again due to an emergency. So I wanted to give a chance for any last comments despite having an LGTM I plan to land this on Monday. Comment at: clang/lib/Sema/SemaTemplate.cpp:8694-86

[PATCH] D128921: [Sema] Merge C++20 concept definitions from different modules in same TU

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG59179d72b2e3: [Sema] Merge C++20 concept definitions from different modules in same TU (authored by ilya-biryukov). Changed prior to commit: https

[PATCH] D125172: [clangd] Disable predefined macros in tests. NFC

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM. Comment at: clang-tools-extra/clangd/unittests/TestTU.cpp:44 + // In tests, we don't need predefined macros (__GNUC__, __CHAR_BIT__) etc. + // There are

[PATCH] D125169: [clangd] Skip extra round-trip in parsing args in debug builds. NFC

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125169/new/ https://reviews.llvm.org/D125169 _

[PATCH] D125109: [clangd] Rewrite TweakTesting helpers to avoid reparsing the same code. NFC

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125109/new/ https://reviews.llvm.org/D125109

[PATCH] D125224: [CodeComplete] prototype of contextual postfix completions

ilya-biryukov added inline comments. Comment at: clang/include/clang/Sema/CodeCompleteConsumer.h:784 + /// When Kind == RK_Pattern, an optional short name for the pattern. + /// Often he pattern combines with a fixit to rewrite surrounding code. + /// NIT: s/h

[PATCH] D121733: Clean pathnames in FileManager.

ilya-biryukov requested changes to this revision. ilya-biryukov added a comment. This revision now requires changes to proceed. Please allow some time for the clangd team to review this before landing. Changes to filenames used to cause unintended consequences for us before. We switched to using

[PATCH] D121733: Clean pathnames in FileManager.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:24 + static auto *Mappings = + new std::array, 645>{{ + {"algorithm", ""}, ppluzhnikov wrote: > ilya-biryukov wrote: > > Don't specify sizes of ar

[PATCH] D125773: [Driver] Do not auto-enable header modules with -std=c++20

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added a project: All. ilya-biryukov requested review of this revision. Herald added a subscriber: MaskRay. Herald added a project: clang. This only shows up if one passes module maps to Clang in `-std=c++20` mod

[PATCH] D125773: [Driver] Do not auto-enable header modules with -std=c++20

ilya-biryukov updated this revision to Diff 430056. ilya-biryukov added a comment. - Revert gnu++20 fix - Always pass -fheader-modules/-fno-header-modules when modules are enabled Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125773/new/ https://re

[PATCH] D125773: [Driver] Do not auto-enable header modules with -std=c++20

ilya-biryukov added a reviewer: rsmith. ilya-biryukov marked 2 inline comments as done. ilya-biryukov added a comment. In D125773#3519048 , @sammccall wrote: > LG, but I think this is choosing a (new) public name for clang modules/header > modules, so ma

[PATCH] D125868: [Driver] Recognize -std=gnu++20 enables C++ modules

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added a project: All. ilya-biryukov requested review of this revision. Herald added a subscriber: MaskRay. Herald added a project: clang. In addition to -std=c++20. This used to result in wrong -cc1 flags produc

[PATCH] D125403: [Serialization] Delta-encode consecutive SourceLocations in TypeLoc

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. NIT: typo in the change description > Randam-access Rand**o**m LGTM overall, the improvements in PCH and PCM sizes seem worthwhile. Please see the NIT about the naming before l

[PATCH] D125952: [Serialization] Delta encode locations in expansion sloc entries

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125952/new/ https://reviews.llvm.org/D125952 _

[PATCH] D125773: [Driver] Do not auto-enable header modules with -std=c++20

ilya-biryukov planned changes to this revision. ilya-biryukov marked 2 inline comments as done. ilya-biryukov added a comment. Richard, thanks for course correcting. I was under impression that header modules are not in the standard, my mistake. It looks like this particular change actually brea

[PATCH] D125773: [Driver] Do not auto-enable header modules with -std=c++20

ilya-biryukov added a comment. In D125773#3525215 , @tahonermann wrote: >> So the proposal is that -fheader-modules=parse would parse #include of >> header unit in the same TU, and import .pcm on import, right? > > Per cpp.includep7

[PATCH] D159351: [Sema] Change order of displayed overloads in diagnostics

ilya-biryukov updated this revision to Diff 557839. ilya-biryukov marked 4 inline comments as done. ilya-biryukov added a comment. - Addressed comments - Added a release note Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159351/new/ https://reviews

[PATCH] D159351: [Sema] Change order of displayed overloads in diagnostics

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGfebf5c97bba7: [Sema] Change order of displayed overloads in diagnostics (authored by ilya-biryukov). Changed prior to commit: https://reviews.llvm

[PATCH] D66516: [clangd] Added highlighting to types dependant on templates.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:177 return; +if (TP->isPointerType() || TP->isLValueReferenceType()) + // When highlighting dependant template types the type can be a pointer or `

[PATCH] D66478: [clangd] Ignore implicit conversion-operator nodes in find refs.

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

[PATCH] D66516: [clangd] Added highlighting to types dependant on templates.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:177 return; +if (TP->isPointerType() || TP->isLValueReferenceType()) + // When highlighting dependant template types the type can be a pointer or j

[PATCH] D66530: [clangd] Don't collect locations that implicitly refer to the target decl.

ilya-biryukov added a comment. This actually seems useful. If one starts the search at `operator bool`, it's nice to see its implicit calls too. What are the cons of having this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66530/new/ https://rev

[PATCH] D66578: Remove \brief commands from doxygen comments.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM. Kill it with fire Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66578/new/ https://reviews.llvm.org/D66578 __

[PATCH] D66592: [clangd] Send suppported codeActionKinds to the client.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:467 + // Per LSP, codeActionProvide can be either boolean or CodeActionOptions. + // CodeActio

[PATCH] D66516: [clangd] Added highlighting to types dependant on templates.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:177 return; +if (TP->isPointerType() || TP->isLValueReferenceType()) + // When highlighting dependant template types the type can be a pointer or j

[PATCH] D66637: [clangd] Support multifile edits as output of Tweaks

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:644 + assert(It.second.Edits && "TextEdits hasn't been generated?"); + if (auto Draft = DraftMgr.getDraft(It.first())) { +llvm::StringRef Contents = *Draft;

[PATCH] D66637: [clangd] Support multifile edits as output of Tweaks

ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:644 + assert(It.second.Edits && "TextEdits hasn't been generated?"); + if (auto Draft = DraftMgr.getDraft(It.first())) {

[PATCH] D66723: [clangd] Add a distinct highlighting for local variables

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:38 Primitive, + LocalVariable, NIT: maybe put it right after `Variable`? IIUC, we do not rely on actual numeric values being the same across different clangd v

[PATCH] D66731: [Driver] Add an option for createInvocationFromCommandLine to recover on errors

ilya-biryukov created this revision. ilya-biryukov added a reviewer: gribozavr. Herald added subscribers: kadircet, arphaman, jkorous. Herald added a project: clang. Previously, it would always return nullptr on any error. This change adds a parameter, controlling whether the function should attem

[PATCH] D66747: Moved GlobList into a separate header file

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66747/new/ https://reviews.llvm.org/D66747 _

[PATCH] D66759: [clangd] Surface errors from command-line parsing

ilya-biryukov created this revision. ilya-biryukov added a reviewer: kadircet. Herald added subscribers: arphaman, jkorous, MaskRay, javed.absar. Herald added a project: clang. ilya-biryukov added a parent revision: D66731: [Driver] Add an option for createInvocationFromCommandLine to recover on e

[PATCH] D66731: [Driver] Add an option for createInvocationFromCommandLine to recover on errors

ilya-biryukov added a comment. In D66731#1644840 , @gribozavr wrote: > - In the doc comment for `CompilerInvocation::CreateFromArgs`, could you add > a note that it does a best effort to provide a CompilerInvocation even if it > returns false? Right no

[PATCH] D66731: [Driver] Add an option for createInvocationFromCommandLine to recover on errors

ilya-biryukov updated this revision to Diff 217319. ilya-biryukov added a comment. - Add a comment to CreateFromArgs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66731/new/ https://reviews.llvm.org/D66731 Files: clang-tools-extra/clangd/Compile

[PATCH] D66731: [Driver] Add an option for createInvocationFromCommandLine to recover on errors

ilya-biryukov updated this revision to Diff 217320. ilya-biryukov added a comment. - Fix english grammar in the comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66731/new/ https://reviews.llvm.org/D66731 Files: clang-tools-extra/clangd/Comp

[PATCH] D66787: GlobList: added a clear test for pattern priority

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66787/new/ https://reviews.llvm.org/D66787 _

[PATCH] D66788: Refactor GlobList from an ad-hoc linked list to a vector

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang-tools-extra/clang-tidy/GlobList.cpp:46 +GlobList::GlobList(StringRef Globs) { + do { +GlobListItem Item; NIT: I sug

[PATCH] D66751: [clangd] Add targetDecl(), which determines what declaration an AST node refers to.

ilya-biryukov added a comment. Mostly LGTM, thanks! Just a few clarifying questions and suggestions This API allows us to get a target declaration for a reference, but does not help with finding the source locations for those references. Do you plan to put this into the API? Have this as a sepa

[PATCH] D66759: [clangd] Surface errors from command-line parsing

ilya-biryukov added a comment. In D66759#1646509 , @kadircet wrote: > Thanks! This looks really useful when we can't build an AST due to unknown > compiler commands, but I am not sure about how useful it is to surface > command-line parsing errors once w

[PATCH] D66731: [Driver] Add an option for createInvocationFromCommandLine to recover on errors

ilya-biryukov updated this revision to Diff 217335. ilya-biryukov marked 2 inline comments as done. ilya-biryukov added a comment. - Start a paragrah with \returns instead of putting it in the middle Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D667

[PATCH] D66516: [clangd] Added highlighting to types dependant on templates.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:177 return; +if (TP->isPointerType() || TP->isLValueReferenceType()) + // When highlighting dependant template types the type can be a pointer or j

[PATCH] D66731: [Driver] Add an option for createInvocationFromCommandLine to recover on errors

ilya-biryukov added inline comments. Comment at: clang/include/clang/Frontend/CompilerInvocation.h:150 /// + /// If an error was encountered while parsing the arguments, \returns false + /// and attempts to recover and continue parsing the rest of the arguments.

[PATCH] D66797: ArrayRef'ized CompilerInvocation::CreateFromArgs

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM. Yes, please! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66797/new/ https://reviews.llvm.org/D66797 ___

[PATCH] D66759: [clangd] Surface errors from command-line parsing

ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. Just to clarify: we only want to surface errors, not warnings from command-line to avoid too much nosie; I'm totally on board with not spamming users with "unknown compiler warning" errors at the start of each file. W

[PATCH] D66759: [clangd] Surface errors from command-line parsing

ilya-biryukov updated this revision to Diff 217579. ilya-biryukov marked 2 inline comments as done and an inline comment as not done. ilya-biryukov added a comment. - Do not flushDiag() on EndSourceFile - Do not reset WasAdjusted outside flusLastDiag() - Add a test that unknown warnings do not pr

[PATCH] D66759: [clangd] Surface errors from command-line parsing

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:465 +Diags.insert(Diags.end(), Preamble->Diags.begin(), Preamble->Diags.end()); + // Finally, add diagnostics coming from the AST. + { kadircet wrote: > ilya-biryukov w

[PATCH] D66759: [clangd] Surface errors from command-line parsing

ilya-biryukov updated this revision to Diff 217581. ilya-biryukov added a comment. - Remove accidental change Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66759/new/ https://reviews.llvm.org/D66759 Files: clang-tools-extra/clangd/ClangdServer.c

[PATCH] D66516: [clangd] Added highlighting to types dependant on templates.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:177 return; +if (TP->isPointerType() || TP->isLValueReferenceType()) + // When highlighting dependant template types the type can be a pointer or i

[PATCH] D66875: [Index] Marked a bunch of classes 'final'

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66875/new/ https://reviews.llvm.org/D66875 _

[PATCH] D66876: Indexing: create PP callbacks in the ASTConsumer

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66876/new/ https://reviews.llvm.org/D66876 _

[PATCH] D66877: Moved the IndexDataConsumer::finish call into the IndexASTConsumer from IndexAction

ilya-biryukov added inline comments. Comment at: clang/lib/Index/IndexingAction.cpp:77 +IndexCtx->getDataConsumer().setPreprocessor(PP); +PP->addPPCallbacks(std::make_unique(IndexCtx)); + } The fact that we call `addPPCallbacks` **after** creating `ASTCo

[PATCH] D66878: [Index] Stopped wrapping FrontendActions in libIndex and its users

ilya-biryukov added a comment. Overall LG, thanks! Not sure why we need to keep `IndexingOptions` everywhere, though, see the relevant comment. Comment at: clang-tools-extra/clangd/index/IndexAction.cpp:217 std::unique_ptr Includes; + index::IndexingOptions Opts; std::u

[PATCH] D66878: [Index] Stopped wrapping FrontendActions in libIndex and its users

ilya-biryukov accepted this revision. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang-tools-extra/clangd/index/IndexAction.cpp:217 std::unique_ptr Includes; + index::Index

[PATCH] D66877: Moved the IndexDataConsumer::finish call into the IndexASTConsumer from IndexAction

ilya-biryukov accepted this revision. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/lib/Index/IndexingAction.cpp:77 +IndexCtx->getDataConsumer().setPreprocessor(PP); +

[PATCH] D66928: [clangd] Collecting main file macro expansion locations in ParsedAST.

ilya-biryukov added a comment. Looks great, just requests for comments and more test-cases from my side Comment at: clang-tools-extra/clangd/ClangdUnit.h:148 + /// All macro expansion locations in the main file. + std::vector MainFileMacroExpLocs; NIT: clar

[PATCH] D66751: [clangd] Add targetDecl(), which determines what declaration an AST node refers to.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. In D66751#1646991 , @sammccall wrote: > Yes. I actually ripped this out of the patch, because it wasn't related > enough and the patch is large. Makes sense and thanks for summing it u

[PATCH] D66928: [clangd] Collecting main file macro expansion locations in ParsedAST.

ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:107 +class CollectMainFileMacroExpansions : public PPCallbacks { + const SourceManager &SM; Maybe make this part of `Collec

[PATCH] D66928: [clangd] Collecting main file macro expansion locations in ParsedAST.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:107 +class CollectMainFileMacroExpansions : public PPCallbacks { + const SourceManager &SM; jvikstrom wrote: > ilya-biryukov wrote: > > Maybe make this part of `CollectMai

[PATCH] D66947: Changed FrontendActionFactory::create to return a std::unique_ptr

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66947/new/ https://reviews.llvm.org/D66947

[PATCH] D66990: [clangd] Add distinct highlightings for declarations of functions and methods

ilya-biryukov added a comment. In D66990#1652172 , @jvikstrom wrote: > Should we have different highlightings for declarations vs usages? (Although > I guess in the end it will be up to the editor if they highlight them > differently as the scope is just

[PATCH] D66928: [clangd] Collecting main file macro expansion locations in ParsedAST.

ilya-biryukov accepted this revision. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM, thanks! Comment at: clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp:260 +// Macros from token c

[PATCH] D66960: [Tooling] Migrated APIs that take ownership of objects to unique_ptr

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM from my side. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66960/new/ https://reviews.llvm.org/D66960 ___

[PATCH] D66516: [clangd] Added highlighting to types dependant on templates.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:177 return; +if (TP->isPointerType() || TP->isLValueReferenceType()) + // When highlighting dependant template types the type can be a pointer or j

[PATCH] D66738: [clangd] Added highlighting for structured bindings.

ilya-biryukov added a comment. In D66738#1652285 , @hokein wrote: > I don't have a strong preference on how to highlight `BindingDecl`, but I > think highlighting them as `Variable` is better than no highlighting them at > all, so LGTM from myside. @ilya

[PATCH] D66995: [clangd] Add highlighting for macro expansions.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:35 +TraverseAST(AST.getASTContext()); +// Add highlightings for Macro Expansions as they are not traversed by the +// visitor. NIT: uncapitalize and use

[PATCH] D66990: [clangd] Add distinct highlightings for declarations of functions and methods

ilya-biryukov added a comment. In D66990#1653833 , @nridge wrote: > There is precedent for highlighting declarations and uses differently in > other C++ editors. For example, Eclipse CDT has separate highlightings for > function and functions declaration

[PATCH] D66990: [clangd] Add distinct highlightings for declarations of functions and methods

ilya-biryukov added a comment. In D66990#1653833 , @nridge wrote: > I think it makes sense to style function declarations differently from > function uses for emphasis. For example, you can use the same color for both, > but make the declarations bold. I

[PATCH] D66990: [clangd] Add distinct highlightings for declarations of functions and methods

ilya-biryukov added a comment. In D66990#1655030 , @nridge wrote: > I was hoping though that a patch like this, which would bring us largely to > parity with Eclipse CDT's highlightings, wouldn't need to blocked on a change > to the upstream protocol pro

[PATCH] D67096: [clangd][vscode] Add a flag to enable semantic highlighting in clangd

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts:113 const semanticHighlightingFeature = - new semanticHighlighting.SemanticHighlightingFeature(); + new semanticHighlighting.SemanticHighlightingFeature(

[PATCH] D67096: [clangd][vscode] Add a flag to enable semantic highlighting in clangd

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts:113 const semanticHighlightingFeature = - new semanticHighlighting.SemanticHighlightingFeature(); + new semanticHighlighting.SemanticHighlightingFeature(

[PATCH] D67096: [clangd][vscode] Add a flag to enable semantic highlighting in clangd

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts:113 const semanticHighlightingFeature = - new semanticHighlighting.SemanticHighlightingFeature(); + new semanticHighlighting.SemanticHighlightingFeature(

[PATCH] D67117: [clangd] Split Preamble.h out of ClangdUnit.h. NFC

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67117/new/ https://reviews.llvm.org/D67117 _

[PATCH] D67163: [Driver] Use shared singleton instance of DriverOptTable

ilya-biryukov created this revision. ilya-biryukov added reviewers: gribozavr, sammccall. Herald added a subscriber: kadircet. Herald added a project: clang. This significantly reduces the time required to run clangd tests, by ~10%. Should also have an effect on other tests that run command-line

[PATCH] D66990: [clangd] Add distinct highlightings for declarations of functions and methods

ilya-biryukov added a comment. In D66990#1656230 , @nridge wrote: > Not a hard requirement, just a nice-to-have for someone moving from one tool > to another :) > If you feel that for now it's better not to do this, I can respect that. Thanks, if that

<    20   21   22   23   24   25   26   27   28   29   >