On Mon, 2 Feb 2026 14:41:35 GMT, Erik Joelsson <[email protected]> wrote:
>> Allow conversion warnings in subsets of the code base. By allowing this, we >> can improve the code base in parts, and see that those parts do not regress >> in the future. >> >> My approach to implement this is by adding support to our make system to >> recognise and handle "variable packs". A "variable pack" is a list of quoted >> variable "appendings". It will be picked up by `NamedParamsMacroTemplate` >> and there recognised by the lack of an assignment operator that is always >> used when sending variables to macros today. To support sending lists of >> "variable appendings", the appendings must quote assignment, spaces and >> quotes. This would be cleanest to implement by hex or base64 encode the >> string. However, this is extremely hard to do in make, and I prefer not >> calling the likes of `od` or `base64` to make the code portable and fast. >> >> With this infrastructure I implement a simple recursive utility to find all >> files matching a pattern in a folder; I then transform that list to variable >> assignments that will add compiler warnings for those files. >> >> This approach is extremely flexible. I can for example combine many calls to >> the `overrideFlags` macro with different source directories and different >> patterns. >> >> The macro will expand to something like (depending on compiler): >> `module_file1.cpp_CXXFLAGS+=-Wconversion` >> `module_file2.cpp_CXXFLAGS+=-Wconversion` >> >> this can flexibly be combined with other flags to overlap: >> `module_file2.cpp_CXXFLAGS+=$(SPACE)-Wotherflag` >> `module_file3.cpp_CXXFLAGS+=$(SPACE)-Wotherflag` >> >> (note the overlapping sets of flags `file1 -Wconversion`, `file2 >> -Wconversion -Wotherflag`, `file3 -Wotherflag`) > > make/common/MakeBase.gmk line 144: > >> 142: $(eval $(call SetupLogging)) >> 143: >> 144: >> ################################################################################ > > Could you add a description of what this section tries to do and how to use > it here? Something similar to the PR comment. Yes. > make/common/MakeBase.gmk line 180: > >> 178: >> 179: # $(call overrideFlags,dir,%.cpp,_CXXFLAGS,-Wconversion) -> >> file1.cpp_CXXFLAGS#+-Wconversion file2.cpp_CXXFLAGS#+-Wconversion >> 180: overrideFlags = $(foreach file,$(call filterFiles,$1,$2,),$(call >> quoteAppend,$(file)$3,$4)) > > I think this is the only macro here that is meant to be called by users. It > would help if you could move it to either the top or the bottom and give it a > proper description on now it is expected to be used. I will do so. I think `quoteAppend` might also be useful as a general way to send "variable packs" or "varargs" and possibly used by others. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/29523#discussion_r2755409462 PR Review Comment: https://git.openjdk.org/jdk/pull/29523#discussion_r2755405721
