labath added a comment. Thanks. The code looks fine to me. Losing the ability to unit test is a bit of a pity, but since test was pretty icky to begin with, I can't say I'm going to miss it too much... One way we could possibly test this is with a full scale test via the "target modules dump ast" command. So, the idea would be to run to process to some point, evaluate some expressions, run "target modules dump ast" and check the things are in the output (or that they are *not* there).
It's hard to say what exactly you should be checking for, since the assumption is that we are not changing the behavior here, and so the existing tests should cover that in theory, but the "dump ast" command is a new thing, we don't have too many of tests for it, and so any tests you add will be useful, even if they're not specifically targeting the thing you fix in this patch. If you are able to test the thing that the previous patch fixed in this way, than that would be great though. I'm not 100% sure it can be done, but since (IIUC) the bug/problem was about parsing too many things, I would expect that would manifest itself in some additional entries showing up in the parsed ast. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67022/new/ https://reviews.llvm.org/D67022 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
