Revert "[lldb] Fix the way we set up the lldb modules infrastructure."

[bnbarham]

This reverts commit c95a0c9.

#65683 split -fincremental-extensions and IncrementalProcessing such that IncrementalProcessing can be used to extend the lifetime of various datastructures without parsing changes. LLDB really should not be parsing any differently to a regular compile, so switch back to using just IncrementalProcessing.

[llvmbot]

@llvm/pr-subscribers-lldb

Changes

This reverts commit c95a0c9.

#65683 split -fincremental-extensions and IncrementalProcessing such that IncrementalProcessing can be used to extend the lifetime of various datastructures without parsing changes. LLDB really should not be parsing any differently to a regular compile, so switch back to using just IncrementalProcessing.

Full diff: https://github.com/llvm/llvm-project/pull/66271.diff

2 Files Affected:

• (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp (+2-1)
• (modified) lldb/test/API/lang/objc/modules-compile-error/TestModulesCompileError.py (+1-1)

diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
index 5ce0d35378230cf..86d5ca1b125df3f 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -670,7 +670,6 @@ ClangModulesDeclVendor::Create(Target &target) {
       arch.GetTriple().str(),
       "-fmodules-validate-system-headers",
       "-Werror=non-modular-include-in-framework-module",
-      "-Xclang=-fincremental-extensions",
       "-Rmodule-build"};
 
   target.GetPlatform()->AddClangModuleCompilationOptions(
@@ -765,6 +764,8 @@ ClangModulesDeclVendor::Create(Target &target) {
                                instance->getFrontendOpts().Inputs[0]))
     return nullptr;
 
+  instance->getPreprocessor().enableIncrementalProcessing();
+
   instance->createASTReader();
 
   instance->createSema(action->getTranslationUnitKind(), nullptr);
diff --git a/lldb/test/API/lang/objc/modules-compile-error/TestModulesCompileError.py b/lldb/test/API/lang/objc/modules-compile-error/TestModulesCompileError.py
index 36e302be2525b51..db6d74bfdb540a5 100644
--- a/lldb/test/API/lang/objc/modules-compile-error/TestModulesCompileError.py
+++ b/lldb/test/API/lang/objc/modules-compile-error/TestModulesCompileError.py
@@ -21,7 +21,7 @@ def test(self):
             "expr @import LLDBTestModule",
             error=True,
             substrs=[
-                "module.h:4:1: error: use of undeclared identifier 'syntax_error_for_lldb_to_find'",
+                "module.h:4:1: error: unknown type name 'syntax_error_for_lldb_to_find'",
                 "syntax_error_for_lldb_to_find // comment that tests source printing",
                 "could not build module 'LLDBTestModule'",
             ],

[bnbarham]

Sorry about the noise here. I accidentally pushed over the same branch name.

[bnbarham]

Isn't that a better way to support the expression parsing logic where we execute statements on the global scope to evaluate their results?

This is currently enabled during module loading, where we definitely don't want any change in behavior IMO. Perhaps we could enable it after? If we think this is useful to keep I'll need to look into how #65683 broke LLDB testing:

lldb-api.commands/expression/diagnostics.TestExprDiagnostics.py
lldb-api.lang/objc/modules.TestObjCModules.py
lldb-api.lang/objc/modules-incomplete.TestIncompleteModules.py
lldb-api.lang/objc/modules-non-objc-target.TestObjCModulesNonObjCTarget.py
lldb-api.lang/objc/modules-objc-property.TestModulesObjCProperty.py

That change should effectively be NFC as far as -fincremental-extensions is concerned, so if you have any ideas that would be appreciated :)

[vgvassilev]

@bnbarham, I think it would be useful to keep it.

If we think this is useful to keep I'll need to look into how #65683 broke LLDB testing:

Incremental processing should also enable the llvm expression parser workflows. The advantage is that now we can write tests against clang-repl which combines the clang frontend and the llvm jit. If we capture the lldb workflows in these tests we will reduce the amount of regressions that inadvertently happen because of these two components are not tested together.

That change should effectively be NFC as far as -fincremental-extensions is concerned, so if you have any ideas that would be appreciated :)

If you can provide links to the failing tests I can take a look.

[bnbarham]

If you can provide links to the failing tests I can take a look.

https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/60022/

[vgvassilev]

Does that PR fix the tests you mentioned?

[bnbarham]

Sorry, which PR? #66281 is a revert of #65683 which caused the failures to begin with. The only non-NFC part of that PR is https://github.com/llvm/llvm-project/pull/65683/files#diff-5c51bae1966e0b475927bfa75751eb83db5d90f828aa825cb24f40dea653c408R714-R715. If I remove the if there, these tests start passing too.

IIRC not having that change caused failures elsewhere... Checking that now.

[bnbarham]

Everything seems to work fine without it, so I'll put up that PR again without that check and close this PR.