[PATCH] D62954: [Syntax] Add a helper to find expansion by its first spelled token

ilya-biryukov updated this revision to Diff 205374. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Added a message to the assertion Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62954/new/ https://reviews.llvm.org/

[PATCH] D62953: [Syntax] Do not glue multiple empty PP expansions to a single mapping

ilya-biryukov updated this revision to Diff 205377. ilya-biryukov added a comment. - Added comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62953/new/ https://reviews.llvm.org/D62953 Files: clang/include/clang/Tooling/Syntax/Tokens.h cl

[PATCH] D62953: [Syntax] Do not glue multiple empty PP expansions to a single mapping

ilya-biryukov added a comment. In D62953#1547799 , @sammccall wrote: > Can you explain why this is important? > (in the code) I've added a few comments into the code that builds token buffers, but I couldn't figure out a good place to mention this in t

[PATCH] D62956: [clangd] Collect tokens of main files when building the AST

ilya-biryukov updated this revision to Diff 205387. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Rename tokens() to getTokens() for consistency with the rest of ParsedAST. - Update comments. - Add a test. Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D62956: [clangd] Collect tokens of main files when building the AST

ilya-biryukov added a comment. In D62956#1547826 , @sammccall wrote: > Can we add a test using TestTU that does a very basic verification of > expanded/spelled tokens (first after preamble, last token in file)? Done. Will land this tomorrow, want to run

[PATCH] D62953: [Syntax] Do not glue multiple empty PP expansions to a single mapping

ilya-biryukov marked 7 inline comments as done. ilya-biryukov added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:318 + /// 2. macro name and arguments for macro expansions. + using SpelledMappings = + llvm::DenseMap; sammccall

[PATCH] D62953: [Syntax] Do not glue multiple empty PP expansions to a single mapping

ilya-biryukov updated this revision to Diff 205537. ilya-biryukov marked 3 inline comments as done. ilya-biryukov added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62953/new/ https://reviews.llvm.org/D62953 Files:

[PATCH] D62953: [Syntax] Do not glue multiple empty PP expansions to a single mapping

ilya-biryukov added a comment. To clarify, I don't think there are new concepts in this patch. Previously, we only took information from source locations into account when building token buffers. That works fine in most cases, but not enough to find the boundaries of empty macro expansions. In o

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov updated this revision to Diff 205589. ilya-biryukov added a comment. - Do the renames Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61637/new/ https://reviews.llvm.org/D61637 Files: clang/include/clang/Tooling/Syntax/BuildTree.h

[PATCH] D63562: [clangd] Format changes produced by rename

ilya-biryukov created this revision. ilya-biryukov added reviewers: hokein, kadircet, sammccall. Herald added subscribers: arphaman, jkorous, MaskRay. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D63562 Files: clang-tools-extra/clangd/ClangdServ

[PATCH] D64576: [Syntax] Do not add a node for 'eof' into the tree

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added subscribers: kristof.beyls, javed.absar. Herald added a project: clang. While useful as a sentinel value when iterating over tokens, having 'eof' in the tree, seems to do more harm than good. Repository:

[PATCH] D64634: [clangd] Fix duplicate highlighting tokens appearing in initializer lists

