[PATCH] D59350: [clangd] Build Dex index after loading all shards in BackgroundIndex.

2019-03-14 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 190579. hokein marked an inline comment as done. hokein added a comment. Format the code. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59350/new/ https://reviews.llvm.org/D59350 Files: clangd/index/Background

[PATCH] D59350: [clangd] Build Dex index after loading all shards in BackgroundIndex.

2019-03-14 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL356126: [clangd] Build Dex index after loading all shards in BackgroundIndex. (authored by hokein, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Repository: rL L

[PATCH] D58345: [clangd] Using symbol name to map includes for STL symbols.

2019-03-14 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 190592. hokein marked 4 inline comments as done. hokein added a comment. Address comments Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58345/new/ https://reviews.llvm.org/D58345 Files: clangd/StdSymbolMap.inc

[PATCH] D58345: [clangd] Using symbol name to map includes for STL symbols.

2019-03-14 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments. Comment at: clangd/index/CanonicalIncludes.cpp:123 static const std::vector> SystemHeaderMap = { ioeric wrote: > ioeric wrote: > > hokein wrote: > > > sammccall wrote: > > > > hokein wrote: > > > > > ioeric wrote: > > > >

[PATCH] D58345: [clangd] Using symbol name to map includes for STL symbols.

2019-03-14 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 190598. hokein marked an inline comment as done. hokein added a comment. One more comment. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58345/new/ https://reviews.llvm.org/D58345 Files: clangd/StdSymbolMap.in

[PATCH] D58345: [clangd] Using symbol name to map includes for STL symbols.

2019-03-14 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE356134: [clangd] Using symbol name to map includes for STL symbols. (authored by hokein, committed by ). Changed prior to commit: https://reviews.llvm.org/D58345?vs=190598&id=190600#toc Repository:

[PATCH] D59364: [clangd] Add std subnamespace symbols to the symbol map.

2019-03-14 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision. hokein added a reviewer: ioeric. Herald added subscribers: jdoerfert, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a reviewer: serge-sans-paille. Herald added a project: clang. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D59364

[PATCH] D59714: [clang-tidy] Separate the check-facing interface

2019-03-25 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision. hokein 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/D59714/new/ https://reviews.llvm.org/D59714 ___

[PATCH] D59757: [clangd] Send empty diagnostics when a file is closed

2019-03-25 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments. Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1000 std::lock_guard Lock(FixItsMutex); FixItsMap[File] = LocalFixIts; } IIUC, it seems that we might have race condition here, considering: - open a file which w

[PATCH] D59757: [clangd] Send empty diagnostics when a file is closed

2019-03-25 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision. hokein added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1000 std::lock_guard Lock(FixItsMutex); FixItsMap[File] = LocalFixIts; } ilya-biryu

[PATCH] D59364: [clangd] Add std subnamespace symbols to the symbol map.

2019-03-25 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. friendly ping ;) Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59364/new/ https://reviews.llvm.org/D59364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.ll

[PATCH] D59764: [pp-trace] Use ClangTool in pp-trace, NFC

2019-03-25 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision. hokein added a reviewer: ioeric. Herald added a subscriber: jdoerfert. Herald added a project: clang. This patch partially reverts the commit rL356849 . Unfortunately, tooling::createExecutorFromCommandLineArgs doesn't corperate we

[PATCH] D59759: [clangd] Add .cu files to VSCode extension

2019-03-25 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. Do we need to add an entry `"onLanguage:cuda"` for cuda in the extension package.json? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59759/new/ https://reviews.llvm.org/D59759

[PATCH] D59764: [pp-trace] Use ClangTool in pp-trace, NFC

2019-03-25 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE356893: [pp-trace] Use ClangTool in pp-trace, NFC (authored by hokein, committed by ). Changed prior to commit: https://reviews.llvm.org/D59764?vs=192083&id=192089#toc Repository: rCTE Clang Tools

[PATCH] D59364: [clangd] Add std subnamespace symbols to the symbol map.

2019-03-25 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE356894: [clangd] Add std subnamespace symbols to the symbol map. (authored by hokein, committed by ). Changed prior to commit: https://reviews.llvm.org/D59364?vs=190618&id=192090#toc Repository: rC

[PATCH] D59759: [clangd] Add .cu files to VSCode extension

