RFR: 8359596: Behavior change when both -Xlint:options and -Xlint:-options flags are given [v2]

[Maurizio Cimadamore]

On Wed, 18 Jun 2025 21:12:12 GMT, Archie Cobbs <acobbs at openjdk.org> wrote:

>> My minor contribution to #24746 (which fixed [JDK-8354556](https://bugs.openjdk.org/browse/JDK-8354556)) accidentally introduced a change in the compiler's behavior when given conflicting lint flags like `-Xlint:options -Xlint:-options`. This PR restores the original behavior.
>> 
>> Although this might be considered a weird corner case, many build systems add flags in multiple stages and this can easily result in both flags being added, and so the behavior in this scenario needs to stay consistent. 
>> 
>> Basically the code was trying to be too clever; when the original logic is restored, the code gets simpler.
>
> Archie Cobbs has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Ensure that "-Xlint:none" still works for the affected warnings.
>   
>   The extra checks for "-Xlint:none" are needed now because of JDK-8352612,
>   which changed the behavior of "-Xlint:none" to no longer imply "-nowarn",
>   which allowed the affected warnings to get away with skipping that check.

> I generally agree with these changes, but I'm not sure that the code in `Lint` (which is not shown in this diff as it has not changed) really adheres to the policy described in the JBS issue that "last option should win". What Lint seems to do is to compute an _initial_ set, like follows:
> 
>     * if `none` is found, start from the empty set
> 
>     * if `all` is found, start from the "everything" set
> 
>     * if neither is found, start from a "predefined" set which contains some default categories
> 
> 
> After that, the initial set is augmented (or reduced) based on the explicit enable/disable options.
> 
> I see at least two issues in this logic:
> 
>     * how is the initial set determined if both `all` and `none` are specified? The "last win" policy might suggest that we use the last of these to determine the initial set, but the code doesn't do that, and I think it gives precedence to `all`.
> 
>     * what if the same category is both enabled and disabled? Well, again, the code doesn't really apply a "last win" policy -- instead, for each category, we do this:
> 
> 
> ```
> // Look for specific overrides
>         for (LintCategory lc : LintCategory.values()) {
>             if (options.isExplicitlyEnabled(Option.XLINT, lc)) {
>                 values.add(lc);
>             } else if (options.isExplicitlyDisabled(Option.XLINT, lc)) {
>                 values.remove(lc);
>             }
>         }
> ```
> 
> That is, for each category we check if it's enabled -- if so we add it to the set. Otherwise if it's disabled, we remove it from the set. So, if a category is both enabled and disabled, it looks like the code should just treat it as enabled?
> 
> So, I'm not sure how this fix (or even the previous code, before we changed any of this) adheres/adhered to the "last win" policy?

Ok, I see that this change only really affects how `options` behave (and few other lint warnings). After debugging, I can clearly see that my analysis above is correct, and that the way in which the set of enabled lint warnings is computed does NOT conform to a "last win" policy. So, if you compile the code below with `-Xlint:unchecked -Xlint:-unchecked`:


class Test<X> {
    static void m(Test t) {
        Test<String> ts = t;
    }
}


It does print the lint warning (meaning that `Werror` would make this fail, even though the last `lint` command passed disables unchecked warnings).

So, the real fix of this PR is in using `isDisabled` instead of `isExplicitlyDisabled`. But I must notice that this carves out a special expection for `options` and `path` lint warnings -- whereas every other lint warning will not resolve "conflicts"  in the same way.

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

PR Comment: https://git.openjdk.org/jdk/pull/25840#issuecomment-2995926729