[PATCH] D67695: [clangd] Implement getBeginning for overloaded operators.

2019-09-26 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/SourceCode.cpp:286 - Before = Lexer::GetBeginningOfToken(Before, SM, LangOpts); - Token Tok; - if (Before.isValid() && - !Lexer::getRawToken(Before, Tok, SM, LangOpts, false) && - Tok.is(tok:

[PATCH] D66647: [clangd] DefineInline action apply logic with fully qualified names

2019-09-26 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. Currently we add too many qualifiers in some cases, e.g. here's a common pattern in clangd: // foo.h #include "Decl.h" namespace clang::clangd { std::string printName(Decl*D); } // foo.cpp #include "foo.h" namespace clang::clangd { std::string p

[PATCH] D68080: [clangd][vscode] Add npm helper commands to package/release the extension.

2019-09-26 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/clients/clangd-vscode/package.json:38 +"package": "vsce package --baseImagesUrl https://raw.githubusercontent.com/llvm/llvm-project/master/clang-tools-extra/clangd/clients/clangd-vscode/";, +

[PATCH] D66647: [clangd] DefineInline action apply logic with fully qualified names

2019-09-26 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. We also need to rename parameters sometimes, right? // Sometimes we need to rename parameters. void usages(int decl_param, int); void usages(int def_param, int now_named) { llvm::errs() << def_param + now_named; } // And template parameters! (the

[PATCH] D68080: [clangd][vscode] Add npm helper commands to package/release the extension.

2019-09-26 Thread Ilya Biryukov via Phabricator via cfe-commits
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/clients/clangd-vscode/package.json:38 +"package": "vsce package --baseImagesUrl https://raw.githubusercontent

[PATCH] D68024: [clangd] Implement GetEligiblePoints

2019-09-26 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/SourceCode.cpp:996 + std::vector Enclosing = {""}; + // FIXME: In addition to namespaces try to generate events for function + // definitions as well. One

[PATCH] D68118: [clangd] Support OverloadExpr in findExplicitReferences

2019-09-26 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov created this revision. ilya-biryukov added a reviewer: kadircet. Herald added subscribers: usaxena95, arphaman, jkorous, MaskRay. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D68118 Files: clang-tools-extra/clangd/FindTarget.cpp

[PATCH] D68119: [clangd] Handle OverloadExpr in targetDecl

2019-09-26 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added subscribers: usaxena95, kadircet, arphaman, jkorous, MaskRay. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D68119 Files: clang-tools-extra/clangd/FindTa

[PATCH] D68120: [clangd] Handle type template parameters in findExplicitReferences

2019-09-26 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov created this revision. ilya-biryukov added a reviewer: kadircet. Herald added subscribers: usaxena95, arphaman, jkorous, MaskRay. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D68120 Files: clang-tools-extra/clangd/FindTarget.cpp

[PATCH] D68124: [clangd] Fix template type aliases in findExplicitReference

2019-09-27 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov created this revision. ilya-biryukov added a reviewer: kadircet. Herald added subscribers: usaxena95, arphaman, jkorous, MaskRay. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D68124 Files: clang-tools-extra/clangd/FindTarget.cpp

[PATCH] D68120: [clangd] Handle type template parameters in findExplicitReferences

2019-09-27 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. In D68120#1685409 , @kadircet wrote: > LGTM. can you also add some tests for template template and non-type template > params ? Will do, was about to look into this. I would expect template template arguments to be broken,

[PATCH] D66647: [clangd] DefineInline action apply logic with fully qualified names

2019-09-27 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. In D66647#1685506 , @kadircet wrote: > I actually have a fixme saying: > > // FIXME: Instead of fully qualifying we should try deducing visible scopes > at > // target location and generate minimal edits. > > > Elaborati

[PATCH] D66647: [clangd] DefineInline action apply logic with fully qualified names

2019-09-27 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/unittests/TweakTesting.h:77 // - if apply() returns an error, returns "fail: " - std::string apply(llvm::StringRef MarkedCode) const; + std::string apply(llvm::StringRef MarkedCode); ---

[PATCH] D68124: [clangd] Fix template type aliases in findExplicitReference

2019-09-27 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/FindTarget.cpp:478 + explicitReferenceTargets(DynTypedNode::create(L.getType()), + DeclRelation::Alias)}; } --

