guiandrade added a comment.
In D67022#1659155 <https://reviews.llvm.org/D67022#1659155>, @labath wrote:
> 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.
I'm trying to play with that command to see if anything changes there, but I
haven't been able to do so yet. I'm using the following snippet.
// main.cpp
#include "Foo.h"
int main() { while (true) foo(); /*Breakpoint A*/ /*Breakpoint C*/ }
// Foo.h
#pragma once
void foo();
// Foo.cpp
#include "Foo.h"
namespace {
struct C { void f() {} };
struct C0 : public C {
int c00, c01, c02;
};
};
void foo() {
C0 c0; // Breakpoint B
c0.f();
}
Removing this patch and the other one, these are the GetClangDeclForDIE calls
we make https://pastebin.com/AcKGCBz7.
After applying this patch, we cut that down to https://pastebin.com/BjwmbmE0.
Nevertheless, in either case our AST ends up looking the same at breakpoint 'C'.
TranslationUnitDecl 0x55fe81dff238 <<invalid sloc>> <invalid sloc>
|-FunctionDecl 0x55fe81dffb28 <<invalid sloc>> <invalid sloc> main 'int ()'
extern
|-FunctionDecl 0x55fe81dffbf8 <<invalid sloc>> <invalid sloc> foo 'void ()'
extern
|-NamespaceDecl 0x55fe81dffc98 <<invalid sloc>> <invalid sloc>
| `-CXXRecordDecl 0x55fe81dffd20 <<invalid sloc>> <invalid sloc>
<undeserialized declarations> struct C0
`-UsingDirectiveDecl 0x55fe81dffec0 <<invalid sloc>> <invalid sloc> Namespace
0x55fe81dffc98 ''
I'm not sure if there's a way to have the AST reflect the extra calls (but I
also know very little about how that part of the code works). Can you guys
think of a piece of code that could do that?
Thanks!
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