2019-03-25 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. In D59759#1441349 , @ilya-biryukov wrote: > In D59759#1441321 , @hokein wrote: > > > Do we need to add an entry `"onLanguage:cuda"` for cuda in the extension > > package.json? > > > Yeah, s

[PATCH] D59759: [clangd] Add .cu files to VSCode extension

2019-03-25 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision. hokein added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts:72 const filePattern: string = '**/*.{' + -['cpp', 'c', 'cc', 'cxx', 'c++', 'm',

[PATCH] D59627: [clang-format] Keep protobuf "package" statement on one line

2019-03-25 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. This patch has caused a regression issue, see the test: verifyFormat("// Detached comment\n\n" "// Leading comment\n" "syntax = \"proto2\"; // trailing comment\n\n" "// in foo.bar package\n" "pac

[PATCH] D59817: [clangd] Add activate command to the vscode extension.

2019-03-26 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision. hokein added a reviewer: ilya-biryukov. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric. Herald added a project: clang. This would help minizime the annoying part of not activating the extension for .cu file. Repository: rCTE Clang Tools Ext

[PATCH] D59817: [clangd] Add activate command to the vscode extension.

2019-03-27 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL357075: [clangd] Add activate command to the vscode extension. (authored by hokein, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES SI

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix descrption to clang-tidy checks.

2019-03-28 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision. Herald added subscribers: cfe-commits, jdoerfert, xazax.hun. Herald added a project: clang. Motivation/Context: in the code review system integrating withclang-tidy, clang-tidy doesn't provide a human-readable description of the fix. Usually developers have to preview

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

2019-03-28 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. This is my first attempt, still missing tests, but it should be in a shape to get early feedbacks. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59932/new/ https://reviews.llvm.org/D59932 _

[PATCH] D59935: Disable warnings when indexing as a standalone action.

2019-03-28 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments. Comment at: clangd/index/IndexAction.cpp:140 +// Avoids some analyses too. +CI.getDiagnosticOpts().IgnoreWarnings = true; return WrapperFrontendAction::BeginInvocation(CI); Can we add a unittest for this? IIRC, `IgnoreW

[PATCH] D59935: Disable warnings when indexing as a standalone action.

2019-03-28 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision. hokein added inline comments. This revision is now accepted and ready to land. Comment at: clangd/index/IndexAction.cpp:138 CI.getLangOpts().CommentOpts.ParseAllComments = true; +// Index the whole file even if there are warnings and -Werro

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

2019-04-01 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 193070. hokein marked 2 inline comments as done. hokein added a comment. Update the patch: - move FixDescriptions to tooling diagnostics, YAML serialization support - add tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

2019-04-01 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. In D59932#1446533 , @JonasToth wrote: > I think the idea is good and implementation, too. If we iterate all checks > anyway (probably?) we could think about adding a severity to the checks, too? > > I know that code-checker and cod

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

2019-04-02 Thread Haojian Wu via Phabricator via cfe-commits
hokein marked an inline comment as done. hokein added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.h:109 + /// not supported. + virtual llvm::StringRef fixDescription() { return ""; } + alexfh wrote: > Checks can provide differen

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

2019-04-03 Thread Haojian Wu via Phabricator via cfe-commits
hokein marked an inline comment as done. hokein added a comment. As discussed offline, the current approach only works for checks provide a single fix, providing such API is somehow misleading. Instead, we'd emit the check fix and the fix description via diagnostic::Note, rather than attaching

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

2019-04-03 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 193510. hokein added a comment. Emit the check fix description via diagnostic::note. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59932/new/ https://reviews.llvm.org/D59932 Files: clang-tools-extra/clang-app

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

2019-04-03 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. Running out of time today, the patch is not finished yet, but it should be good for another initial review/comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59932/new/ https://reviews.llvm.org/D59932 __

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

2019-04-05 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 193856. hokein marked 8 inline comments as done. hokein added a comment. Fix apply-replacements, and address comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59932/new/ https://reviews.llvm.org/D59932 Fi

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

2019-04-05 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. In D59932#1453565 , @alexfh wrote: > This looks like a more promising direction. Thanks for the readiness to > experiment with this. > > See the comments inline. Thanks for the comments. Now all existing tests are passed, the pat

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

2019-04-05 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 193857. hokein added a comment. Remove a stale comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59932/new/ https://reviews.llvm.org/D59932 Files: clang-tools-extra/clang-apply-replacements/lib/Tooling/A

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

2019-04-05 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments. Comment at: clang/include/clang/Tooling/Core/Diagnostic.h:67-71 + // Get the chosen fix to apply for this diagnostic. + // FIXME: currently we choose the first non-empty fix, extend it to support + // fix selection. + const llvm::StringMap *getCh

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

2019-04-05 Thread Haojian Wu via Phabricator via cfe-commits
hokein marked an inline comment as done. hokein added inline comments. Comment at: clang/include/clang/Tooling/Core/Diagnostic.h:67-71 + // Get the chosen fix to apply for this diagnostic. + // FIXME: currently we choose the first non-empty fix, extend it to support + // fix s

[PATCH] D60323: [clangd] Include compile command inference in fileStatus API

2019-04-08 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision. hokein added inline comments. This revision is now accepted and ready to land. Comment at: clangd/GlobalCompilationDatabase.cpp:70 /*Output=*/""); + Result.Heuristic = "default flags for unknown file"; + return Re

[PATCH] D60560: [clangd] Enable clang-tidy by default.

2019-04-11 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision. hokein added a reviewer: sammccall. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric, ilya-biryukov. Herald added a project: clang. We have turned on the flag internally for a while, and we don't receive complains. Should be good to turn it on

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

2019-04-12 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. Thanks, looks better now. I saw there are still a few undone comments, please mark them done when you address them. Comment at: docs/clang-tidy/checks/abseil-make-unique.rst:19 + +per the Abseil tips and guidelines at https://abseil.io/tips/126. ---

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

2019-04-12 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments. Comment at: clang/include/clang/Tooling/Core/Diagnostic.h:67-71 + // Get the chosen fix to apply for this diagnostic. + // FIXME: currently we choose the first non-empty fix, extend it to support + // fix selection. + const llvm::StringMap *getCh

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

2019-04-12 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 194817. hokein marked an inline comment as done. hokein added a comment. Update and rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59932/new/ https://reviews.llvm.org/D59932 Files: clang-tools-extra/cl

[PATCH] D60257: [Lookup] Invisible decls should not be ambiguous when renaming.

2019-04-12 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments. Comment at: unittests/Tooling/LookupTest.cpp:215 + + // Potentially ambiguous symbols that are not visible at reference location + // are not considered ambiguous. The test seems hard to understand what it actually does, could you

[PATCH] D60560: [clangd] Enable clang-tidy by default.

2019-04-12 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL358282: [clangd] Enable clang-tidy by default. (authored by hokein, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.o

[PATCH] D60776: [clang-tidy] Add test support for the fix description.

2019-04-16 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision. hokein added a reviewer: alexfh. Herald added a subscriber: xazax.hun. Herald added a project: clang. Add a new annotation "CHECK-FIX-DESCRIPTIONS" to lit test to verify the fix description provided by checks. Repository: rG LLVM Github Monorepo https://reviews.l

[PATCH] D60776: [clang-tidy] Add test support for the fix description.

2019-04-16 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 195385. hokein added a comment. cleanup Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60776/new/ https://reviews.llvm.org/D60776 Files: clang-tools-extra/test/clang-tidy/check_clang_tidy.py clang-tools-extr

[PATCH] D59932: [clang-tidy] Add fix descriptions to clang-tidy checks.

2019-04-17 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC358576: [clang-tidy] Add fix descriptions to clang-tidy checks. (authored by hokein, committed by ). Changed prior to commit: https://reviews.llvm.org/D59932?vs=194817&id=195550#toc Repository: rC Cl

[PATCH] D60821: [clangd] Emit better error messages when rename fails.

2019-04-17 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision. hokein added a reviewer: sammccall. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric, ilya-biryukov. Herald added a project: clang. Currently we emit an unfriendly "clang diagnostic" message when rename fails. This patch makes clangd to emit a

[PATCH] D60827: [rename] Deduplicate symbol occurrences

2019-04-17 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision. hokein added reviewers: arphaman, kadircet. Herald added subscribers: dexonsmith, jkorous. Herald added a project: clang. SymbolOccurrences is not guaranteed to be unique -- as some parts of the AST may get traversed twice, we may add duplicated occurrences, we should

[PATCH] D60857: [clang-tidy] Address post-commit comments

2019-04-18 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision. hokein added a reviewer: alexfh. Herald added a subscriber: xazax.hun. Herald added a project: clang. hokein updated this revision to Diff 195692. hokein added a comment. Cleanup. Also add a test to verify clang-tidy only apply the first alternative fix. Repositor

[PATCH] D60857: [clang-tidy] Address post-commit comments

2019-04-18 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 195692. hokein added a comment. Cleanup. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60857/new/ https://reviews.llvm.org/D60857 Files: clang-tools-extra/clang-tidy/ClangTidy.cpp clang-tools-extra/test/cla

[PATCH] D60821: [clangd] Emit better error messages when rename fails.

2019-04-18 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 195701. hokein marked 4 inline comments as done. hokein added a comment. Address comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60821/new/ https://reviews.llvm.org/D60821 Files: clang-tools-extra/cla

[PATCH] D60821: [clangd] Emit better error messages when rename fails.

2019-04-18 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:57 public: + RefactoringResultCollector(DiagnosticsEngine &DE) : Diags(DE) {} void handleError(llvm::Error Err) override { sammccall wrote: > why inject the DE here (and hand

[PATCH] D60821: [clangd] Emit better error messages when rename fails.

2019-04-18 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 195715. hokein marked an inline comment as done. hokein added a comment. Simplify the code further. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60821/new/ https://reviews.llvm.org/D60821 Files: clang-tools-

[PATCH] D60821: [clangd] Emit better error messages when rename fails.

2019-04-18 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE358658: [clangd] Emit better error messages when rename fails. (authored by hokein, committed by ). Changed prior to commit: https://reviews.llvm.org/D60821?vs=195715&id=195717#toc Repository: rCTE

[PATCH] D60857: [clang-tidy] Address post-commit comments

2019-04-18 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments. Comment at: clang-tools-extra/test/clang-tidy/alternative-fixes.cpp:1-7 +// RUN: clang-tidy -checks='-*,llvm-namespace-comment,clang-diagnostic-*' %s -- \ +// RUN: | FileCheck -implicit-check-not='{{warning|error|note}}:' %s + +// Verify clang-tidy

[PATCH] D60857: [clang-tidy] Address post-commit comments

2019-04-18 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 195720. hokein marked an inline comment as done. hokein added a comment. Use check_clang_tidy. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60857/new/ https://reviews.llvm.org/D60857 Files: clang-tools-extra

[PATCH] D60857: [clang-tidy] Address post-commit comments

2019-04-18 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 195733. hokein added a comment. fix doxygen comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60857/new/ https://reviews.llvm.org/D60857 Files: clang-tools-extra/clang-tidy/ClangTidy.cpp clang-tools-ex

[PATCH] D60857: [clang-tidy] Address post-commit comments

2019-04-18 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC358666: [clang-tidy] Address post-commit comments (authored by hokein, committed by ). Changed prior to commit: https://reviews.llvm.org/D60857?vs=195733&id=195734#toc Repository: rC Clang CHANGES S

[PATCH] D60827: [rename] Deduplicate symbol occurrences

2019-04-18 Thread Haojian Wu via Phabricator via cfe-commits
hokein marked an inline comment as done. hokein added inline comments. Comment at: clang-tools-extra/unittests/clangd/ClangdTests.cpp:1160 +TEST_F(ClangdVFSTest, NoDuplicatedTextEditsOnRename) { + MockFSProvider FS; kadircet wrote: > maybe put the test under c

[PATCH] D52710: [clangd] Add "check-clangd" target

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision. hokein added a reviewer: sammccall. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric, ilya-biryukov, mgorny. So we don't have to run "check-clang-tools" (which builds and tests all clang tools) to verify our clangd-related change. It'd save wait

[PATCH] D52710: [clangd] Add "check-clangd" target

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits
hokein marked an inline comment as done. hokein added inline comments. Comment at: test/CMakeLists.txt:36 set(CLANG_TOOLS_TEST_DEPS # For the clang-apply-replacements test that uses clang-rename. sammccall wrote: > can we add all of clangd to this list with

[PATCH] D52710: [clangd] Add "check-clangd" target

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 167689. hokein marked 2 inline comments as done. hokein added a comment. Remove clangd-binary dep in check-clang-tools, and use check-clangd. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52710 Files: test/CMakeLists.txt Index: test/CMa

[PATCH] D52710: [clangd] Add "check-clangd" target

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments. Comment at: test/CMakeLists.txt:76 -set(llvm_utils - FileCheck count not - ) - -foreach(t ${llvm_utils}) - if(TARGET ${t}) -list(APPEND CLANG_TOOLS_TEST_DEPS ${t}) - endif() -endforeach() - +macro(add_llvm_utils_deps deps) + set(llvm_utils

[PATCH] D52713: Move llvm util dependencies from clang-tools-extra to add_lit_target.

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision. hokein added a reviewer: sammccall. Herald added subscribers: llvm-commits, mgorny. Address fixme in r301762. And would simplify the cmake file in clang-tools-extra. Repository: rL LLVM https://reviews.llvm.org/D52713 Files: cmake/modules/AddLLVM.cmake Index

[PATCH] D52714: Build clang-headers when building clang tools.

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision. hokein added a reviewer: sammccall. Herald added subscribers: kadircet, ioeric, ilya-biryukov, mgorny. clang tools require clang headers to work on real project, e.g. when we build clangd via `ninja clangd`, we expect the binary can run on real-world project (without

[PATCH] D52710: [clangd] Add "check-clangd" target

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 167692. hokein added a comment. Remove llvm util dependencies (we move it to add_lit_target(). Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52710 Files: test/CMakeLists.txt Index: test/CMakeLists.txt ===

[PATCH] D52721: [Preprocessor] Fix a crash when handling non-alpha include header.

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision. hokein added a reviewer: sammccall. Herald added a subscriber: jsji. This also fixes a crash of code completion on `#include "./^"`. Repository: rC Clang https://reviews.llvm.org/D52721 Files: lib/Lex/PPDirectives.cpp test/Preprocessor/include-nonalpha-no-cr

[PATCH] D52714: Build clang-headers when building clang tools.

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL343459: Build clang-headers when building clang tools. (authored by hokein, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D52714 Files: cfe/tr

[PATCH] D52710: [clangd] Add "check-clangd" target

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. Need someone stamps on https://reviews.llvm.org/D52713 before landing this patch :) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52710 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.l

[PATCH] D52713: Move llvm util dependencies from clang-tools-extra to add_lit_target.

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. In https://reviews.llvm.org/D52713#1250947, @steveire wrote: > inlining this into three `add_dependencies` calls would remove the `if > (TARGET)` conditions. Are those dependencies conditionally built? Yeah, I believe these targets are built conditionally, see https://

[PATCH] D52721: [Preprocessor] Fix a crash when handling non-alpha include header.

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 167724. hokein marked an inline comment as done. hokein added a comment. Update the code. Repository: rC Clang https://reviews.llvm.org/D52721 Files: lib/Lex/PPDirectives.cpp test/Preprocessor/include-nonalpha-no-crash.c Index: test/Preprocessor/inc

[PATCH] D52721: [Preprocessor] Fix a crash when handling non-alpha include header.

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments. Comment at: lib/Lex/PPDirectives.cpp:1891 StringRef OriginalFilename = Filename; if (!File) { +while (!Filename.empty() && !isAlphanumeric(Filename.front())) { sammccall wrote: > everything in this block is guar

[PATCH] D52713: Move llvm util dependencies from clang-tools-extra to add_lit_target.

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL343473: Move llvm util dependencies from clang-tools-extra to add_lit_target. (authored by hokein, committed by ). Repository: rL LLVM https://reviews.llvm.org/D52713 Files: llvm/trunk/cmake/modules

[PATCH] D52710: [clangd] Add "check-clangd" target

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL343474: [clangd] Add "check-clangd" target (authored by hokein, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D52710 Files: clang-tools-extra/

[PATCH] D52721: [Preprocessor] Fix a crash when handling non-alpha include header.

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 167737. hokein marked 3 inline comments as done. hokein added a comment. Address review comments. Repository: rC Clang https://reviews.llvm.org/D52721 Files: lib/Lex/PPDirectives.cpp test/Preprocessor/include-nonalpha-no-crash.c Index: test/Preproce

[PATCH] D52721: [Preprocessor] Fix a crash when handling non-alpha include header.

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. I verified this patch is passed all clang tests (`ninja check-clang`). Repository: rC Clang https://reviews.llvm.org/D52721 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/list

[PATCH] D52721: [Preprocessor] Fix a crash when handling non-alpha include header.

2018-10-01 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC343481: [Preprocessor] Fix a crash when handling non-alpha include header. (authored by hokein, committed by ). Changed prior to commit: https://reviews.llvm.org/D52721?vs=167737&id=167738#toc Reposito

[PATCH] D52774: [Preprocessor] Fix an assertion failure trigged in clangd #include completion.

2018-10-02 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision. hokein added a reviewer: sammccall. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric, ilya-biryukov. - Fix an assertion, the Filename should fallback to the written name in the code if the correct typo heuristic fails. - Fix an inconsistent is

[PATCH] D52775: [clangd] Add a #include completion test that triggers an assertion.

2018-10-02 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision. hokein added a reviewer: sammccall. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric, ilya-biryukov. Test for https://reviews.llvm.org/D52774. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52775 Files: unittests/clangd/Cod

[PATCH] D52774: [Preprocessor] Fix an assertion failure trigged in clangd #include completion.

2018-10-02 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. For reference, the crash stack is like: ClangdTests: ../include/llvm/ADT/StringSet.h:39: std::pair llvm::StringSet::insert(llvm::StringRef) [AllocatorTy = llvm::MallocAllocator]: Assertion `!Key.empty()' failed. #0 0x0061e5ff llvm::sys::PrintStackTrace(llvm::

[PATCH] D52778: [Preprocessor] Hide include typo correction behind SpellChecking.

2018-10-02 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision. hokein added a reviewer: sammccall. Similar to Sema typo correction, the Preprocessor typo correction should also be hidden behind the SpellChecking flag. Repository: rC Clang https://reviews.llvm.org/D52778 Files: lib/Lex/PPDirectives.cpp Index: lib/Lex/PPD

[PATCH] D52774: [Preprocessor] Fix an assertion failure trigged in clangd #include completion.

2018-10-02 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 167935. hokein added a comment. Rescope the patch. Repository: rC Clang https://reviews.llvm.org/D52774 Files: lib/Lex/PPDirectives.cpp Index: lib/Lex/PPDirectives.cpp === --- lib/Lex/PPD

[PATCH] D52774: [PProcesssor] Filename should fall back to the written name when typo correction fails.

2018-10-02 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. In https://reviews.llvm.org/D52774#1252294, @kristina wrote: > Could you add a regression test please? Also could you run this through a > debugger and get a backtrace from a debugger instead of the crash report, it > seems to be missing a few things likely from optimiza

[PATCH] D52778: [Preprocessor] Hide include typo correction behind SpellChecking.

2018-10-02 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 167939. hokein added a comment. Fix review comment. Repository: rC Clang https://reviews.llvm.org/D52778 Files: lib/Lex/PPDirectives.cpp Index: lib/Lex/PPDirectives.cpp === --- lib/Lex/PP

[PATCH] D52778: [Preprocessor] Hide include typo correction behind SpellChecking.

2018-10-02 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. hokein marked an inline comment as done. Closed by commit rC343591: [Preprocessor] Hide include typo correction behind SpellChecking. (authored by hokein, committed by ). Changed prior to commit: https://reviews.llvm.org/

[PATCH] D52774: [PProcesssor] Filename should fall back to the written name when typo correction fails.

2018-10-02 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 167942. hokein retitled this revision from " [PProcesssor] Filename should fall back to the written name when typo correction fails. " to "[PProcesssor] Filename should fall back to the written name when typo correction fails.". hokein added a comment. Reba

[PATCH] D52781: [clangd] Don't make check-clangd as a dependency in check-clang-tools

2018-10-02 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision. hokein added a reviewer: sammccall. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric, ilya-biryukov, mgorny. check-clang-tools will run check-clangd first, and then run the rest tests. If clangd tests fails, check-clang-tools would be stopped.

[PATCH] D52774: [PProcesssor] Filename should fall back to the written name when typo correction fails.

2018-10-02 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC343592: [Preprocesssor] Filename should fall back to the written name when typo… (authored by hokein, committed by ). Changed prior to commit: https://reviews.llvm.org/D52774?vs=167942&id=167948#toc Re

[PATCH] D52775: [clangd] Add a #include completion test that triggers an assertion.

2018-10-02 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE343593: [clangd] Add a #include completion test that triggers an assertion. (authored by hokein, committed by ). Changed prior to commit: https://reviews.llvm.org/D52775?vs=167908&id=167949#toc Repos

[PATCH] D52781: [clangd] Don't make check-clangd as a dependency in check-clang-tools

2018-10-02 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL343608: [clangd] Don't make check-clangd as a dependency in check-clang-tools (authored by hokein, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/

[PATCH] D52726: [clangd] Support refs() in dex. Largely cloned from MemIndex.

2018-10-04 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments. Comment at: clangd/index/dex/Dex.h:61 } // Symbols are owned by BackingData, Index takes ownership. + template nit: this comment is stale too. Comment at: unittests/clangd/DexTests.cpp:618 +Files.push_b

[PATCH] D52531: [clangd] clangd-indexer gathers refs and stores them in index files.

2018-10-04 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. Looks great! I played around this patch a bit on LLVM. The output size is 50MB (binary)/1.2G (YAML). The ref output is not complete (missing all refs in headers, since we only collect refs from main file), I think we will add an option to `SymbolCollector` to allow coll

[PATCH] D52726: [clangd] Support refs() in dex. Largely cloned from MemIndex.

2018-10-04 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. LGTM Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52726 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.l

[PATCH] D52871: [clangd] Use canonical declarations in ReferenceFinder.

2018-10-04 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision. hokein added a reviewer: sammccall. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric, ilya-biryukov. handleDeclOccurrencce reports a canonical declartion, so stick to use canonical declarations to determine whether a declaration is in the target

[PATCH] D52531: [clangd] clangd-indexer gathers refs and stores them in index files.

2018-10-04 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. looks good. Comment at: clangd/index/Serialization.cpp:301 +// A refs section has data grouped by Symbol. Each symbol has: +// - SymbolID: 20 bytes +// - NumRefs: varint --

[PATCH] D52871: [clangd] Use canonical declarations in ReferenceFinder.

2018-10-04 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 168248. hokein marked an inline comment as done. hokein added a comment. Remove unnecessary labels. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52871 Files: clangd/XRefs.cpp unittests/clangd/XRefsTests.cpp Index: unittests/clangd/XRe

[PATCH] D52871: [clangd] Use canonical declarations in ReferenceFinder.

2018-10-04 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE343763: [clangd] Use canonical declarations in ReferenceFinder. (authored by hokein, committed by ). Changed prior to commit: https://reviews.llvm.org/D52871?vs=168248&id=168250#toc Repository: rL

[PATCH] D52871: [clangd] Use canonical declarations in ReferenceFinder.

2018-10-04 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL343763: [clangd] Use canonical declarations in ReferenceFinder. (authored by hokein, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D52871 Files:

[PATCH] D52877: [Index] Respect "IndexFunctionLocals" option during index.

2018-10-04 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision. hokein added a reviewer: sammccall. Herald added subscribers: kadircet, arphaman, ioeric, ilya-biryukov. Previously, clangd index ignored symbols defined in the function body even IndexFunctionLocals is true. Repository: rC Clang https://reviews.llvm.org/D52877

[PATCH] D52872: [clangd] Make binary index format the default, remove dead flag.

2018-10-04 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments. Comment at: clangd/indexer/IndexerMain.cpp:58 SymbolCollector::Options Opts; -Opts.FallbackDir = AssumedHeaderDir; return createStaticIndexingAction( If we remove the `assume-header-dir`, we probably remove `FallbackDir

[PATCH] D52877: [Index] Respect "IndexFunctionLocals" option for type loc.

2018-10-04 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC343767: [Index] Respect "IndexFunctionLocals" option for type loc. (authored by hokein, committed by ). Changed prior to commit: https://reviews.llvm.org/D52877?vs=168261&id=168263#toc Repository: rC

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