[PATCH] D68072: Reference qualifiers in member templates causing extra indentation.

2019-09-27 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov reopened this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. The patch breaks some tests, see http://lab.llvm.org:8011/builders/clang-cmake-x86_64-sde-avx512-linux/builds/27891/steps/ninja%20check%201/logs/FAIL%3A%20Clang-Unit%3A%3AFormatTes

[PATCH] D68120: [clangd] Handle type template parameters in findExplicitReferences

2019-09-27 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 222126. ilya-biryukov added a comment. - Add tests for non-type template paremeters Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68120/new/ https://reviews.llvm.org/D68120 Files: clang-tools-extra/cla

[PATCH] D68137: [clangd] Handle template arguments in findExplicitReferences

2019-09-27 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov created this revision. ilya-biryukov added a reviewer: kadircet. Herald added subscribers: usaxena95, arphaman, jkorous, MaskRay. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D68137 Files: clang-tools-extra/clangd/FindTarget.cpp

[PATCH] D68137: [clangd] Handle template arguments in findExplicitReferences

2019-09-27 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/FindTarget.cpp:576 +case TemplateArgument::Expression: + break; // Handled by visited functions. +}; This comment is a bit unclear, I'll have to change it. The idea is that thi

[PATCH] D68124: [clangd] Fix template type aliases in findExplicitReference

2019-09-27 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 04. ilya-biryukov added a comment. - Add a comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68124/new/ https://reviews.llvm.org/D68124 Files: clang-tools-extra/clangd/FindTarget.cpp clang-to

[PATCH] D68143: [clang] Make handling of unnamed template params similar to function params

2019-09-27 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments. Comment at: clang/lib/AST/DeclTemplate.cpp:513 getDefaultArgumentInfo()->getTypeLoc().getEndLoc()); - else -return TypeDecl::getSourceRange(); + else if(getName().empty()) +return SourceRange(getBeginLoc()); -

[PATCH] D67695: [clangd] Implement getBeginning for overloaded operators.

2019-09-30 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. Mostly NITs, except the naming of the new `TokenKind` enum. I think it's better to pick something that's not clashing with `clang::tok::TokenKind`, even if the enum is in a different namespace. Comment at: clang-tools-extra/clangd/SourceCode.cpp:

[PATCH] D67695: [clangd] Implement getBeginning for overloaded operators.

2019-09-30 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. Mostly LGTM Comment at: clang-tools-extra/clangd/SourceCode.cpp:258 + +TokenKind getTokenKind(SourceLocation Loc, const SourceManager &SM, +

[PATCH] D67695: [clangd] Implement getBeginning for overloaded operators.

2019-10-01 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/SourceCode.cpp:256 + SourceLocation getBeginningOfIdentifier(const Position &Pos, const SourceManager &SM, hokein wrote: > the function name doesn'

[PATCH] D68137: [clangd] Handle template arguments in findExplicitReferences

2019-10-01 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov marked 6 inline comments as done. ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/FindTarget.cpp:556 + // We re-define Traverse*, since there's no corresponding Visit*. + bool TraverseTemplateArgumentLoc(TemplateArgumentLoc A) { ---

[PATCH] D68137: [clangd] Handle template arguments in findExplicitReferences

2019-10-01 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 222554. ilya-biryukov marked 2 inline comments as done. ilya-biryukov added a comment. - Restore dlog() - Check reference to function pointer non-type template parameters in tests - Extend comment of TraverseTemplateArgumentLoc Repository: rG LLVM Gi

[PATCH] D66647: [clangd] DefineInline action apply logic with fully qualified names

2019-10-01 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. In D66647#1686021 , @kadircet wrote: > So currently AST doesn't store any information regarding template parameter > locations except the deepest one. > Therefore I've changed the availability to discard any methods inside

[PATCH] D68143: [clang] Make handling of unnamed template params similar to function params

2019-10-01 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments. Comment at: clang/lib/AST/DeclTemplate.cpp:513 getDefaultArgumentInfo()->getTypeLoc().getEndLoc()); - else -return TypeDecl::getSourceRange(); + else if(getName().empty()) +return SourceRange(getBeginLoc()); -

[PATCH] D68143: [clang] Make handling of unnamed template params similar to function params

2019-10-01 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments. Comment at: clang/lib/AST/DeclTemplate.cpp:513 getDefaultArgumentInfo()->getTypeLoc().getEndLoc()); - else -return TypeDecl::getSourceRange(); + else if(getName().empty()) +return SourceRange(getBeginLoc()); -

