hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land.
The change looks good to me. > as discussed offline i don't see much value in having an extra flag to choose > between ast-based and pseudo-based implementation, as the pseudo-based one is > a super-set of the ast-based implementation. hence it shouldn't be regressing > change in any way, therefore i am in favor of just using the pseudo based > implementation inside clangdserver at all times (this is already hidden > behind a flag ATM). +1 > independent of that, branch cut is next tuesday. so i'd rather not land it > until the cut happens (existing implementation is also crashing, but it's the > devil we know and it would be nice to not introduce new failures). What the main concern here? This feature is hidden under a flag which is off by default (and I don't think any user would turn it because the syntax-tree implementation is crashy), it should not affect any users, so it seems fine to land it even before the branch cut. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130011/new/ https://reviews.llvm.org/D130011 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
