[PATCH] D77507: [clangd] Fix HitMapping assertion in Tokens.cpp

2020-04-07 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. ayup, no problem and thanks for the fix. I'll abandon this change. Best! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77507/new/ https://reviews.llvm.org/D77507 ___ cfe-com

[PATCH] D77507: [clangd] Fix HitMapping assertion in Tokens.cpp

2020-04-06 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. In D77507#1964309 , @vabridgers wrote: > Thank you for the comments. I'll keep looking at this to find a proper fix, > but any hints you may have would be greatly appreciated (debugging tips, > strategies, areas of code to focu

[PATCH] D77507: [clangd] Fix HitMapping assertion in Tokens.cpp

2020-04-06 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. Thank you for the comments. I'll keep looking at this to find a proper fix, but any hints you may have would be greatly appreciated (debugging tips, strategies, areas of code to focus on). This code is new to me, so I may not be as efficient at tracking this down as

[PATCH] D77507: [clangd] Fix HitMapping assertion in Tokens.cpp

2020-04-06 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Oh dear, this seems like a design bug. TokenBuffer only attempts to record "top-level" expansions, i.e. `FOO(BAR(BAZ))` will record the expansion of FOO, and say the tokens `FOO ( BAR ( BAZ ) )` were expanded into `some other thing` in an essentially-opaque way. The p

[PATCH] D77507: [clangd] Fix HitMapping assertion in Tokens.cpp

2020-04-06 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Thanks for tracking this down. Unfortunately I don't think removing the assertion is enough, the code doesn't handle this case correctly. Comment at: clang/unittests/Tooling/Syntax/TokensTest.cpp:487 )"}, + // Baseline case, bugz: https://bugs.

[PATCH] D77507: [clangd] Fix HitMapping assertion in Tokens.cpp

2020-04-05 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 255193. vabridgers added a comment. Remove extraneous test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77507/new/ https://reviews.llvm.org/D77507 Files: clang/lib/Tooling/Syntax/Tokens.cpp clang/

[PATCH] D77507: [clangd] Fix HitMapping assertion in Tokens.cpp

2020-04-05 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers created this revision. vabridgers added reviewers: ilya-biryukov, sammccall. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, MaskRay. Herald added a project: clang. Extend test cases for tokens, and remove assertion that is unneeded and hitting in Tokens.