Use test-specific module caches to avoid stale header conflicts

[vsk]

Summary

Stale global module caches cause problems for the bots. The modules
become invalid when clang headers are updated by version control, and
tests which use these modules fail to compile, e.g:

fatal error: file '.../__stddef_max_align_t.h' has been modified since the module file '/var/.../Darwin.pcm' was built
note: please rebuild precompiled header '/var/.../Darwin.pcm'

Eventually we should transition to having just a single module cache to speed
tests up. This patch should be just enough to fix the spurious bot failures due
to stale caches.

rdar://36479805

[aprantl]

Thanks, this is a great start!

packages/Python/lldbsuite/test/make/Makefile.rules

240

Is it safe that this is a relative path?

242

Should we also specify -fcxxmodules here? Or will that break some bots with older clangs?

[vsk]

• Upload a diff with context.

packages/Python/lldbsuite/test/make/Makefile.rules

240

I admit it's not great, but it's at least in keeping with the other targets we define in this Makefile (like a.out, the dsym, etc). As I'm writing this, I've realized I did not upload a diff with context, so this might not have been apparent!

242

I've tried to avoid changing the flags used by each Makefile when possible. The only exception is my removal of "$(subst -fmodules,, $(CFLAGS))" in CXXFLAGS, which I needed to do because the substitution would make us pass "-cache-path=foo".

We might consider adding in -fcxxmodules as a followup (maybe just in CXXFLAGS?).

[labath]

I don't know much about clang modules, but the change seems reasonable to me.

[vsk]

Per an offline comment by Adrian, include -gmodules in the mandatory set of module flags.

[vsk]

