sammccall added a comment. In D64717#1585682 <https://reviews.llvm.org/D64717#1585682>, @SureYeaah wrote:
> In D64717#1585632 <https://reviews.llvm.org/D64717#1585632>, @sammccall wrote: > > > In D64717#1585542 <https://reviews.llvm.org/D64717#1585542>, @SureYeaah > > wrote: > > > > > In D64717#1585512 <https://reviews.llvm.org/D64717#1585512>, @sammccall > > > wrote: > > > > > > > Are you sure we want to disable extraction here, rather than just do > > > > the extraction at a higher level? > > > > > > > > E.g. if `bar(1,2,3, f[[o]]o(4,5));` seems like it should extract the > > > > call too `foo(4,5)`, not fail to trigger entirely. > > > > > > > > > Selecting `f[[o]]o(4,5)` will just extract the `foo` which would be a > > > `DeclRefExpr`. We want to extract the `CallExpr` for which we would need > > > to select the brackets as well. > > > > > > I agree, but this patch doesn't do that. (I think?) > > Instead it bans extraction entirely in this case. > > > Yes it doesn't. Our current model doesn't extend the user selection and as > such we only check the commonAncestor of whatever the user has selected. > Should this be changed then? Yes I think you want to walk up the tree like you do for finding the target stmt, giving up when you hit a non-expr. This didn't matter prior to this patch as void subexprs are extremely rare. But declrefs are not! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64717/new/ https://reviews.llvm.org/D64717 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
