[PATCH] D61333: [ASTImporter] Fix LLDB lookup in transparent ctx and with ext src

Wed Jul 17 03:05:47 PDT 2019

martong marked an inline comment as done.
martong added inline comments.

================
Comment at: lldb/packages/Python/lldbsuite/test/lang/c/ast/TestAST.py:57
+        # This expr command imports __sFILE with definition
+        # (FILE is a typedef to __sFILE.)
+        self.expect(
----------------
teemperor wrote:
> I think this test has a good chance to fail once someone renamed/removes/changes this internal struct (especially if it's currently  abstracted with a typedef). Would it be possible to just make a minimal module with open and FILE/__sFILE instead? If it's too much trouble, then I'm also fine with merging this as-is (as this regression is easy to fix).
I'd rather keep this as-is, because I don't have enough confidence and experience with c modules (and with macOS).

================
Comment at: lldb/packages/Python/lldbsuite/test/lang/c/ast/TestAST.py:67
+        # Check that the AST log contains exactly one definition of __sFILE.
+        f = open(log_file)
+        log_lines = f.readlines()
----------------
teemperor wrote:
> It seems that this is the only different in the test compared to TestCModules.py. Would it be possible to just add this logging/checking to TestCModules.py as it's anyway testing something very similar?
Ok, I have removed the TestAST.py and added the extra logging and the check into TestCModules.py.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61333/new/

https://reviews.llvm.org/D61333