On Tue, 3 Feb 2026 15:46:02 GMT, Leo Korinth <[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`)
>
> Leo Korinth has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Changes after comments from Erik and Kim

This looks ok from a build perspective. The question about supporting "warning 
ghettos" in Hotspot isn't really something I want to get into, so will leave 
that for other reviewers, so if you can reach agreement there, then this is ok 
with me.

-------------

Marked as reviewed by erikj (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/29523#pullrequestreview-3746358521

Reply via email to