support reusing PCH if linktarget has additional reasonable defines

Where reasonable means they are from a list of defines known not
to affect the system headers, and so they are safe to differ from
how the PCH was built. A bit hackish, but works in practice.

Change-Id: Ia00d2e4c56212aca05ba9d47abbb0d253998219f
Reviewed-on: https://gerrit.libreoffice.org/79364
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lunak@collabora.com>

8 files changed, 137 insertions, 22 deletions

diff --git a/pch/inc/clangfix.hxx b/pch/inc/clangfix.hxx
new file mode 100644
index 000000000000..9d1c131081b8
--- /dev/null
+++ b/pch/inc/clangfix.hxx
@@ -0,0 +1,29 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#ifdef __clang__
+// This is a bit lame, but when using -DMACRO that wasn't present when the PCH was built,
+// Clang suddenly subjects it to the -Wunused-macros check, which isn't the case otherwise.
+// And many of these macros are not actually used. So use them here to silence the warnings.
+// See gb_PrecompiledHeader_ignore_flags_system in solenv/gbuild/PrecompiledHeaders.mk .
+
+#ifdef SAX_DLLIMPLEMENTATION
+#endif
+#ifdef FASTSAX_DLLIMPLEMENTATION
+#endif
+#ifdef SYSTEM_EXPAT
+#endif
+#ifdef SYSTEM_LIBXML
+#endif
+#ifdef SYSTEM_ZLIB
+#endif
+
+#endif // __clang__
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sax/Library_expwrap.mk b/sax/Library_expwrap.mk
index 17a91e4d5904..f4d811886ca0 100644
--- a/sax/Library_expwrap.mk
+++ b/sax/Library_expwrap.mk
@@ -16,6 +16,8 @@ $(eval $(call gb_Library_set_include,expwrap,\
 	$$(INCLUDE) \
 ))
 
+$(eval $(call gb_Library_reuse_precompiled_header,expwrap,pch/inc/pch/precompiled_system))
+
 $(eval $(call gb_Library_add_defs,expwrap,\
 	-DFASTSAX_DLLIMPLEMENTATION \
 ))