ilya-biryukov added a comment. Have we tried figuring out why `RecursiveASTVisitor` visits the argument lists twice? Is that an expected behavior? Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:37 +llvm::sort(Tokens, + [](const HighlightingToke

[PATCH] D64634: [clangd] Fix duplicate highlighting tokens appearing in initializer lists

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM (I think @hokein 's comments were addressed too) > So it seems to be expected. (and looking at the documentation for > InitListExpr it seems to be difficult to change Recurs

[PATCH] D64747: [clangd] Skip implicit nodes when traversing for highlightings

ilya-biryukov created this revision. ilya-biryukov added reviewers: jvikstrom, hokein. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay. Herald added a project: clang. Implicit code adds corner cases to handle and does not provide any value as those nodes cannot be mapped back to sou

[PATCH] D64747: [clangd] Skip implicit nodes when traversing for highlightings

ilya-biryukov abandoned this revision. ilya-biryukov added a comment. Ah, sorry, that's actually the default... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64747/new/ https://reviews.llvm.org/D64747

[PATCH] D64518: [LibTooling] Relax Transformer to allow rewriting macro expansions

ilya-biryukov added a comment. This clearly increases the utility of the library, but also seems to add corner cases that the library won't handle (see the comment about unittests for an example). WDYT about those? Are they important, should we support producing warnings in those cases to let t

[PATCH] D64762: [AST] Treat semantic form of InitListExpr as implicit code in traversals

ilya-biryukov created this revision. ilya-biryukov added a reviewer: gribozavr. Herald added a subscriber: kadircet. Herald added a project: clang. In particular, do not traverse the semantic form shouldVisitImplicitCode() returns false. This simplifies the common case of traversals, avoiding the

[PATCH] D64762: [AST] Treat semantic form of InitListExpr as implicit code in traversals

ilya-biryukov added inline comments. Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:2332 S->isSemanticForm() ? S->getSyntacticForm() : S, Queue)); TRY_TO(TraverseSynOrSemInitListExpr( S->isSemanticForm() ? S : S->getSemanticForm(), Queue)); ---

[PATCH] D64762: [AST] Treat semantic form of InitListExpr as implicit code in traversals

ilya-biryukov updated this revision to Diff 210052. ilya-biryukov added a comment. - Add a test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64762/new/ https://reviews.llvm.org/D64762 Files: clang/include/clang/AST/RecursiveASTVisitor.h clan

[PATCH] D64741: [clangd] Added highlighting for tokens that are macro arguments.

ilya-biryukov added a comment. How should this behave when if the same token is used multiple times inside a macro and the uses are different? Could we add this to tests? E.g. #define W(a) class a { void test() { int a = 10; } } W(foo); // <-- `foo` is a variable of a class? Repository:

[PATCH] D64762: [AST] Treat semantic form of InitListExpr as implicit code in traversals

ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments. Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:2332 S->isSemanticForm() ? S->getSyntacticForm() : S, Queue)); TRY_TO(TraverseSynOrSemInitListExpr( S->isSemanticForm()

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

ilya-biryukov created this revision. ilya-biryukov added reviewers: rnk, sammccall. Herald added a subscriber: kadircet. Herald added a project: clang. Instead of asserting all typos are corrected in the sema destructor. The sema destructor is not run in the common case of running the compiler wi

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

ilya-biryukov updated this revision to Diff 210105. ilya-biryukov added a comment. Fix a typo (xD) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64799/new/ https://reviews.llvm.org/D64799 Files: clang/lib/Sema/Sema.cpp clang/test/SemaObjC/typo

[PATCH] D64741: [clangd] Added highlighting for tokens that are macro arguments.

ilya-biryukov added a comment. > I had completely missed that there could be conflicting tokens when only > highlighting macro arguments as well. Added code to just remove conflicting > tokens. Picking one of the highlightings looks fine, but we probably want to make sure it's deterministic. G

[PATCH] D64741: [clangd] Added highlighting for tokens that are macro arguments.

ilya-biryukov added a comment. Thanks for pointing out I missed stuff. The strategy for conflicting highlightings LG, just a few NITs left from my side. In D64741#1589144 , @jvikstrom wrote: > Already added the case you sent a comment on in the test case

[PATCH] D64861: [clang-tidy] Adjust location of namespace comment diagnostic

ilya-biryukov created this revision. ilya-biryukov added reviewers: alexfh, hokein. Herald added subscribers: kadircet, xazax.hun. Herald added a project: clang. If there is no comment, place it at the closing brace of a namespace definition. Previously it was placed at the next character after th

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

ilya-biryukov added a comment. I tried to find a good place to emit unresolved typos earlier (to make sure CodeGen does not ever get `TypoExpr`), but couldn't find one. Please let me know if there's some obvious place I'm missing. Also open to suggestions for putting assertions on whether codege

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

ilya-biryukov updated this revision to Diff 210338. ilya-biryukov added a comment. - Remove -disable-free from the test, it is no longer required to workaround the crash Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64799/new/ https://reviews.llvm

[PATCH] D64863: [clangd] Ignore diags from builtin files

ilya-biryukov added a comment. Thanks for switching to `SM` everywhere, makes the code much more readable! A rough proposal for testing this: // test.cpp // RUN: clang++ -DFOO=b ./test.cpp int a = FOO; This produces a note inside a `` buffer. We could probably have something similar poin

[PATCH] D64518: [LibTooling] Relax Transformer to allow rewriting macro expansions

ilya-biryukov added a comment. In D64518#1588092 , @ymandel wrote: > This seems like a good candidate for configuration -- the user could then > choose which mode to run in. But, I'm also open to just reporting these > conditions as errors. It's alread

[PATCH] D64518: [LibTooling] Relax Transformer to allow rewriting macro expansions

ilya-biryukov added a comment. In D64518#1589857 , @ymandel wrote: > In D64518#1589768 , @ilya-biryukov > wrote: > > > In D64518#1588092 , @ymandel wrote: > > > > > This see

[PATCH] D64863: [clangd] Ignore diags from builtin files

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/Diagnostics.cpp:473 + SourceManager &SM = Info.getSourceManager(); + if (!InsideMainFile && SM.isWrittenInBuiltinFile(Info.getLocation())) { +IgnoreDiagnostics::log(DiagLevel, Info);

[PATCH] D64907: [AST] Traverse attributes inside DEF_TRAVERSE_DECL macro

ilya-biryukov created this revision. ilya-biryukov added reviewers: rsmith, gribozavr. Herald added a project: clang. Instead of traversing inside the TraverseDecl() function. Previously the attributes were traversed after Travese(Some)Decl returns. Logically attributes are properties of particul

[PATCH] D64907: [AST] Traverse attributes inside DEF_TRAVERSE_DECL macro

ilya-biryukov updated this revision to Diff 210507. ilya-biryukov added a comment. - Visit attributes before visiting the Decl Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64907/new/ https://reviews.llvm.org/D64907 Files: clang/include/clang/AS

[PATCH] D64907: [AST] Traverse attributes inside DEF_TRAVERSE_DECL macro

ilya-biryukov added a comment. Wanted to write unit tests, but couldn't find any that check traversal order. I'm aware of `Tooling/RecursiveASTVisitorTests`, but they mostly check that implicit nodes are visited. Do we have other tests for traversals? Repository: rG LLVM Github Monorepo CHA

[PATCH] D64918: [ASTUnit] Fix a regression in cached completions

ilya-biryukov created this revision. ilya-biryukov added a reviewer: kadircet. Herald added a subscriber: arphaman. Herald added a project: clang. After r345152 cached completions started adding namespaces after nested name specifiers, e.g. in `some_name::^` The CCC_Symbol indicates the completed

[PATCH] D64915: [clangd] cleanup: unify the implemenation of checking a location is inside main file.

ilya-biryukov added a comment. Neat! Many thanks, that's a very useful cleanup. A few suggestions from my side. Comment at: clang-tools-extra/clangd/SourceCode.cpp:332 +bool isInsideMainFile(SourceLocation Loc, const SourceManager &SM) { + return Loc.isValid() && SM.isWrittenI

[PATCH] D64627: [clangd] Suppress unwritten scopes when expanding auto.

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

[PATCH] D64918: [ASTUnit] Fix a regression in cached completions

ilya-biryukov updated this revision to Diff 210555. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - s/preamble/cached completions (in tests) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64918/new/ https://reviews.l

[PATCH] D64918: [ASTUnit] Fix a regression in cached completions

ilya-biryukov added inline comments. Comment at: clang/test/Index/complete-qualified-with-preamble.cpp:7 +// START-OF-LINE: Class +// -- With preambles. +// RUN: CINDEXTEST_EDITING=1 c-index-test -code-completion-at=%s:3:1 %s \ sammccall wrote: > Nit: This (and t

[PATCH] D64475: [clangd] Duplicate lines of semantic highlightings sent removed.

ilya-biryukov added a comment. A few suggestions from me, I haven't looked into the diffing algorithm and the tests yet. Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:458 +// edit there are stale previous highlightings. +std::lock_guard Lock(HighlightingsMut

[PATCH] D64475: [clangd] Duplicate lines of semantic highlightings sent removed.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:274 +std::vector +diffHighlightings(ArrayRef NewHighlightings, + ArrayRef OldHighlightings) { NIT: maybe rename to `New` and `Old`, the suffix of t

[PATCH] D64475: [clangd] Duplicate lines of semantic highlightings sent removed.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:307 +TEST(SemanticHighlighting, HighlightingDiffer) { + struct { Can we test this in a more direct manner by specifying: 1. annotated input for ol

[PATCH] D64475: [clangd] Duplicate lines of semantic highlightings sent removed.

ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:458 +// edit there are stale previous highlightings. +std::lock_guard Lock(HighlightingsMutex); +FileToHighlightings.erase(Fil

[PATCH] D64915: [clangd] cleanup: unify the implemenation of checking a location is inside main file.

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/SourceCodeTests.cpp:448 + }; + for (const auto *HeaderDecl : {"Header1", "Header2", "Header3"}) +EXPEC

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

ilya-biryukov added a comment. @rsmith, I'll look into emitting the typos when we pop expression evaluation context, but do we expect this to cover **all** cases where `TypoExpr`s are produced? (conservatively assuming that the answer is "no") should we land this patch and also emit at the end

[PATCH] D64518: [LibTooling] Relax Transformer to allow rewriting macro expansions

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

[PATCH] D64924: [LibTooling] Add function to translate and validate source range for editing

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/include/clang/Tooling/Refactoring/SourceCode.h:80 +llvm::Optional +getRangeForEdit(const CharSourceRange &EditRange, const ASTContext &Co

[PATCH] D64985: [clangd] Provide a way to publish highlightings in non-racy manner

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, javed.absar. Herald added a project: clang. By exposing a callback that can guard code publishing results of 'onMainAST' callback in the same manner we gu

[PATCH] D64985: [clangd] Provide a way to publish highlightings in non-racy manner

ilya-biryukov updated this revision to Diff 210805. ilya-biryukov added a comment. - Use the same mechanism for diagnostics - Change typedef to function)> - Update a comment - s/PublishResults/PublishFn - Reformat Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D64985: [clangd] Provide a way to publish highlightings in non-racy manner

ilya-biryukov updated this revision to Diff 210807. ilya-biryukov added a comment. - Update usage of DiagsMu in a comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64985/new/ https://reviews.llvm.org/D64985 Files: clang-tools-extra/clangd/Cl

[PATCH] D64985: [clangd] Provide a way to publish highlightings in non-racy manner

ilya-biryukov updated this revision to Diff 210808. ilya-biryukov added a comment. - Remove a leftover comment from the previous version Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64985/new/ https://reviews.llvm.org/D64985 Files: clang-tools-

[PATCH] D64985: [clangd] Provide a way to publish highlightings in non-racy manner

ilya-biryukov added a comment. Thanks for all the suggestions. This is ready for the next round now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64985/new/ https://reviews.llvm.org/D64985 ___ cfe-com

[PATCH] D64638: [CrossTU] Fix plist macro expansion if macro in other file.

ilya-biryukov added a comment. `StaticAnalyzer/Core` does not depend on `clangFrontend` now, you can see this by looking at `lib/StaticAnalyzer/Core/CMakeLists.txt`: add_clang_library(clangStaticAnalyzerCore ... LINK_LIBS clangAST clangASTMatchers clangAnalysis clangBasic

[PATCH] D64475: [clangd] Duplicate lines of semantic highlightings sent removed.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:307 +llvm::StringRef NewCode; +std::vector DiffedLines; + } TestCases[]{ @hokein rightfully pointed out that mentioning all changed lines ma

[PATCH] D64985: [clangd] Provide a way to publish highlightings in non-racy manner

ilya-biryukov updated this revision to Diff 210823. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Group PublishFn with onMainAST Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64985/new/ https://reviews.llvm.org/D6

[PATCH] D64475: [clangd] Duplicate lines of semantic highlightings sent removed.

ilya-biryukov added a comment. The fix for a race condition on remove has landed in rL366577 , this revision would need a small update after it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64475/new/ https://r

[PATCH] D64863: [clangd] Ignore diags from builtin files

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/Diagnostics.cpp:601 - if (mentionsMainFile(*LastDiag) || - (LastDiag->Severity >= DiagnosticsEngine::Level::Error && - IncludeLinesWithErrors.insert(LastDiag->Range.start.line).second)) { --

[PATCH] D64863: [clangd] Ignore diags from builtin files

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/Diagnostics.cpp:128 +return false; + Position StartPos = sourceLocToPosition(SM, IncludeInMainFile); NIT: inline `StartPos`, it has online a single usage now. Comm

[PATCH] D64762: [AST] Treat semantic form of InitListExpr as implicit code in traversals

ilya-biryukov added inline comments. Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:2332 S->isSemanticForm() ? S->getSyntacticForm() : S, Queue)); TRY_TO(TraverseSynOrSemInitListExpr( S->isSemanticForm() ? S : S->getSemanticForm(), Queue)); ---

[PATCH] D64762: [AST] Treat semantic form of InitListExpr as implicit code in traversals

ilya-biryukov updated this revision to Diff 211035. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Rewrite code as suggested in the review Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64762/new/ https://reviews.ll

[PATCH] D64863: [clangd] Ignore diags from builtin files

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/Diagnostics.cpp:563 FillDiagBase(*LastDiag); -adjustDiagFromHeader(*LastDiag, Info, *LangOpts); +if (!InsideMainFile) + LastDiagWasAdjusted = adjustDiagFromHeader(*LastDiag, Info, *LangOpt

[PATCH] D65263: [clangd] a prototype for triggering rename after extracting.

ilya-biryukov added a comment. If we are going down that path, could we consider to pre-selecting a name of the extract variable and its usage (using multi-select)? I.e. the optimal workflow seems to be (`[[code]] marks selected regions`): // Input: foo([[10+10]]); // 'extract' turns

[PATCH] D65387: [clangd] Add a callback mechanism for handling responses from client.

ilya-biryukov added a comment. Could you add a bit more context to the description of the change: why do we need the callback and what is the problem with what we have now? I'm sure this was discussed offline and the change is justified, just wanted to make sure we write it down in the change hi

[PATCH] D64863: [clangd] Ignore diags from builtin files

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM. Many thanks for fixing this. Comment at: clang-tools-extra/clangd/Diagnostics.cpp:111 -void adjustDiagFromHeader(Diag &D, const clang::Diagnostic &Info,

[PATCH] D64475: [clangd] Duplicate lines of semantic highlightings sent removed.

ilya-biryukov added a comment. In D64475#1602195 , @jvikstrom wrote: > In D64475#1593481 , @ilya-biryukov > wrote: > > > The fix for a race condition on remove has landed in rL366577 > >

[PATCH] D64475: [clangd] Duplicate lines of semantic highlightings sent removed.

ilya-biryukov added a comment. Mostly final NITs from me, but there is an important one about removing the `erase` call on `didOpen`. Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:467 +std::lock_guard Lock(HighlightingsMutex); +FileToHighlightings.erase(File

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

ilya-biryukov added a comment. @rsmith, emitting the typos on pop expression evaluation context resulted in a bunch of failures when trying to transform the resulting errors, see a typical stacktrace below. It seems we can actually pop expression evaluation contexts between producing and corre

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

ilya-biryukov added a comment. Thanks for the suggestion. Sounds reasonably simple, I'll try this and report back with the result. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64799/new/ https://reviews.llvm.org/D64799

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

ilya-biryukov added a comment. Tried the suggested approach and ran into the issue described above. Either we make the diagnostics less useful ('did you mean foo::bar?' turns into 'unresolved identifier bar'; without mentioning the correction) or we even stop providing some corrections in addit

[PATCH] D64475: [clangd] Duplicate lines of semantic highlightings sent removed.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1125 +Old = std::move(FileToHighlightings[File]); +FileToHighlightings[File] = Highlightings; + } NIT: avoid copying (from `Highlightings` into the map) under a

[PATCH] D64475: [clangd] Duplicate lines of semantic highlightings sent removed.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:408 +} + } // namespace ilya-biryukov wrote: > Could you also add a separate test that checks diffs when the number of lines > is getting smaller (

[PATCH] D65263: [clangd] a prototype for triggering rename after extracting.

ilya-biryukov added a comment. In D65263#1606371 , @hokein wrote: > I agree that doing rename after extraction is not an ideal approach (both > correctness and latency), there should be other better ways to achieve this. > However as a LSP server, we ma

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

ilya-biryukov added a comment. An experiment with popping on expression evaluation context failed, I couldn't find a good way to fix the problems described above. Typo correction can and will run after the evaluation context where expression created is popped and the diagnostic we produce depend

[PATCH] D65517: [Preprocessor] Always discard body of #define if we failed to parse it

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added a project: clang. Preivously we would only discard it if we failed to parse parameter lists. If we do not consume the body, parser sees tokens inside directive. In turns, this leads to spurious diagnostics

[PATCH] D64907: [AST] Traverse attributes inside DEF_TRAVERSE_DECL macro

ilya-biryukov updated this revision to Diff 212591. ilya-biryukov added a comment. Herald added a subscriber: mgorny. - Add a test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64907/new/ https://reviews.llvm.org/D64907 Files: clang/include/clan

[PATCH] D64907: [AST] Traverse attributes inside DEF_TRAVERSE_DECL macro

ilya-biryukov updated this revision to Diff 212592. ilya-biryukov added a comment. - Reformat Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64907/new/ https://reviews.llvm.org/D64907 Files: clang/include/clang/AST/RecursiveASTVisitor.h clang/u

[PATCH] D64475: [clangd] Duplicate lines of semantic highlightings sent removed.

ilya-biryukov accepted this revision. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. LGTM from my side Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1125 +Old = std::move(FileToHighlightings[File]); +FileToHighlightings[File] =

[PATCH] D65510: [clangd] Fix implicit template instatiations appearing as topLevelDecls.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:68 + if (const auto *TD = dyn_cast(D)) +return TD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation; + return false; We also want to skip `TSK_ExplicitInsta

[PATCH] D65517: [Preprocessor] Always discard body of #define if we failed to parse it

ilya-biryukov updated this revision to Diff 212754. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Update a comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65517/new/ https://reviews.llvm.org/D65517 Files:

[PATCH] D65517: [Preprocessor] Always discard body of #define if we failed to parse it

ilya-biryukov added a subscriber: hans. ilya-biryukov added a comment. @hans, could you please merge rL367530 into the release? Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65517/new/ https://reviews.llvm.org/D65517 __

[PATCH] D65517: [Preprocessor] Always discard body of #define if we failed to parse it

ilya-biryukov added a comment. SG, just wanted to make sure it's on your list ;-) Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65517/new/ https://reviews.llvm.org/D65517 ___ cfe-commits mailing list cfe-commits@lists

[PATCH] D65510: [clangd] Fix implicit template instatiations appearing as topLevelDecls.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:68 + if (const auto *TD = dyn_cast(D)) +return TD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation; + return false; hokein wrote: > ilya-biryukov wrote: >

[PATCH] D65525: [clangd] Add new helpers to make tweak tests scale better. Convert most tests. NFC

ilya-biryukov added a comment. Neat, thanks for the cleanup! A few suggestions from me, no major comments. Comment at: clang-tools-extra/clangd/unittests/TweakTesting.cpp:74 +return true; + llvm::consumeError(PrepareResult.takeError()); + return false; Ma

[PATCH] D65591: [AST] Add a flag indicating if any subexpression had errors

ilya-biryukov created this revision. ilya-biryukov added a reviewer: rsmith. Herald added a subscriber: jfb. Herald added a project: clang. ilya-biryukov added a child revision: D65592: [Parser] Avoid spurious 'missing template' error in presence of typos. The only subexpression that is considere

[PATCH] D65592: [Parser] Avoid spurious 'missing template' error in presence of typos

ilya-biryukov created this revision. ilya-biryukov added a reviewer: rsmith. Herald added a project: clang. ilya-biryukov added a parent revision: D65591: [AST] Add a flag indicating if any subexpression had errors. Suppress those diagnostics if lhs of a member expression contains errors. Typo co

[PATCH] D65525: [clangd] Add new helpers to make tweak tests scale better. Convert most tests. NFC

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/unittests/TweakTesting.h:44 +// Snippet is an expression. +Expression, + }; ilya-biryukov wrote: > I wonder whether we could use `Function` and get rid of 'expression' mode > comp

[PATCH] D65625: [clangd] Allow the client to specify multiple compilation databases in the 'initialize' request

ilya-biryukov added a subscriber: sammccall. ilya-biryukov added a comment. Could this be handled outside clangd? If not, what are the complications? We currently have a capability to specify a path to a single `compile_commands.json`. I would expect clients using this extension (Theia?) also ha

[PATCH] D65525: [clangd] Add new helpers to make tweak tests scale better. Convert most tests. 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/D65525/new/ https://reviews.llvm.org/D65525 _

[PATCH] D65655: [clangd] Fix a crash when presenting values for Hover

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay. Herald added a project: clang. We should pass the expression type, not a variable type when printing the resulting value. Variable type may be different f

[PATCH] D65655: [clangd] Fix a crash when presenting values for Hover

ilya-biryukov added a subscriber: hans. ilya-biryukov added a comment. @hans, could we merge this commit into the release branch? Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65655/new/ https://reviews.llvm.org/D65655 __

[PATCH] D65517: [Preprocessor] Always discard body of #define if we failed to parse it

ilya-biryukov added a comment. Thanks! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65517/new/ https://reviews.llvm.org/D65517 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/ma

[PATCH] D65735: [AST] Fix RecursiveASTVisitor visiting implicit constructor initializers.

ilya-biryukov added a comment. The fix looks good. Comment at: clang/unittests/Tooling/RecursiveASTVisitorTests/InitListExprPreOrder.cpp:17 // syntactic and semantic form. class InitListExprPreOrderVisitor : public ExpectedLocationVisitor { Could you cr

[PATCH] D65735: [AST] Fix RecursiveASTVisitor visiting implicit constructor initializers.

ilya-biryukov added a comment. LGTM from my side, a few optional NITs. Feel free to land as soon as @hokein stamps. Comment at: clang/unittests/Tooling/RecursiveASTVisitorTests/ImplicitCtorInitializer.cpp:16 + +// Check to ensure that CXXCtorInitializer is not visited when imp

[PATCH] D65510: [clangd] Fix implicit template instatiations appearing as topLevelDecls.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:68 + if (const auto *TD = dyn_cast(D)) +return TD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation; + return false; jvikstrom wrote: > ilya-biryukov wrote:

[PATCH] D65510: [clangd] Fix implicit template instatiations appearing as topLevelDecls.

ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:68 + if (const auto *TD = dyn_cast(D)) +return TD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation; + return false; ilya-biryukov wrote: > jvikstrom wrote:

[PATCH] D65752: [Sema] Refactor LookupVisibleDecls. NFC

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added subscribers: kadircet, arphaman, jkorous. Herald added a project: clang. We accumulated some configuration parameters for LookupVisibleDecls that are being passed unchanged to recursive calls, e.g. LoadExt

[PATCH] D65752: [Sema] Refactor LookupVisibleDecls. NFC

ilya-biryukov updated this revision to Diff 213371. ilya-biryukov added a comment. - Remove accidental change from revision Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65752/new/ https://reviews.llvm.org/D65752 Files: clang/lib/Sema/SemaLookup

[PATCH] D65752: [Sema] Refactor LookupVisibleDecls. NFC

ilya-biryukov added a comment. This is just a proposal, there are probably other ways to reach better readability, e.g. group some of those parameters into a struct. But let me know what you think, happy to refactor in a slightly different manner or simply drop this revision. Repository: rG

[PATCH] D65796: [clangd] Compute scopes eagerly in IncludeFixer

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay. Herald added a project: clang. Computing lazily leads to crashes. In particular, computing scopes may produce diagnostics (from inside template instantiat

<    12   13   14   15   16   17   18   19   20   21   >