kadircet added a comment. Sorry for leaving this without any replies, I think the summary you have is already of our offline discussion. Let me raise my final concerns, if you think we're covered for those and others don't chime in this week I suppose we can consider this as good to go.
--- I think 2nd case is fine, as we're unlikely to regress anything by handling just LHS. The rare case just won't get the benefits. --- I am more worried about creating "incorrect" nodes in some cases. I think it isn't valid in C/C++ for both LHS && RHS to be pointers/arrays in a subscript expression, but I've got no idea about when it's diagnosed in clang and what is put into the AST. If that detection happens before creating any SubscriptExprs (i.e. hitting changes in this patch), I guess we're all fine (i.e. we won't generate a node with an incorrect `ResultType`). If it happens after creating these exprs though, now we'll have a bunch of expressions with erroneous `ResultType` info which might trip over some things. (Worst case scenario, detection completely fails and some code that should've been diagnosed as being broken will miscompile instead.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107275/new/ https://reviews.llvm.org/D107275 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
