vgvassilev wrote: > > > What do you think about this approach? > > > > > > Makes sense to me. Could be harder than expected but let's try it out. > > @vgvassilev Glad to hear it makes sense! It might be a bit more nuanced than > it looks, but I've pushed a commit with the necessary changes—could you take > a look?
This looks very good! I've left a comment and we are almost good to go. One thing we can consider is try running all of the clang tests in incremental mode: the ones which have no diagnostics should have only declarations and no statements. That setup should help us find out where the statement disambiguation falls short. However, that's outside of the scope of this PR. > > Regarding a separate optimization: I think we could simplify > `isCXXDeclarationStatement` by reusing the logic in `isCXXSimpleDeclaration`. > Specifically, we could remove the top-level switch tand reuse the > TPResult::Error case in `isCXXSimpleDeclaration`: > > ```c++ > // In case of an error, let the declaration parsing code handle it. > if (TPR == TPResult::Error) > return true; > ``` > > In this scenario, if we encounter an expression like if or for, > `isCXXSimpleDeclaration` would treat it as an error and default to treating > it as a declaration. However, when `isCXXDeclarationStatement` is called with > `DisambiguatingWithExpression=true`, we could instead treat it as an > expression. > > While this might be slightly outside the scope of the current issue, I think > it’s worth considering for a cleaner implementation. What do you think, or we > can open another PR? That makes sense. However, if we do a more invasive refactoring probably we will have to do what I proposed in the previous paragraph. https://github.com/llvm/llvm-project/pull/178842 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
