Page MenuHomePhabricator

Ignore generated @import statements in the expression evaluator
ClosedPublic

Authored by teemperor on May 5 2019, 4:55 AM.

Details

Summary

The ClangModulesDeclVendor is currently interpreting all injected @import statements in our expression
wrapper as modules that the user has explicitly requested to be persistently loaded. As we inject
@import statements with our std module prototype, the ClangModulesDeclVendor will start compiling
and loading unrelated C++ modules because it thinks the user has requested that it should load them. As
the ClangModulesDeclVendor is lacking the setup to compile these modules (e.g. it lacks the include paths),
it will then actually just fail to compile them and cause the whole expression evaluation to fail. This causes
these tests to fail on systems that enable the ClangModulesDeclVendor (such as macOS).

This patch fixes this by preventing the ClangModulesDeclVendor from interpreting @import statements
in the wrapper source code. This is done by check if the import happens in the fake source file containing
our wrapper code (which implies it was generated by LLDB).

This patch doesn't reenable the tests as there is more work needed to get the tests running on macOS (D67760)

Diff Detail

Repository
rL LLVM

Event Timeline

teemperor created this revision.May 5 2019, 4:55 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 5 2019, 4:55 AM

A slightly more elegant solution might be to inject a #line directive that changes to a different source file for the code that the user entered. I've been long wanting to make expr -g more palatable to end users by hiding the LLDB-injected code in a separate source file by default. If that turns out to be too much work, feel free to land this version.

lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
111 ↗(On Diff #198171)

///

141 ↗(On Diff #198171)

Just to make sure I'm understanding:

expr @import Foo will still work, but it will import Foo into the expression rather than into the context from which we ASTImport definitions?

To clarify:

I'd like expr -g to write out one temporary file (lldb-expr.mm) that only contains the code the user typed and in the expression. The expression should say

@implementation $__lldb_objc_class ($__lldb_category) 
+(void)%s:(void *)$__lldb_arg {
#line "/tmp/lldb-expr.mm" 1
  blah();
#line "/tmp/hidden.mm" 8
}

and then we set a breakpoint at /tmp/lldb-expr.mm:1.

teemperor updated this revision to Diff 213803.Aug 7 2019, 12:02 AM
teemperor retitled this revision from Ignore generated @import statements in the expression evaluator to fix import-std-module tests on macOS to Ignore generated @import statements in the expression evaluator.
teemperor edited the summary of this revision. (Show Details)
teemperor marked 3 inline comments as done.
  • Moved from macro to source file name filtering. No longer enable the tests with this patch as we still have the include path bug to fix.
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
141 ↗(On Diff #198171)

Yeah, we only filter our own generated imports that are non-permanent and only for one expression.

teemperor updated this revision to Diff 220987.Sep 20 2019, 2:45 AM
teemperor marked an inline comment as done.
teemperor edited the summary of this revision. (Show Details)
  • Rebased and some minor refactoring to prevent having multiple places with the duplicated wrapper file name.
This revision was not accepted when it landed; it landed in state Needs Review.Sep 23 2019, 11:57 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptSep 23 2019, 11:57 PM