• Skip tests which fail when -fmodules is passed (https://bugs.llvm.org/show_bug.cgi?id=36048).

[aprantl]

Skip tests which fail when -fmodules is passed (https://bugs.llvm.org/show_bug.cgi?id=36048).

Wait.. this patch is not supposed to change the set of tests that get -fmodules passed to them. It should only add -fmodules-cache-path to tests that do pass -fmodules. Why is this necessary?

[vsk]

In D42277#985002, @aprantl wrote:

Skip tests which fail when -fmodules is passed (https://bugs.llvm.org/show_bug.cgi?id=36048).

Wait.. this patch is not supposed to change the set of tests that get -fmodules passed to them. It should only add -fmodules-cache-path to tests that do pass -fmodules. Why is this necessary?

It's not necessary, but we should make this change. It falls out of removing this FIXME:

# FIXME: C++ modules aren't supported on all platforms.
CXXFLAGS += $(subst -fmodules,, $(CFLAGS))

If we keep it around, we'd need an extra hack to strip out "-fmodules-cache-path=module-cache" as well. Without that we'd pass "-cache-path=module-cache", which clang would reject.

Another benefit of making this change is that it lets us know which tests are actually failing with modules enabled, instead of hiding them.

[aprantl]

Okay got it. I'll investigate the PR separately.

[vsk]

Is this OK to commit?

packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py

def setUp(self):
         # Call super's setUp().
         TestBase.setUp(self)
         # Find the line number to break at.
         self.line = line_number('main.cpp', '// Set break point at this line.')
 
     @expectedFailureAll(
         oslist=["windows"],
         bugnumber="llvm.org/pr24462: Data formatters have problems on Windows")
+    @skipIf(debug_info="gmodules",
+            bugnumber="https://bugs.llvm.org/show_bug.cgi?id=36048")
     def test_with_run_command(self):
         """Test that that file and class static variables display correctly."""
         self.build()
         self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
 
         lldbutil.run_break_set_by_file_and_line(
             self, "main.cpp", self.line, num_expected_locations=1, loc_exact=True)
 

packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-globals/TestDataFormatterGlobals.py

 """
 Test lldb data formatter subsystem.
 """
 
 from __future__ import print_function
 
 
 import os
 import time
 import lldb
 from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
 import lldbsuite.test.lldbutil as lldbutil
 
 
 class GlobalsDataFormatterTestCase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
     def setUp(self):
         # Call super's setUp().
         TestBase.setUp(self)
         # Find the line number to break at.
         self.line = line_number('main.cpp', '// Set break point at this line.')
 
+    @skipIf(debug_info="gmodules",
+            bugnumber="https://bugs.llvm.org/show_bug.cgi?id=36048")
     def test_with_run_command(self):
         """Test that that file and class static variables display correctly."""
         self.build()
         self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
 
         lldbutil.run_break_set_by_file_and_line(
             self, "main.cpp", self.line, num_expected_locations=1, loc_exact=True)
 

packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/list/TestDataFormatterLibcxxList.py

def setUp(self):
         self.line2 = line_number('main.cpp',
                                  '// Set second break point at this line.')
         self.line3 = line_number('main.cpp',
                                  '// Set third break point at this line.')
         self.line4 = line_number('main.cpp',
                                  '// Set fourth break point at this line.')
 
     @add_test_categories(["libc++"])
+    @skipIf(debug_info="gmodules",
+            bugnumber="https://bugs.llvm.org/show_bug.cgi?id=36048")
     def test_with_run_command(self):
         """Test that that file and class static variables display correctly."""
         self.build()
         self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
 
         lldbutil.run_break_set_by_file_and_line(
             self, "main.cpp", self.line, num_expected_locations=-1)
         lldbutil.run_break_set_by_file_and_line(

packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/vector/TestDataFormatterLibcxxVector.py

 from lldbsuite.test import lldbutil
 
 
 class LibcxxVectorDataFormatterTestCase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
     @add_test_categories(["libc++"])
+    @skipIf(debug_info="gmodules",
+            bugnumber="https://bugs.llvm.org/show_bug.cgi?id=36048")
     def test_with_run_command(self):
         """Test that that file and class static variables display correctly."""
         self.build()
         self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
 
         bkpt = self.target().FindBreakpointByID(
             lldbutil.run_break_set_by_source_regexp(
                 self, "break here"))

packages/Python/lldbsuite/test/lang/objc/modules-auto-import/Makefile

 LEVEL = ../../../make
 OBJC_SOURCES := main.m
 
-CFLAGS += -fmodules -gmodules -g
+CFLAGS += $(MANDATORY_MODULE_BUILD_CFLAGS)
 
 include $(LEVEL)/Makefile.rules

packages/Python/lldbsuite/test/lang/objc/modules-auto-import/TestModulesAutoImport.py

class ObjCModulesAutoImportTestCase(TestBase):
 
     def setUp(self):
         # Call super's setUp().
         TestBase.setUp(self)
         # Find the line number to break inside main().
         self.line = line_number('main.m', '// Set breakpoint 0 here.')
 
     @skipUnlessDarwin
-    @skipIf(macos_version=["<", "10.12"])
+    @skipIf(macos_version=["<", "10.12"], debug_info=no_match(["gmodules"]))
     def test_expr(self):
         self.build()
         exe = self.getBuildArtifact("a.out")
         self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
 
         # Break inside the foo function which takes a bar_ptr argument.
         lldbutil.run_break_set_by_file_and_line(
             self, "main.m", self.line, num_expected_locations=1, loc_exact=True)

packages/Python/lldbsuite/test/lang/objc/modules-incomplete/Makefile

 LEVEL = ../../../make
 
 OBJC_SOURCES := main.m myModule.m
 
 include $(LEVEL)/Makefile.rules
 
-CFLAGS += -fmodules -I$(PWD)
+CFLAGS += $(MANDATORY_MODULE_BUILD_CFLAGS) -I$(PWD)
 LDFLAGS += -framework Foundation

packages/Python/lldbsuite/test/lang/objc/modules-incomplete/TestIncompleteModules.py

class IncompleteModulesTestCase(TestBase):
     def setUp(self):
         # Call super's setUp().
         TestBase.setUp(self)
         # Find the line number to break inside main().
         self.line = line_number('main.m', '// Set breakpoint 0 here.')
 
     @skipUnlessDarwin
     @unittest2.expectedFailure("rdar://20416388")
-    @skipIf(macos_version=["<", "10.12"])
+    @skipIf(macos_version=["<", "10.12"], debug_info=no_match(["gmodules"]))
     def test_expr(self):
         self.build()
         exe = self.getBuildArtifact("a.out")
         self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
 
         # Break inside the foo function which takes a bar_ptr argument.
         lldbutil.run_break_set_by_file_and_line(
             self, "main.m", self.line, num_expected_locations=1, loc_exact=True)

packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/Makefile

 LEVEL = ../../../make
 
 C_SOURCES := myModule.c
 
 OBJC_SOURCES := main.m
 
 include $(LEVEL)/Makefile.rules
 
-CFLAGS += -fmodules -I$(PWD)
+CFLAGS += $(MANDATORY_MODULE_BUILD_CFLAGS) -I$(PWD)

packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/TestModulesInlineFunctions.py

class ModulesInlineFunctionsTestCase(TestBase):
 
     def setUp(self):
         # Call super's setUp().
         TestBase.setUp(self)
         # Find the line number to break inside main().
         self.line = line_number('main.m', '// Set breakpoint here.')
 
     @skipUnlessDarwin
-    @skipIf(macos_version=["<", "10.12"])
+    @skipIf(macos_version=["<", "10.12"], debug_info=no_match(["gmodules"]))
     def test_expr(self):
         self.build()
         exe = self.getBuildArtifact("a.out")
         self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
 
         # Break inside the foo function which takes a bar_ptr argument.
         lldbutil.run_break_set_by_file_and_line(
             self, "main.m", self.line, num_expected_locations=1, loc_exact=True)

packages/Python/lldbsuite/test/make/Makefile.rules

 else
   CFLAGS_NO_DEBUG = -O0 $(ARCHFLAG)$(ARCH) $(FRAMEWORK_INCLUDES) $(ARCH_CFLAGS) $(CFLAGS_EXTRAS)
 endif
 
 ifeq "$(MAKE_DWO)" "YES"
   CFLAGS += -gsplit-dwarf
 endif
 
+CLANG_MODULE_CACHE_DIR := module-cache
+
+MANDATORY_MODULE_BUILD_CFLAGS := -fmodules -gmodules -fmodules-cache-path=$(CLANG_MODULE_CACHE_DIR)
+
 ifeq "$(MAKE_GMODULES)" "YES"
-  CFLAGS += -fmodules -gmodules
+  CFLAGS += $(MANDATORY_MODULE_BUILD_CFLAGS)
 endif
 
-CXXFLAGS += -std=c++11
-# FIXME: C++ modules aren't supported on all platforms.
-CXXFLAGS += $(subst -fmodules,, $(CFLAGS))
+CXXFLAGS += -std=c++11 $(CFLAGS)
 LD = $(CC)
 LDFLAGS ?= $(CFLAGS)
 LDFLAGS += $(LD_EXTRAS) $(ARCH_LDFLAGS)
 ifeq (,$(filter $(OS), Windows_NT Android Darwin))
   ifneq (,$(filter YES,$(ENABLE_THREADS)))
     LDFLAGS += -pthread
   endif
 endif

 .SUFFIXES: .mm .o
 .mm.o:
   $(CXX) $(CXXFLAGS) -c $<
 
 .PHONY: clean
 dsym: $(DSYM)
 all:  $(EXE) $(DSYM)
 clean::
-  $(RM) $(OBJECTS) $(PREREQS) $(PREREQS:.d=.d.tmp) $(DWOS) $(ARCHIVE_NAME) $(ARCHIVE_OBJECTS)
+  $(RM) -rf $(OBJECTS) $(PREREQS) $(PREREQS:.d=.d.tmp) $(DWOS) $(ARCHIVE_NAME) $(ARCHIVE_OBJECTS) $(CLANG_MODULE_CACHE_DIR)
 ifneq "$(DYLIB_NAME)" ""
   $(RM) -r $(DYLIB_FILENAME).dSYM
   $(RM) $(DYLIB_OBJECTS) $(DYLIB_PREREQS) $(DYLIB_PREREQS:.d=.d.tmp) $(DYLIB_DWOS) $(DYLIB_FILENAME) $(DYLIB_FILENAME).debug
 endif
 ifneq "$(PCH_OUTPUT)" ""
   $(RM) $(PCH_OUTPUT)
 endif
 ifneq "$(DSYM)" ""