Skip to content

Commit b55176e

Browse files
dkulpclaude
andcommitted
fix(build): keep the PCH-only flags in one variable so the opt-out can't drift
1f121a3 added -Winvalid-pch alongside -fpch-preprocess but left makefiles/fppinit.mk filtering only the literal -fpch-preprocess, so the four FPPINIT objects -- which deliberately opt out of the PCH -- went on carrying a PCH-only warning flag. It is inert today because none of those TUs include fpp-pch.h, so gcc never goes looking for a .gch, but it would start warning the moment one of them did, on exactly the objects that asked not to use the PCH. Two hand-maintained copies of the same list drifting apart is what broke the PCH to begin with, so name the list once: PCH_CFLAGS is defined where the flags are added and filtered by reference where they are removed. Verified from make -n on 32-bit armhf and aarch64: normal object -fpch-preprocess yes -Winvalid-pch yes -g1 yes FPPINIT object no no no fpp-pch.h.gch yes yes yes Full build otherwise unchanged: 177.7s, 204 objects, rc=0, and zero -Winvalid-pch warnings. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 386e7c6 commit b55176e

3 files changed

Lines changed: 13 additions & 3 deletions

File tree

src/Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ fppversion_defines.h: fppversion.c
9999
ifeq '$(findstring clang,$(CXXCOMPILER))' ''
100100
# Target-specific variables are inherited by a target's prerequisites, and
101101
# $(PCH_FILE) is a prerequisite of every %.o -- including the fppinit objects,
102-
# which do "CFLAGS := $(filter-out -fpch-preprocess -g1,$(CFLAGS))". fppinit is
102+
# which do "CFLAGS := $(filter-out $(PCH_CFLAGS) -g1,$(CFLAGS))". fppinit is
103103
# prepended to TARGETS, so it is built first and its objects are always what
104104
# pulls the shared .gch in, which means the .gch ends up built with flags that
105105
# no translation unit ever uses. gcc then silently discards it for every TU

src/makefiles/common/setup.mk

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,12 +129,18 @@ ifeq '$(findstring clang,$(CXXCOMPILER))' ''
129129
# Common CFLAGS
130130
ifeq ($(DISTRIBUTED_COMPILE),)
131131
PCH_FILE=fpp-pch.h.gch
132+
# Flags that mean something only to a compile that consumes the PCH. Objects
133+
# which deliberately opt out filter this same variable back out rather than
134+
# repeating its contents, so the two lists cannot drift apart -- see
135+
# makefiles/fppinit.mk.
136+
#
132137
# -Winvalid-pch: a .gch gcc decides it cannot use is discarded SILENTLY, and
133138
# the build then re-parses the whole of fpp-pch.h in every TU -- slower than
134139
# having no PCH at all, with nothing in the output to say so. That state went
135140
# unnoticed for two months (see the note on the fpp-pch.h.gch rule in
136141
# ../Makefile). This makes the next one loud instead: one warning per TU.
137-
CFLAGS+=-fpch-preprocess -Winvalid-pch
142+
PCH_CFLAGS := -fpch-preprocess -Winvalid-pch
143+
CFLAGS+=$(PCH_CFLAGS)
138144
else
139145
CFLAGS+=-DNOPCH
140146
endif

src/makefiles/fppinit.mk

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,11 @@ LIBS_fppinit += \
1616
FPPINIT_DEPS=libfpp_capeutils.so
1717
endif
1818

19-
boot/FPPINIT.o boot/FPPINIT_Config.o boot/FPPINIT_Network.o boot/FPPINIT_Audio.o: CFLAGS := $(filter-out -fpch-preprocess -g1,$(CFLAGS))
19+
# These objects do not use the PCH, so drop the PCH-only flags ($(PCH_CFLAGS),
20+
# defined next to where they are added in makefiles/common/setup.mk) along with
21+
# -g1. Note this is inherited by these targets' prerequisites: see the comment
22+
# on the fpp-pch.h.gch rule in ../Makefile for why that matters.
23+
boot/FPPINIT.o boot/FPPINIT_Config.o boot/FPPINIT_Network.o boot/FPPINIT_Audio.o: CFLAGS := $(filter-out $(PCH_CFLAGS) -g1,$(CFLAGS))
2024

2125

2226
fppinit: $(OBJECTS_fppinit) $(FPPINIT_DEPS)

0 commit comments

Comments
 (0)