Re: [PATCH] D70512: [clangd] Rethink how SelectionTree deals with macros and #includes.

Right, should be fixed in 1374f7bd9f07115a7c69908bf50ba22e77b0e149 (Just like the last revert, libstdc++ is letting me get away with things that the others don't...) On Tue, Dec 3, 2019 at 9:53 PM Nico Weber via Phabricator < revi...@reviews.llvm.org> wrote: > thakis added a comment. > > Mac is

[PATCH] D70512: [clangd] Rethink how SelectionTree deals with macros and #includes.

thakis added a comment. Mac is happy now, but it fails to build on Windows: http://45.33.8.238/win/3327/step_4.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70512/new/ https://reviews.llvm.org/D70512

[PATCH] D70512: [clangd] Rethink how SelectionTree deals with macros and #includes.

thakis added a comment. That particular bot does GN builds, but these test failures repro in the cmake build for me on both macs I tried. (Remember that the official mac bots are on greendragon, not on buildbot -- I'd guess it shows there too.) Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D70512: [clangd] Rethink how SelectionTree deals with macros and #includes.

sammccall added a comment. Reverted as 905b002c139f039a32ab9bf1fad63d745d12423f @thakis any information available about that bot configuration? I'm not seeing any failures on "official" ones, I can't reproduce locally, and th

Re: [PATCH] D70512: [clangd] Rethink how SelectionTree deals with macros and #includes.

Thanks! It's not obvious what's going on, I can't reproduce it. I'm sure I'll end up reverting, but as I can't see it on any of the "official" bots either - do you have any description of the configuration? On Fri, Nov 29, 2019 at 7:06 PM Nico Weber via Phabricator < revi...@reviews.llvm.org> wrot

[PATCH] D70512: [clangd] Rethink how SelectionTree deals with macros and #includes.

thakis added a comment. hover.test is failing on Mac: http://45.33.8.238/mac/3308/step_7.txt Please take a look, and revert if it takes a while. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70512/new/ https://reviews.llvm.org/D70512 __

[PATCH] D70512: [clangd] Rethink how SelectionTree deals with macros and #includes.

This revision was automatically updated to reflect the committed changes. sammccall marked 2 inline comments as done. Closed by commit rG19daa21f841a: [clangd] Rethink how SelectionTree deals with macros and #includes. (authored by sammccall). Changed prior to commit: https://reviews.llvm.org/D

[PATCH] D70512: [clangd] Rethink how SelectionTree deals with macros and #includes.

hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. Thanks, the patch looks good! please also update the patch description. Comment at: clang-tools-extra/clangd/Selection.cpp:63 + // Claims whichever expanded tokens in Source

[PATCH] D70512: [clangd] Rethink how SelectionTree deals with macros and #includes.

merge_guards_bot added a comment. Build result: pass - 60302 tests passed, 0 failed and 732 were skipped. Log files: console-log.txt , CMakeCache.txt

[PATCH] D70512: [clangd] Rethink how SelectionTree deals with macros and #includes.

sammccall updated this revision to Diff 231206. sammccall added a comment. Rewrote patch with a better approach (claiming expanded tokens rather than spelled). Added more tests, including one showing a problem with multiple arg expansion. Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[PATCH] D70512: [clangd] Rethink how SelectionTree deals with macros and #includes.

sammccall planned changes to this revision. sammccall added a comment. No, this patch is busted, and the tests were too simple go catch it. The issue is that with no exclusivity check, given `{{{ MACRO }}}` all the enclosing blocks get to count themselves selected. We need an exclusivity check

[PATCH] D70512: [clangd] Rethink how SelectionTree deals with macros and #includes.

merge_guards_bot added a comment. Build result: fail - 60299 tests passed, 1 failed and 732 were skipped. failed: Clangd Unit Tests._/ClangdTests/SelectionTest.PathologicalPreprocessor Log files: console-log.txt

[PATCH] D70512: [clangd] Rethink how SelectionTree deals with macros and #includes.

sammccall updated this revision to Diff 230948. sammccall marked 10 inline comments as done. sammccall added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70512/new/ https://reviews.llvm.org/D70512 Files: clan

[PATCH] D70512: [clangd] Rethink how SelectionTree deals with macros and #includes.

hokein added inline comments. Comment at: clang-tools-extra/clangd/Selection.cpp:49 +Result = New; + else if (Result != New) +Result = SelectionTree::Partial; I might be missing something, I don't understand why we set Partial if `Result != New`.

[PATCH] D70512: [clangd] Rethink how SelectionTree deals with macros and #includes.

merge_guards_bot added a comment. Build result: pass - 60204 tests passed, 0 failed and 732 were skipped. Log files: console-log.txt , CMakeCache.txt

[PATCH] D70512: [clangd] Rethink how SelectionTree deals with macros and #includes.

sammccall added a comment. This fixes: - https://github.com/clangd/clangd/issues/126 - https://github.com/clangd/clangd/issues/202 (I've amended the commit message) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70512/new/ https://reviews.llvm.org

[PATCH] D70512: [clangd] Rethink how SelectionTree deals with macros and #includes.

sammccall created this revision. sammccall added a reviewer: hokein. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, MaskRay. Herald added a project: clang. The exclusive-claim model is successful at resolving conflicts over tokens between parent/child or siblings.