[PATCH] D68143: [clang] Make handling of unnamed template params similar to function params

2019-10-01 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. Many thanks! LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68143/new/ https://reviews.llvm.org/D68143

[PATCH] D68273: [clangd] Fix raciness in code completion tests

2019-10-01 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. This is still racy if there were > 1 request. I wonder if there a better way to address this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68273/new/ https://reviews.llvm.org/D68273 __

[PATCH] D68273: [clangd] Fix raciness in code completion tests

2019-10-02 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. It just hit me that we should probably just wait for the async request to finish before returning from completion. We might be doing this to reduce latency a little, but I don't recall us measuring that this provides a significant performance win. Would that work?

[PATCH] D67901: [clangd] Improve semantic highlighting in dependent contexts (fixes #154)

2019-10-02 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:219 + bool VisitDependentNameTypeLoc(DependentNameTypeLoc L) { +addToken(L.getNameLoc(), HighlightingKind::DependentType); +return true; nridge wrote: > ho

[PATCH] D67901: [clangd] Improve semantic highlighting in dependent contexts (fixes #154)

2019-10-02 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:505 + case HighlightingKind::DependentType: +return "entity.name.type.dependent.cpp"; + case HighlightingKind::DependentName: Maybe have a separate category f

[PATCH] D68335: [CodeComplete] Ensure object is the same in compareOverloads()

2019-10-02 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added subscribers: usaxena95, kadircet, arphaman, jkorous. Herald added a project: clang. This fixes a regression that led to size() not being available in clangd when completing 'deque().^'. Repository: rG

[PATCH] D68335: [CodeComplete] Ensure object is the same in compareOverloads()

2019-10-04 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments. Comment at: clang/lib/Sema/SemaCodeComplete.cpp:1188 ExprValueKind ObjectKind) { + // This function does not take object type into account. + if (Candidate.getDeclContext() != Incumbent.getDeclContext

[PATCH] D68335: [CodeComplete] Ensure object is the same in compareOverloads()

2019-10-04 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 223160. 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/D68335/new/ https://reviews.llvm.org/D68335 Files:

[PATCH] D67536: [WIP] [clangd] Add support for an inactive regions notification

2019-10-04 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments. Comment at: clang-tools-extra/clangd/Compiler.cpp:66 CI->getLangOpts()->CommentOpts.ParseAllComments = true; + CI->getPreprocessorOpts().DetailedRecord = true; return CI; hokein wrote: > I'm not sure how does this flag

[PATCH] D68273: [clangd] Fix raciness in code completion tests

2019-10-04 Thread Ilya Biryukov via Phabricator via cfe-commits
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/unittests/CodeCompleteTests.cpp:1141 +ReceivedRequestCV.wait(Lock,

[PATCH] D45482: [clangd] Match AST and Index label for template Symbols

2018-04-13 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 142372. ilya-biryukov marked 2 inline comments as done. ilya-biryukov added a comment. - Also test plain text completion text - Clarify the comment - Simplify conditions in getTemplateOrThis Repository: rCTE Clang Tools Extra https://reviews.llvm.or

[PATCH] D45482: [clangd] Match AST and Index label for template Symbols

2018-04-13 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments. Comment at: unittests/clangd/FileIndexTests.cpp:218 + EXPECT_EQ(Sym.CompletionLabel, "vector"); + EXPECT_EQ(Sym.CompletionSnippetInsertText, "vector<${1:class Ty}>"); + SeenVector = true; sammccall wrote: > If s

[PATCH] D42966: Fix USR generation in the presence of #line directives or linemarkes

2018-04-17 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. Could you elaborate on why the old behaviour is broken? Repository: rC Clang https://reviews.llvm.org/D42966 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe

[PATCH] D45442: Parse .h files as objective-c++ if we don't have a compile command.

2018-04-19 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. Herald added a subscriber: jkorous. LGTM Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45442 ___ cfe-commits mail

[PATCH] D45095: [clang-tidy] Align usage of ClangTool interface with new VFS injection

2018-04-19 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments. Comment at: clang-tidy/ClangTidy.cpp:91 public: - ErrorReporter(ClangTidyContext &Context, bool ApplyFixes, -llvm::IntrusiveRefCntPtr BaseFS) - : Files(FileSystemOptions(), BaseFS), DiagOpts(new DiagnosticOptions()), +

[PATCH] D45478: [clangd] Merge symbols in global-sym-builder on the go

2018-04-19 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. Herald added a subscriber: jkorous. In https://reviews.llvm.org/D45478#1064027, @sammccall wrote: > Is this patch still relevant after haojian's string deduplication? Apparently it does. It has an advantage of distributing the work more evenly between the program

[PATCH] D45478: [clangd] Merge symbols in global-sym-builder on the go

2018-04-19 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments. Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:53 +/// Combines occurrences of the same symbols across translation units. +class SymbolMerger { ilya-biryukov wrote: > sammccall wrote: > > Seems reasonab

[PATCH] D45815: [libclang] Add options to limit skipping of function bodies

2018-04-20 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. In https://reviews.llvm.org/D45815#1072094, @nik wrote: > @ilya: Using SkipFunctionBodies_AllExceptTemplates for the preamble might be > also useful for clangd. Unfortunately, that's also the biggest performance win you get when skipping the function bodies. I.e

[PATCH] D45887: [CodeComplete] Fix completion at the end of keywords

2018-04-20 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov created this revision. ilya-biryukov added reviewers: sammccall, bkramer, arphaman, aaron.ballman. Make completion behave consistently no matter if it is run at the start, in the middle or at the end of an identifier that happens to be a keyword or a macro name. Since completion is o

[PATCH] D44932: [CodeComplete] Fix completion in the middle of ident in ctor lists.

2018-04-21 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 143435. ilya-biryukov added a comment. - Fixed failing assert on the end of file. Added a test for that. Repository: rC Clang https://reviews.llvm.org/D44932 Files: lib/Lex/Lexer.cpp test/CodeCompletion/ctor-initializer.cpp test/CodeCompletio

[PATCH] D44932: [CodeComplete] Fix completion in the middle of ident in ctor lists.

2018-04-21 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments. Comment at: lib/Lex/Lexer.cpp:1667-1668 + assert(CurPtr < BufferEnd && "eof at completion point"); + while (isIdentifierBody(*CurPtr)) +++CurPtr; + BufferPtr = CurPtr; aaron.ballman wrote: > You should c

[PATCH] D45815: [libclang] Add options to limit skipping of function bodies

2018-04-24 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. In https://reviews.llvm.org/D45815#1073581, @nik wrote: > Hmm, that indicates that template function bodies are actually not that > dominant, which I also haven't thought. Now I'm doubting the correctness of > my patch/tests. The numbers are definitely interesti

[PATCH] D45815: [libclang] Add options to limit skipping of function bodies

2018-04-24 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments. Comment at: include/clang/Frontend/ASTUnit.h:679 + struct SkipFunctionBodiesOptions { +SkipFunctionBodiesOptions() {} +enum { None, MainFileAndPreamble, Preamble } Scope = None; Default ctor will be generated automati

[PATCH] D45999: [clangd] Retrieve minimally formatted comment text in completion.

2018-04-24 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov created this revision. ilya-biryukov added reviewers: sammccall, hokein, ioeric. Herald added subscribers: jkorous, MaskRay, klimek. Previous implementation used to extract brief text from doxygen comments. Brief text parsing slows down completion and is not suited for non-doxygen co

[PATCH] D46000: [AST] Added a helper to extract a user-friendly text of a comment.

2018-04-24 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov created this revision. ilya-biryukov added reviewers: sammccall, hokein, ioeric. The helper is used in clangd for documentation shown in code completion and storing the docs in the symbols. See https://reviews.llvm.org/D45999. This patch reuses the code of the Doxygen comment lexer,

[PATCH] D46001: [CodeComplete] Expose helpers to get RawComment of completion result.

2018-04-24 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov created this revision. ilya-biryukov added reviewers: sammccall, hokein, ioeric. Used in clangd, see https://reviews.llvm.org/D45999. Repository: rC Clang https://reviews.llvm.org/D46001 Files: include/clang/Sema/CodeCompleteConsumer.h lib/AST/RawCommentList.cpp lib/Sema/

[PATCH] D46002: [clangd] Parse all comments in Sema and completion.

2018-04-24 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov created this revision. ilya-biryukov added reviewers: sammccall, hokein, ioeric. Herald added subscribers: jkorous, MaskRay, klimek. And add tests for the comment extraction code. clangd will now show non-doxygen comments in completion for results coming from Sema and Dynamic index.

[PATCH] D45887: [CodeComplete] Fix completion at the end of keywords

2018-04-24 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments. Comment at: lib/Lex/Lexer.cpp:1647 // looking up the identifier in the identifier table. IdentifierInfo *II = PP->LookUpIdentifierInfo(Result); sammccall wrote: > move this down to where it's used - it's ignored fo

[PATCH] D45887: [CodeComplete] Fix completion at the end of keywords

2018-04-24 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 143731. ilya-biryukov marked 3 inline comments as done. ilya-biryukov added a comment. - Added comments - Fix nits in tests Repository: rC Clang https://reviews.llvm.org/D45887 Files: lib/Lex/Lexer.cpp test/CodeCompletion/end-of-ident-macro.cpp

[PATCH] D42966: Fix USR generation in the presence of #line directives or linemarkes

2018-04-24 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. In https://reviews.llvm.org/D42966#1069674, @mikhail.ramalho wrote: > Sure. Basically, the previous code would not generate the USR for the > function's parameters. > > The issue was that SM.getFileEntryForID would return NULL because there is no > actual file, th

[PATCH] D42966: Fix USR generation in the presence of #line directives or linemarkes

2018-04-24 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. Sorry for the delay. In https://reviews.llvm.org/D42966#1069674, @mikhail.ramalho wrote: > Sure. Basically, the previous code would not generate the USR for the > function's parameters. > The issue was that SM.getFileEntryForID would return NULL because there is

[PATCH] D42966: Fix USR generation in the presence of #line directives or linemarkes

2018-04-25 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. In https://reviews.llvm.org/D42966#1077249, @mikhail.ramalho wrote: > They are declared in some file defined by the line markers; the file are > not registered in the SourceManager as actual files, so getting the > FileEntry will always fail, that's why I changed

[PATCH] D42966: Fix USR generation in the presence of #line directives or linemarkes

2018-04-25 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. > I assume all examples in the current patch will produce USRs even without > your changes, is this correct or am I missing something? Or is the that whenever there's a `#line` directive we get into a "virtual" file that's not registered in the `SourceManager`?

[PATCH] D41537: Optionally add code completion results for arrow instead of dot

2018-04-25 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. Important: please upload the patch with full context diff Comment at: include/clang-c/Index.h:5278 + /** + * \brief Whether to try dot to arrow correction if arrow operator can be applied. + */ This implies that "dot to arro

[PATCH] D46000: [AST] Added a helper to extract a user-friendly text of a comment.

2018-04-25 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 143881. ilya-biryukov added a comment. Added forgotten bits of the change Repository: rC Clang https://reviews.llvm.org/D46000 Files: include/clang/AST/CommentLexer.h include/clang/AST/RawCommentList.h lib/AST/CommentLexer.cpp lib/AST/RawCo

[PATCH] D46001: [CodeComplete] Expose helpers to get RawComment of completion result.

2018-04-25 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 143882. ilya-biryukov added a comment. Remove accidentally added changes that should be part of https://reviews.llvm.org/D46000 Repository: rC Clang https://reviews.llvm.org/D46001 Files: include/clang/Sema/CodeCompleteConsumer.h lib/Sema/Sema

[PATCH] D46001: [CodeComplete] Expose helpers to get RawComment of completion result.

2018-04-25 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a reviewer: arphaman. ilya-biryukov added a subscriber: arphaman. ilya-biryukov added a comment. In https://reviews.llvm.org/D46001#1077781, @ioeric wrote: > This seems to do what we want for clangd, but we should also get the code > owner or someone who knows the code better

[PATCH] D46002: [clangd] Parse all comments in Sema and completion.

2018-04-25 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments. Comment at: clangd/ClangdUnit.cpp:362 CI->getFrontendOpts().DisableFree = false; +CI->getLangOpts()->CommentOpts.ParseAllComments = true; } sammccall wrote: > Any idea about whether this will affect performance sig

[PATCH] D46000: [AST] Added a helper to extract a user-friendly text of a comment.

2018-04-25 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. In https://reviews.llvm.org/D46000#1077926, @ioeric wrote: > Overall looks good. Could you add tests for the new methods? Sure. There are a few tests in https://reviews.llvm.org/D46002, but I haven't (yet) moved them to clang. Repository: rC Clang https://re

[PATCH] D46000: [AST] Added a helper to extract a user-friendly text of a comment.

2018-04-25 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 143928. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Attempt to reuse lexing code with/without command parsing. - Get rid of SkipWs. Repository: rC Clang https://reviews.llvm.org/D46000 Files: include/clang/AST

[PATCH] D46000: [AST] Added a helper to extract a user-friendly text of a comment.

2018-04-25 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments. Comment at: lib/AST/CommentLexer.cpp:471 + case '\r': +TokenPtr = skipNewline(TokenPtr, CommentEnd); +formTokenWithChars(T, TokenPtr, tok::newline); ioeric wrote: > Can we share code with `lexCommentTextWi

[PATCH] D46000: [AST] Added a helper to extract a user-friendly text of a comment.

2018-04-25 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 143929. ilya-biryukov added a comment. - Update a comment after latest changes Repository: rC Clang https://reviews.llvm.org/D46000 Files: include/clang/AST/CommentLexer.h include/clang/AST/RawCommentList.h lib/AST/CommentLexer.cpp lib/AST/

[PATCH] D46000: [AST] Added a helper to extract a user-friendly text of a comment.

2018-04-25 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 143930. ilya-biryukov added a comment. - Fix indentation Repository: rC Clang https://reviews.llvm.org/D46000 Files: include/clang/AST/CommentLexer.h include/clang/AST/RawCommentList.h lib/AST/CommentLexer.cpp lib/AST/RawCommentList.cpp In

[PATCH] D44932: [CodeComplete] Fix completion in the middle of ident in ctor lists.

2018-04-25 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 143935. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Fix the comment Repository: rC Clang https://reviews.llvm.org/D44932 Files: lib/Lex/Lexer.cpp test/CodeCompletion/ctor-initializer.cpp test/CodeCompletio

[PATCH] D46000: [AST] Added a helper to extract a user-friendly text of a comment.

2018-04-25 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments. Comment at: lib/AST/RawCommentList.cpp:380 +SourceMgr.getSpellingColumnNumber(Tok.getLocation(), &LocInvalid); +if (LocInvalid) + TokColumn = 0; ilya-biryukov wrote: > ioeric wrote: > > Explain when this would

[PATCH] D46000: [AST] Added a helper to extract a user-friendly text of a comment.

2018-04-26 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 144083. ilya-biryukov marked 2 inline comments as done. ilya-biryukov added a comment. - Remove tryLexCommands(), call into helper that parses commands directly - Addressed other review comments Repository: rC Clang https://reviews.llvm.org/D46000

[PATCH] D46000: [AST] Added a helper to extract a user-friendly text of a comment.

2018-04-26 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov marked 4 inline comments as done. ilya-biryukov added inline comments. Comment at: include/clang/AST/RawCommentList.h:118 + /// // Parts of it might be indented. + /// /* The comments styles might be mixed. */ + /// into ioeric wrote:

[PATCH] D41537: Optionally add code completion results for arrow instead of dot

2018-04-26 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. Could you upload the patch with full context? Comment at: include/clang/Sema/CodeCompleteConsumer.h:704 +CXAvailabilityKind Availability, +const std::vector &Corrections) : Allocator(Allocator

[PATCH] D45815: [libclang] Add options to limit skipping of function bodies

2018-04-26 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov requested changes to this revision. ilya-biryukov added a comment. This revision now requires changes to proceed. > OK, I've rechecked this change. I don't see any obvious mistake :) I think I got to the bottom of it. We didn't expect a big win, because we expect people to not put

[PATCH] D42966: Fix USR generation in the presence of #line directives or linemarkes

2018-04-26 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added subscribers: sammccall, ioeric, hokein, bkramer. ilya-biryukov added a comment. In https://reviews.llvm.org/D42966#1079438, @mikhail.ramalho wrote: > The virtual file is actually registered in the SourceManager but the > FileEntry for it is NULL (USRGeneration.cpp:33), which

[PATCH] D46180: [clang-format] Refactor #include insertion/deletion functionality into a class.

2018-04-27 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. This change LG as an extraction of the helper functionality to be reused in clang, clang-tidy, etc. However, I feel there are potential improvements both to the underlying code and the new APIs that we could make. I left some comments, trying to focus on interface

[PATCH] D46183: [clangd] Incorporate #occurrences in scoring code complete results.

2018-04-27 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. the patch LG and fixes the problem of llvm namespace not showing up in completions from the static index. Let's add some tests and ship it! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46183 ___ cfe

[PATCH] D46180: [clang-format] Refactor #include insertion/deletion functionality into a class.

2018-05-02 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. > This is really a corner cases that users might not need to know about. But an > example is: > Code: "#include " (without \n at the end). After inserting , #include > \n#include \n (this is good). However, if you insert another , the > code would become "#includ

[PATCH] D46180: [clang-format] Refactor #include insertion/deletion functionality into a class.

2018-05-02 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments. Comment at: lib/Format/Format.cpp:1691 // 0. Otherwise, returns the priority of the matching category or INT_MAX. - int getIncludePriority(StringRef IncludeName, bool CheckMainHeader) { + int getIncludePriority(StringRef IncludeName, bool

[PATCH] D42966: Fix USR generation in the presence of #line directives or linemarkes

2018-05-02 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. In https://reviews.llvm.org/D42966#1085257, @mikhail.ramalho wrote: > Should we ignore linemarkers and use filename + offset of the real file? I would say "yes". Let's not rely on linemarkers, unless we can explain why that's a good idea. > Should we try to calc

[PATCH] D46183: [clangd] Incorporate #occurrences in scoring code complete results.

2018-05-03 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D45815: [libclang] Allow skipping function bodies in preamble only

2018-05-04 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. Sorry for the delay Repository: rC Clang https://reviews.llvm.org/D45815 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45815: [libclang] Allow skipping function bodies in preamble only

2018-05-04 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments. Comment at: include/clang/Frontend/ASTUnit.h:104 + enum class SkipFunctionBodiesScope { None, MainFileAndPreamble, Preamble }; + Maybe move this out of `ASTUnit`? Would allow removing the first qualifier in usages outside

[PATCH] D41537: Optionally add code completion results for arrow instead of dot

2018-05-04 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. Sorry for the delay. I really like the direction of this patch! What's left is defining the semantics of corrections more thoroughly to make sure we don't have tricky corner cases that users of the API can't deal with. PS. This patch is still lacking full context o

[PATCH] D46180: [clang-format] Refactor #include insertion/deletion functionality into a class.

2018-05-04 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. I somehow missed the review email, sorry for the delayed response. Just one nit and one question from me on changed behavior in the tests (quoted vs angled #include). Otherwise LG, just wanted to make sure the change in behavior is intentional.

[PATCH] D46180: [clang-format] Refactor #include insertion/deletion functionality into a class.

2018-05-04 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang https://reviews.llvm.org/D46180 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.l

[PATCH] D46524: [clangd] Extract scoring/ranking logic, and shave yaks.

2018-05-07 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. The change makes both testing and scoring code better. Even though those are largely independent changes, perfectly happy to review them together. I mostly have NITs here, overall the changes LG. Comment at: clangd/CodeComplete.cpp:36 +#define

[PATCH] D46524: [clangd] Extract scoring/ranking logic, and shave yaks.

2018-05-08 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. The new uploaded diff has lots of unrelated changes to clang-tidy, clang-move, etc... Looking at commits, it seems `arc diff` was called with the wrong base commit... Could you please reupload the change? Comment at: clangd/Quality.h:45 + +// A

[PATCH] D46000: [AST] Added a helper to extract a user-friendly text of a comment.

2018-05-08 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 145683. ilya-biryukov marked 3 inline comments as done. ilya-biryukov added a comment. Herald added a subscriber: mgorny. - Move unit tests from clangd code to AST tests - Assert locations are valid - Address review other comments Repository: rC Clan

[PATCH] D46000: [AST] Added a helper to extract a user-friendly text of a comment.

2018-05-08 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments. Comment at: lib/AST/RawCommentList.cpp:376 +SourceMgr.getSpellingColumnNumber(Tok.getLocation(), &LocInvalid); +if (LocInvalid) + TokColumn = 0; ioeric wrote: > This is a bit confusing... Could you please add

[PATCH] D46001: [CodeComplete] Expose helpers to get RawComment of completion result.

2018-05-08 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. @arphaman, friendly ping. In case you're the wrong person to review it, I'll try to find someone else. Repository: rC Clang https://reviews.llvm.org/D46001 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D48211: [clangd] Do not show namespace comments.

2018-06-27 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 153028. ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment. - Move the namespace check to happen before canRequestComment call Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48211 Files: clangd/CodeComplet

[PATCH] D48211: [clangd] Do not show namespace comments.

2018-06-27 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments. Comment at: clangd/CodeCompletionStrings.cpp:172 // get this declaration, so we don't show documentation in that case. if (Result.Kind != CodeCompletionResult::RK_Declaration) return ""; sammccall wrote: > while her

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

2018-06-27 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. Sorry for the late response, was on vacation. Have you considered doing the same filtering in ASTUnit's `StoredDiagnosticConsumer`? It should not be more difficult and allows to avoid changing the clang's diagnostic interfaces. That's what we do in clangd. I wond

[PATCH] D48687: [clangd] Avoid duplicates in findDefinitions response

2018-06-28 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. Thanks for the patch! Could we try to figure out why the duplicates were there in the first place and why the paths were different? It should be easy to mock exactly the same setup you have in #37963, i.e. create a vfs with three files and compilation database tha

[PATCH] D48687: [clangd] Avoid duplicates in findDefinitions response

2018-06-28 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. Also some heads-up: this would probably conflict https://reviews.llvm.org/D47846 that moves the same function into a different file. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48687 ___ cfe-commi

[PATCH] D48634: [clangd] Improve output of --help and --version. NFC.

2018-06-28 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48634 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

<    16   17   18   19   20   21   22   23   24   25   >