[Lldb-commits] [PATCH] D67022: Enhance SymbolFileDWARF::ParseDeclsForContext performance

[Guilherme Andrade via Phabricator via lldb-commits]

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