Ignore generated @import statements in the expression evaluator

[teemperor]

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)

[aprantl]

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?

[aprantl]

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]

• 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]

• Rebased and some minor refactoring to prevent having multiple places with the duplicated wrapper file name.

lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

 
 //===----------------------------------------------------------------------===//
 // Utility Methods for Clang
 //===----------------------------------------------------------------------===//
 
 class ClangExpressionParser::LLDBPreprocessorCallbacks : public PPCallbacks {
   ClangModulesDeclVendor &m_decl_vendor;
   ClangPersistentVariables &m_persistent_vars;
+  clang::SourceManager &m_source_mgr;
   StreamString m_error_stream;
   bool m_has_errors = false;
 
 public:
   LLDBPreprocessorCallbacks(ClangModulesDeclVendor &decl_vendor,
-                            ClangPersistentVariables &persistent_vars)
-      : m_decl_vendor(decl_vendor), m_persistent_vars(persistent_vars) {}
+                            ClangPersistentVariables &persistent_vars,
+                            clang::SourceManager &source_mgr)
+      : m_decl_vendor(decl_vendor), m_persistent_vars(persistent_vars),
+        m_source_mgr(source_mgr) {}
 
   void moduleImport(SourceLocation import_location, clang::ModuleIdPath path,
                     const clang::Module * /*null*/) override {
+    // Ignore modules that are imported in the wrapper code as these are not
+    // loaded by the user.
+    llvm::StringRef filename =
+        m_source_mgr.getPresumedLoc(import_location).getFilename();
+    if (filename == ClangExpressionSourceCode::g_prefix_file_name)
+      return;
+
     SourceModule module;
 
     for (const std::pair<IdentifierInfo *, SourceLocation> &component : path)
       module.path.push_back(ConstString(component.first->getName()));
 
     StreamString error_stream;
 
     ClangModulesDeclVendor::ModuleVector exported_modules;
ClangExpressionParser::ClangExpressionParser(ExecutionContextScope *exe_scope, Expression &expr,
   m_compiler->createPreprocessor(TU_Complete);
 
   if (ClangModulesDeclVendor *decl_vendor =
           target_sp->GetClangModulesDeclVendor()) {
     ClangPersistentVariables *clang_persistent_vars =
         llvm::cast<ClangPersistentVariables>(
             target_sp->GetPersistentExpressionStateForLanguage(
                 lldb::eLanguageTypeC));
-    std::unique_ptr<PPCallbacks> pp_callbacks(
-        new LLDBPreprocessorCallbacks(*decl_vendor, *clang_persistent_vars));
+    std::unique_ptr<PPCallbacks> pp_callbacks(new LLDBPreprocessorCallbacks(
+        *decl_vendor, *clang_persistent_vars, m_compiler->getSourceManager()));
     m_pp_callbacks =
         static_cast<LLDBPreprocessorCallbacks *>(pp_callbacks.get());
     m_compiler->getPreprocessor().addPPCallbacks(std::move(pp_callbacks));
   }
 
   // 8. Most of this we get from the CompilerInstance, but we also want to give
   // the context an ExternalASTSource.
 

lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h

 #include <string>
 
 namespace lldb_private {
 
 class ExecutionContext;
 
 class ClangExpressionSourceCode : public ExpressionSourceCode {
 public:
+  /// The file name we use for the wrapper code that we inject before
+  /// the user expression.
+  static const llvm::StringRef g_prefix_file_name;
   static const char *g_expression_prefix;
 
   static ClangExpressionSourceCode *CreateWrapped(llvm::StringRef filename,
                                                   llvm::StringRef prefix,
                                                   llvm::StringRef body) {
     return new ClangExpressionSourceCode(filename, "$__lldb_expr", prefix, body,
                                          Wrap);
   }

lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp

 #include "lldb/Target/Language.h"
 #include "lldb/Target/Platform.h"
 #include "lldb/Target/StackFrame.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/StreamString.h"
 
 using namespace lldb_private;
 
+#define PREFIX_NAME "<lldb wrapper prefix>"
+
+const llvm::StringRef ClangExpressionSourceCode::g_prefix_file_name = PREFIX_NAME;
+
 const char *ClangExpressionSourceCode::g_expression_prefix =
-    R"(
-#line 1 "<lldb wrapper prefix>"
+"#line 1 \"" PREFIX_NAME R"("
 #ifndef offsetof
 #define offsetof(t, d) __builtin_offsetof(t, d)
 #endif
 #ifndef NULL
 #define NULL (__null)
 #endif
 #ifndef Nil
 #define Nil (__null)