sammccall added a comment.

(This looks really cool, nice work! only a driveby comment from me I'm afraid)

In D148457#4304915 <https://reviews.llvm.org/D148457#4304915>, @zyounan wrote:

> The dot prior to AST node kind indicates partial selection and the asterisk 
> indicates complete selection according to SelectionTree::print 
> <https://searchfox.org/llvm/rev/be9c91843bab5bb46574c27836bfcd9ad6fc9ef5/clang-tools-extra/clangd/Selection.cpp#1009>.
>
> By invoking `Tree.commonAncestor()` it would stop at `BinaryOperator` despite 
> the fact that we're expecting `IntegerLiteral`, that represents the number 
> being accordance with the macro expansion.
>
> In order to address such case, I think we could go down the `SelectionTree` a 
> little further and start evaluating at the node with complete selection.

I think rather if the common ancestor isn't completely selected, evaluation 
shouldn't happen. (I think this is what Nathan was suggesting by "strict"?)

(Here, "+ 2" doesn't have a value, the + is binary rather than unary)

The partial/fully selected concept in SelectionTree was originally intended for 
exactly this kind of thing, so I'm happy to see it get some use :-)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148457/new/

https://reviews.llvm.org/D148457

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to