diff --git a/sax/Library_sax.mk b/sax/Library_sax.mk
index 6ef11645f6f3..7fff54b7eed7 100644
--- a/sax/Library_sax.mk
+++ b/sax/Library_sax.mk
@@ -16,6 +16,8 @@ $(eval $(call gb_Library_set_include,sax,\
 
 $(eval $(call gb_Library_use_external,sax,boost_headers))
 
+$(eval $(call gb_Library_reuse_precompiled_header,sax,pch/inc/pch/precompiled_system))
+
 $(eval $(call gb_Library_use_sdk_api,sax))
 
 $(eval $(call gb_Library_use_libraries,sax,\
diff --git a/solenv/gbuild/LinkTarget.mk b/solenv/gbuild/LinkTarget.mk
index 4fc16df1f3a8..a6deccdad131 100644
--- a/solenv/gbuild/LinkTarget.mk
+++ b/solenv/gbuild/LinkTarget.mk
@@ -574,6 +574,7 @@ $(WORKDIR)/Clean/LinkTarget/% :
 		$(call gb_LinkTarget_get_headers_target,$(LINKTARGET)) \
 		$(call gb_LinkTarget_get_objects_list,$(LINKTARGET)) \
 		$(call gb_LinkTarget_get_pch_timestamp,$(LINKTARGETMAKEFILENAME)) \
+		$(call gb_LinkTarget_get_pch_reuse_timestamp,$(LINKTARGETMAKEFILENAME)) \
 		$(ILIBTARGET) \
 		$(AUXTARGETS)) && \
 		cat $${RESPONSEFILE} /dev/null | $(if $(filter WNT,$(OS)),env -i PATH="$$PATH") xargs -n 200 rm -fr && \
@@ -1563,7 +1564,7 @@ endef
 define gb_LinkTarget_set_precompiled_header
 ifneq ($(gb_ENABLE_PCH),)
 $(call gb_LinkTarget__set_precompiled_header_impl,$(1),$(2),$(notdir $(2)),$(4))
-$(call gb_PrecompiledHeader_generate_rules,$(notdir $(2)),$(4))
+$(call gb_PrecompiledHeader_generate_rules,$(notdir $(2)),$(1),$(4))
 endif
 
 endef
@@ -1575,9 +1576,13 @@ $(call gb_LinkTarget__set_precompiled_header_variables,$(1),$(2),$(3),$(4))
 
 $(call gb_LinkTarget_get_pch_timestamp,$(4)) : $(call gb_LinkTarget_get_pch_reuse_timestamp,$(4))
 
-$(call gb_LinkTarget_get_pch_reuse_timestamp,$(4)) : $(call gb_PrecompiledHeader_get_target,$(3),$(4))
+# We need to depend on a special for_reuse target that depends on the linktarget that owns the PCH.
+# Depending directly on the PCH could cause that PCH to be built with this linktarget's flags.
+$(call gb_LinkTarget_get_pch_reuse_timestamp,$(4)) : $(call gb_PrecompiledHeader_get_for_reuse_target,$(3),$(4))
 	$(call gb_PrecompiledHeader_check_flags,$(4),$(2),\
 		$(call gb_PrecompiledHeader_get_target,$(3),$(4)),$$(PCH_CXXFLAGS) $$(PCH_DEFS) $$(gb_LinkTarget_EXCEPTIONFLAGS))
+	$$(call gb_PrecompiledHeader__copy_reuse_files,$(1),$(3),$(4))
+	mkdir -p $$(dir $$@) && touch $$@
 
 endef
 
@@ -1585,6 +1590,9 @@ endef
 define gb_LinkTarget_reuse_precompiled_header
 ifneq ($(gb_ENABLE_PCH),)
 $(call gb_LinkTarget__reuse_precompiled_header_impl,$(1),$(2),$(notdir $(2)),$(4))
+ifeq ($(COM_IS_CLANG),TRUE)
+$(call gb_LinkTarget_add_defs,$(1),-include $(SRCDIR)/pch/inc/clangfix.hxx)
+endif
 endif
 
 endef
diff --git a/solenv/gbuild/PrecompiledHeaders.mk b/solenv/gbuild/PrecompiledHeaders.mk
index 8854784c5a41..d04059296cad 100644
--- a/solenv/gbuild/PrecompiledHeaders.mk
+++ b/solenv/gbuild/PrecompiledHeaders.mk
@@ -29,39 +29,46 @@ gb_PrecompiledHeader__get_debugdir = $(if $(call gb_LinkTarget__symbols_enabled,
 # for $(1)'s and things that are constant.
 # The defines are needed to get the right version of gb_PrecompiledHeader__get_debugdir.
 
-# $(call gb_PrecompiledHeader_generate_rules,pchtarget,linktargetmakefilename)
+# $(call gb_PrecompiledHeader_generate_rules,pchtarget,linktarget,linktargetmakefilename)
 define gb_PrecompiledHeader_generate_rules
 
-$(call gb_PrecompiledHeader_get_dep_target,$(1),$(2)) :
+$(call gb_PrecompiledHeader_get_dep_target,$(1),$(3)) :
 	$$(call gb_Helper_abbreviate_dirs,\
 		mkdir -p $$(dir $$@) && \
-		echo "$$(call gb_PrecompiledHeader_get_target,$(1),$(2)) : $$(gb_Helper_PHONY)" > $$@)
+		echo "$$(call gb_PrecompiledHeader_get_target,$(1),$(3)) : $$(gb_Helper_PHONY)" > $$@)
 
 # despite this being only one .d file, need to run concat-deps on it to
 # re-write external headers from UnpackedTarball
-$(call gb_PrecompiledHeader_get_target,$(1),$(2)) :
+$(call gb_PrecompiledHeader_get_target,$(1),$(3)) :
+	test "$$(PCH_LINKTARGETMAKEFILENAME)" = "$(3)" \
+		 || ( echo "Error, PCH $(1) built by $$(PCH_LINKTARGETMAKEFILENAME) instead of $(3)" >&2; exit 1)
 	rm -f $$@
-	$$(call gb_PrecompiledHeader__command,$$@,$(1),$$<,$$(PCH_DEFS),$$(PCH_CXXFLAGS) $$(gb_PrecompiledHeader_EXCEPTIONFLAGS),$$(INCLUDE),$(2))
-	$$(call gb_PrecompiledHeader__sum_command,$$@,$(1),$$<,$$(PCH_DEFS),$$(PCH_CXXFLAGS) $$(gb_PrecompiledHeader_EXCEPTIONFLAGS),$$(INCLUDE),$(2))
-	echo $$(sort $$(PCH_DEFS) $$(PCH_CXXFLAGS) $$(gb_PrecompiledHeader_EXCEPTIONFLAGS)) > $$(call gb_PrecompiledHeader_get_target,$(1),$(2)).flags
+	$$(call gb_PrecompiledHeader__command,$$@,$(1),$$<,$$(PCH_DEFS),$$(PCH_CXXFLAGS) $$(gb_PrecompiledHeader_EXCEPTIONFLAGS),$$(INCLUDE),$(3))
+	$$(call gb_PrecompiledHeader__sum_command,$$@,$(1),$$<,$$(PCH_DEFS),$$(PCH_CXXFLAGS) $$(gb_PrecompiledHeader_EXCEPTIONFLAGS),$$(INCLUDE),$(3))
+	echo $$(sort $$(PCH_DEFS) $$(PCH_CXXFLAGS) $$(gb_PrecompiledHeader_EXCEPTIONFLAGS)) > $$(call gb_PrecompiledHeader_get_target,$(1),$(3)).flags
 ifeq ($(gb_FULLDEPS),$(true))
 	$$(call gb_Helper_abbreviate_dirs,\
-		RESPONSEFILE=$$(call var2file,$$(shell $$(gb_MKTEMP)),200,$$(call gb_PrecompiledHeader_get_dep_target_tmp,$(1),$(2))) && \
+		RESPONSEFILE=$$(call var2file,$$(shell $$(gb_MKTEMP)),200,$$(call gb_PrecompiledHeader_get_dep_target_tmp,$(1),$(3))) && \
 		$$(call gb_Executable_get_command,concat-deps) $$$${RESPONSEFILE} \
-			> $$(call gb_PrecompiledHeader_get_dep_target,$(1),$(2)) && \
-		rm -f $$$${RESPONSEFILE} $$(call gb_PrecompiledHeader_get_dep_target_tmp,$(1),$(2)))
+			> $$(call gb_PrecompiledHeader_get_dep_target,$(1),$(3)) && \
+		rm -f $$$${RESPONSEFILE} $$(call gb_PrecompiledHeader_get_dep_target_tmp,$(1),$(3)))
 endif
 
+$(call gb_PrecompiledHeader_get_for_reuse_target,$(1),$(3)) : $(call gb_LinkTarget_get_target,$(2))
+	$$(call gb_PrecompiledHeader__create_reuse_files,$(2),$(1),$(3))
+	mkdir -p $$(dir $$@) && touch $$@
+
 .PHONY : $(call gb_PrecompiledHeader_get_clean_target,$(1))
 $(call gb_PrecompiledHeader_get_clean_target,$(1)) :
 	$$(call gb_Output_announce,$(1),$(false),PCH,1)
 	-$$(call gb_Helper_abbreviate_dirs,\
-		rm -f $$(call gb_PrecompiledHeader_get_target,$(1),$(2)) \
-			$$(call gb_PrecompiledHeader_get_target,$(1),$(2)).obj \
-			$$(call gb_PrecompiledHeader_get_target,$(1),$(2)).pdb \
-			$$(call gb_PrecompiledHeader_get_target,$(1),$(2)).sum \
-			$$(call gb_PrecompiledHeader_get_target,$(1),$(2)).flags \
-			$$(call gb_PrecompiledHeader_get_dep_target,$(1),$(2)))
+		rm -f $$(call gb_PrecompiledHeader_get_target,$(1),$(3)) \
+			$$(call gb_PrecompiledHeader_get_target,$(1),$(3)).obj \
+			$$(call gb_PrecompiledHeader_get_target,$(1),$(3)).pdb \
+			$$(call gb_PrecompiledHeader_get_target,$(1),$(3)).sum \
+			$$(call gb_PrecompiledHeader_get_target,$(1),$(3)).flags \
+			$$(call gb_PrecompiledHeader_get_target,$(1),$(3)).reuse \
+			$$(call gb_PrecompiledHeader_get_dep_target,$(1),$(3)))
 
 endef
 
@@ -74,22 +81,72 @@ $(call gb_LinkTarget_get_pch_timestamp,$(1)) :
 
 endef
 
-# $(call gb_PrecompiledHeader_check_saved_flags_command_pattern,linktargetmakefilename,pchcxxfile,pchfile,flags)
+# $(call gb_PrecompiledHeader_check_flags,linktargetmakefilename,pchcxxfile,pchfile,flags)
 # When creating a PCH, the PCH's CXXFLAGS are saved to a matching .flags file. When reusing the PCH
 # from another linktarget, use the file to check that the linktarget uses the same CXXFLAGS as the PCH.
 # This complements the check in gb_CxxObject__set_pchflags.
 define gb_PrecompiledHeader_check_flags
 $$(call gb_Helper_abbreviate_dirs,\
-	grep -q -x -F -- "$$(sort $(4))" $(3).flags || ( \
+	$$(if $$(strip $$(call gb_PrecompiledHeader_check_flags_internal,$$(shell cat $(3).flags),$(4),$(2))),false,true) || ( \
 		echo Error reusing $(2) by $(1). >&2 && \
 		echo -n " precompiled header flags : ">&2 && \
 		cat $(3).flags >&2 && \
 		echo    "            object flags  : "$$(sort $(4)) >&2 && \
+		echo    " reason : $$(call gb_PrecompiledHeader_check_flags_internal,$$(shell cat $(3).flags),$(4),$(2))" >&2 && \
 		echo Incorrect precompiled header setup or internal gbuild error. >&2 ; \
-		exit 1) && \
-	mkdir -p $$(dir $$@) && touch $$@ \
+		exit 1) \
 )
 
 endef
 
+# When trying to reuse one PCH between multiple linktargets, there is a problem that we have
+# various defines that cause mismatch in the check above, but these defines actually should not affect the PCH.
+# Specifically, there are 3 kinds:
+# - -DXXX_DLLIMPLEMENTATION - they are used only in our headers, should not affect system headers.
+# - -DSYSTEM_XXX - they are used only by our code (if at all), should not affect system headers
+# - various configuration defines - they again should only be used by our code and not system headers
+# Technically, different compilers handle additional defines like this:
+# - GCC
+#   * It is explicitly allowed to have different macros, as long as they do not affect the PCH.
+#   * With -Winvalid-pch GCC will even warn if there is a change in a macro affecting the PCH.
+#   * https://gcc.gnu.org/onlinedocs/gcc/Precompiled-Headers.html
+# - Clang
+#   * I could not find an official statement on what happens if definitions are different.
+#   * In practice a conflict does not seem to be detected, but the PCH and all the code in it
+#     acts according to the settings it was built with. Using a PCH and adding more defines
+#     seems to be functionally equivalent to creating the definitions only after the PCH inclusion.
+#   * As a side-effect, macros defined on the command line not present in the PCH suddenly
+#     trigger the -Wunused-macros warning. See bottom of pch/inc/clangfix.hxx .
+# - MSVC
+#   * MSVC explicitly states that the definitions must be the same, but they are not checked,
+#     and "unpredictable results can occur" if files depend on them.
+#   * In practice the situation seems to be the same as with Clang, the PCH and the code from it
+#     act according to the settings it was built with.
+#   * https://docs.microsoft.com/en-us/cpp/build/creating-precompiled-header-files
+# So while this is officially tricky, in practice it seems to work to allow PCH reuse if the linktarget
+# has more defines than the PCH was built with, as long as the defines do not affect the PCH.
+gb_PrecompiledHeader_ignore_flags_system := \
+-DSAX_DLLIMPLEMENTATION \
+-DFASTSAX_DLLIMPLEMENTATION \
+-DSYSTEM_EXPAT \
+-DSYSTEM_LIBXML \
+-DSYSTEM_ZLIB \
+-include $(SRCDIR)/pch/inc/clangfix.hxx \
+
+# Probably also update pch/inc/clangfix.hxx if you extend the list.
+
+# $(call gb_PrecompiledHeader_check_flags_internal,pchfileflags,flags,pchcxxfile)
+# Check if two sets of flags are compatible, allowing reuse of the PCH. Flags are compatible if
+# - they are the same
+# - the PCH is precompiled_system and the linktarget has additional defines listed above
+define gb_PrecompiledHeader_check_flags_internal
+$(if $(filter-out $(2),$(1)),$(filter-out $(2),$(1)), \
+	$(if $(filter-out $(1),$(2)),\
+		$(if $(filter-out precompiled_system,$(notdir $(3))),$(filter-out $(1),$(2)), \
+			$(foreach flag,$(filter-out $(1),$(2)),$(filter-out $(gb_PrecompiledHeader_ignore_flags_system),$(flag))) \
+		) \
+	,) \
+)
+endef
+
 # vim: set noet sw=4:
diff --git a/solenv/gbuild/TargetLocations.mk b/solenv/gbuild/TargetLocations.mk
index ddcb753cb12d..801d188be737 100644
--- a/solenv/gbuild/TargetLocations.mk
+++ b/solenv/gbuild/TargetLocations.mk
@@ -150,6 +150,7 @@ gb_PackageInfo_get_target = $(WORKDIR)/PackageInfo
 gb_Postprocess_get_target = $(WORKDIR)/Postprocess/$(1)
 gb_PrecompiledHeader_get_dep_target = $(WORKDIR)/Dep/PrecompiledHeader/$(call gb_PrecompiledHeader__get_debugdir,$(2))/$(1).hxx$(gb_PrecompiledHeader_EXT).d
 gb_PrecompiledHeader_get_dep_target_tmp = $(call gb_PrecompiledHeader_get_dep_target,$(1),$(2)).tmp
+gb_PrecompiledHeader_get_for_reuse_target = $(WORKDIR)/PrecompiledHeader/$(call gb_PrecompiledHeader__get_debugdir,$(2))/$(1).hxx$(gb_PrecompiledHeader_EXT).reuse
 gb_PrecompiledHeader_get_target = $(WORKDIR)/PrecompiledHeader/$(call gb_PrecompiledHeader__get_debugdir,$(2))/$(1).hxx$(gb_PrecompiledHeader_EXT)
 gb_PropertiesTranslateTarget_get_target = $(WORKDIR)/PropertiesTranslateTarget/$(1).properties
 gb_Pyuno_get_final_target = $(WORKDIR)/Pyuno/$(1).final
diff --git a/solenv/gbuild/platform/com_GCC_class.mk b/solenv/gbuild/platform/com_GCC_class.mk
index fd3703b875b5..25142f70625c 100644
--- a/solenv/gbuild/platform/com_GCC_class.mk
+++ b/solenv/gbuild/platform/com_GCC_class.mk
@@ -139,6 +139,9 @@ $(call gb_Helper_abbreviate_dirs,\
 endef
 endif
 
+# not needed
+gb_PrecompiledHeader__create_reuse_files =
+gb_PrecompiledHeader__copy_reuse_files =
 
 # YaccTarget class
 
diff --git a/solenv/gbuild/platform/com_MSC_class.mk b/solenv/gbuild/platform/com_MSC_class.mk
index afdc424b80c5..3f2630e0eec4 100644
--- a/solenv/gbuild/platform/com_MSC_class.mk
+++ b/solenv/gbuild/platform/com_MSC_class.mk
@@ -110,6 +110,19 @@ endef
 define gb_PrecompiledHeader__sum_command
 endef
 
+# When building a PCH, MSVC also creates a .pdb file with debug info. So for reuse
+# add the .pdb to the PCH's files and then use the .pdb also for linktargets that reuse the PCH.
+# call gb_PrecompiledHeader__create_reuse_files,linktarget,pchtarget,linktargetmakefilename
+define gb_PrecompiledHeader__create_reuse_files
+cp $(call gb_LinkTarget_get_pdbfile_in,$(1)) $(call gb_PrecompiledHeader_get_target,$(2),$(3)).pdb
+endef
+
+# call gb_PrecompiledHeader__copy_reuse_files,linktarget,pchtarget,linktargetmakefilename
+define gb_PrecompiledHeader__copy_reuse_files
+rm -f $(call gb_LinkTarget_get_pdbfile_in,$(1))
+cp $(call gb_PrecompiledHeader_get_target,$(2),$(3)).pdb $(call gb_LinkTarget_get_pdbfile_in,$(1))
+endef
+
 # AsmObject class
 
 gb_AsmObject_get_source = $(1)/$(2).asm