Using precompiled header causes compilation error of tagged files

[DoDoENT]

Problem is that precompiled header is built only for non-tagged source files and all files attempt to use it. So when a specific file is tagged with arm or neon tag, then this file cannot use PCH and compile warning is issued. When all warnings are treated as errors, this causes compilation failure.

This problem was also present in r10e.

[DoDoENT]

Here is my patch which workarounds the issue by disabling usage of PCH for all files that have tags:

diff  orig/build-binary.mk build-binary.mk 
443,444c443,454
<     # All obj files are dependent on the PCH
<     $(foreach src,$(filter $(all_cpp_patterns),$(LOCAL_SRC_FILES)),\
---
>      # Filter obj files that are dependent on the PCH (only those without tags)
>     ifeq (true,$(LOCAL_ARM_NEON))
>         TAGS_TO_FILTER=arm
>     else
>         TAGS_TO_FILTER=arm neon
>     endif
> 
>     allowed_src := $(foreach src,$(filter $(all_cpp_patterns),$(LOCAL_SRC_FILES)),\
>         $(if $(filter $(TAGS_TO_FILTER),$(LOCAL_SRC_FILES_TAGS.$(src))),,$(src))\
>     )
>     # All files without tags depend on PCH
>     $(foreach src,$(allowed_src),\
447,449c457,458
< 
<     # Files from now on build with PCH
<     LOCAL_CPPFLAGS += -Winvalid-pch -include $(LOCAL_OBJS_DIR)/$(LOCAL_BUILT_PCH)
---
>     # Make sure those files are built with PCH
>     $(call add-src-files-target-cflags,$(allowed_src),-Winvalid-pch -include $(LOCAL_OBJS_DIR)/$(LOCAL_BUILT_PCH))

[DanAlbert]

Thanks for the patch. I added a test case and uploaded for review:https://android-review.googlesource.com/207977

[DoDoENT]

@DanAlbert, thank you for accepting the patch. I would only like to note that this patch works by disabling PCH for tagged files (i.e. files tagged with arm or neon will not use PCH). While this works fine for our use case when such files are minority, the real fix would actually be to create a separate PCH for each tag combination in use (I believe this is what Xcode does when compiling iOS projects).

[DanAlbert]

Yeah, I had considered that. There are bigger problems to chase right now, so I figure something is better than nothing :)

One thing that can be done to avoid this is to use a separate (intermediate) static library for the sources that would normally be tagged. This way you can set LOCAL_ARM_MODE and LOCAL_ARM_NEON correctly for each static lib and the LOCAL_PCH will automatically be built with the right flags for each lib. Then you just use them via LOCAL_WHOLE_STATIC_LIBRARIES to ensure you get the full library included.

[DoDoENT]

Thanks @DanAlbert, that is a good idea.