Skip to content

Commit

Permalink
8347474: Options singleton is used before options are parsed
Browse files Browse the repository at this point in the history 
Reviewed-by: vromero
  • Loading branch information
@archiecobbs
archiecobbs committed Jan 19, 2025
1 parent 3804082 commit 644d154
Show file tree
Hide file tree
Showing 16 changed files with 364 additions and 171 deletions.
64 changes: 46 additions & 18 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/code/Lint.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,22 +108,44 @@ public Lint suppress(LintCategory... lc) {
}

private final Context context;
private final Options options;

// These are initialized lazily to avoid dependency loops
private Symtab syms;
private Names names;

// Invariant: it's never the case that a category is in both "values" and "suppressedValues"
private final EnumSet<LintCategory> values;
private final EnumSet<LintCategory> suppressedValues;
private EnumSet<LintCategory> values;
private EnumSet<LintCategory> suppressedValues;

private static final Map<String, LintCategory> map = new ConcurrentHashMap<>(20);

@SuppressWarnings("this-escape")
protected Lint(Context context) {
// initialize values according to the lint options
Options options = Options.instance(context);
this.context = context;
context.put(lintKey, this);
options = Options.instance(context);
}

// Instantiate a non-root ("symbol scoped") instance
protected Lint(Lint other) {
other.initializeRootIfNeeded();
this.context = other.context;
this.options = other.options;
this.syms = other.syms;
this.names = other.names;
this.values = other.values.clone();
this.suppressedValues = other.suppressedValues.clone();
}

// Process command line options on demand to allow use of root Lint early during startup
private void initializeRootIfNeeded() {

// Already initialized?
if (values != null)
return;

// Initialize enabled categories based on "-Xlint" flags
if (options.isSet(Option.XLINT) || options.isSet(Option.XLINT_CUSTOM, "all")) {
// If -Xlint or -Xlint:all is given, enable all categories by default
values = EnumSet.allOf(LintCategory.class);
Expand Down Expand Up @@ -162,21 +184,11 @@ protected Lint(Context context) {
}

suppressedValues = LintCategory.newEmptySet();

this.context = context;
context.put(lintKey, this);
}

protected Lint(Lint other) {
this.context = other.context;
this.syms = other.syms;
this.names = other.names;
this.values = other.values.clone();
this.suppressedValues = other.suppressedValues.clone();
}

@Override
public String toString() {
initializeRootIfNeeded();
return "Lint:[enable" + values + ",suppress" + suppressedValues + "]";
}

Expand Down Expand Up @@ -404,6 +416,7 @@ public static EnumSet<LintCategory> newEmptySet() {
* the SuppressWarnings annotation.
*/
public boolean isEnabled(LintCategory lc) {
initializeRootIfNeeded();
return values.contains(lc);
}

Expand All @@ -414,11 +427,26 @@ public boolean isEnabled(LintCategory lc) {
* current entity being itself deprecated.
*/
public boolean isSuppressed(LintCategory lc) {
initializeRootIfNeeded();
return suppressedValues.contains(lc);
}

/**
* Helper method. Log a lint warning if its lint category is enabled.
*
* @param log warning destination
* @param warning key for the localized warning message
*/
public void logIfEnabled(Log log, LintWarning warning) {
logIfEnabled(log, null, warning);
}

/**
* Helper method. Log a lint warning if its lint category is enabled.
*
* @param log warning destination
* @param pos source position at which to report the warning
* @param warning key for the localized warning message
*/
public void logIfEnabled(Log log, DiagnosticPosition pos, LintWarning warning) {
if (isEnabled(warning.getLintCategory())) {
Expand Down Expand Up @@ -450,7 +478,7 @@ public EnumSet<LintCategory> suppressionsFrom(Symbol symbol) {
* @return set of lint categories, possibly empty but never null
*/
private EnumSet<LintCategory> suppressionsFrom(JCAnnotation annotation) {
initializeIfNeeded();
initializeSymbolsIfNeeded();
if (annotation == null)
return LintCategory.newEmptySet();
Assert.check(annotation.attribute.type.tsym == syms.suppressWarningsType.tsym);
Expand All @@ -459,7 +487,7 @@ private EnumSet<LintCategory> suppressionsFrom(JCAnnotation annotation) {

// Find the @SuppressWarnings annotation in the given stream and extract the recognized suppressions
private EnumSet<LintCategory> suppressionsFrom(Stream<Attribute.Compound> attributes) {
initializeIfNeeded();
initializeSymbolsIfNeeded();
EnumSet<LintCategory> result = LintCategory.newEmptySet();
attributes
.filter(attribute -> attribute.type.tsym == syms.suppressWarningsType.tsym)
Expand All @@ -480,7 +508,7 @@ private EnumSet<LintCategory> suppressionsFrom(Attribute.Compound suppressWarnin
return result;
}

private void initializeIfNeeded() {
private void initializeSymbolsIfNeeded() {
if (syms == null) {
syms = Symtab.instance(context);
names = Names.instance(context);
Expand Down
28 changes: 16 additions & 12 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/code/Preview.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, 2024, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2018, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -68,6 +68,9 @@ public class Preview {
/** flag: are preview features enabled */
private final boolean enabled;

/** flag: is the "preview" lint category enabled? */
private final boolean verbose;

/** the diag handler to manage preview feature usage diagnostics */
private final MandatoryWarningHandler previewHandler;

Expand All @@ -80,7 +83,6 @@ public class Preview {
private final Set<JavaFileObject> sourcesWithPreviewFeatures = new HashSet<>();

private final Names names;
private final Lint lint;
private final Log log;
private final Source source;

Expand All @@ -101,10 +103,9 @@ protected Preview(Context context) {
names = Names.instance(context);
enabled = options.isSet(PREVIEW);
log = Log.instance(context);
lint = Lint.instance(context);
source = Source.instance(context);
this.previewHandler =
new MandatoryWarningHandler(log, source, lint.isEnabled(LintCategory.PREVIEW), true, "preview", LintCategory.PREVIEW);
verbose = Lint.instance(context).isEnabled(LintCategory.PREVIEW);
previewHandler = new MandatoryWarningHandler(log, source, verbose, true, "preview", LintCategory.PREVIEW);
forcePreview = options.isSet("forcePreview");
majorVersionToSource = initMajorVersionToSourceMap();
}
Expand Down Expand Up @@ -176,12 +177,10 @@ public void warnPreview(int pos, Feature feature) {
public void warnPreview(DiagnosticPosition pos, Feature feature) {
Assert.check(isEnabled());
Assert.check(isPreview(feature));
if (!lint.isSuppressed(LintCategory.PREVIEW)) {
sourcesWithPreviewFeatures.add(log.currentSourceFile());
previewHandler.report(pos, feature.isPlural() ?
LintWarnings.PreviewFeatureUsePlural(feature.nameFragment()) :
LintWarnings.PreviewFeatureUse(feature.nameFragment()));
}
markUsesPreview(pos);
previewHandler.report(pos, feature.isPlural() ?
LintWarnings.PreviewFeatureUsePlural(feature.nameFragment()) :
LintWarnings.PreviewFeatureUse(feature.nameFragment()));
}

/**
Expand All @@ -191,12 +190,17 @@ public void warnPreview(DiagnosticPosition pos, Feature feature) {
*/
public void warnPreview(JavaFileObject classfile, int majorVersion) {
Assert.check(isEnabled());
if (lint.isEnabled(LintCategory.PREVIEW)) {
if (verbose) {
log.mandatoryWarning(null,
LintWarnings.PreviewFeatureUseClassfile(classfile, majorVersionToSource.get(majorVersion).name));
}
}

/**
* Mark the current source file as using a preview feature. The corresponding classfile
* will be generated with minor version {@link ClassFile#PREVIEW_MINOR_VERSION}.
* @param pos the position at which the preview feature was used.
*/
public void markUsesPreview(DiagnosticPosition pos) {
sourcesWithPreviewFeatures.add(log.currentSourceFile());
}
Expand Down
46 changes: 23 additions & 23 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/file/BaseFileManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
import javax.tools.JavaFileObject;
import javax.tools.JavaFileObject.Kind;

import com.sun.tools.javac.code.Lint;
import com.sun.tools.javac.code.Lint.LintCategory;
import com.sun.tools.javac.main.Option;
import com.sun.tools.javac.main.OptionHelper;
Expand Down Expand Up @@ -87,18 +88,11 @@ protected BaseFileManager(Charset charset) {
*/
public void setContext(Context context) {
log = Log.instance(context);
lint = Lint.instance(context);
options = Options.instance(context);
classLoaderClass = options.get("procloader");

// Detect Lint options, but use Options.isLintSet() to avoid initializing the Lint class
boolean warn = options.isLintSet(LintCategory.PATH.option);
boolean fileClashOption = options.isLintSet(LintCategory.OUTPUT_FILE_CLASH.option);
locations.update(log, warn, FSInfo.instance(context));

// Only track file clashes if enabled
synchronized (this) {
outputFilesWritten = fileClashOption ? new HashSet<>() : null;
}
// Initialize locations
locations.update(log, lint, FSInfo.instance(context));

// Setting this option is an indication that close() should defer actually closing
// the file manager until after a specified period of inactivity.
Expand All @@ -112,14 +106,16 @@ public void setContext(Context context) {
// in seconds, of the period of inactivity to wait for, before the file manager
// is actually closed.
// See also deferredClose().
String s = options.get("fileManager.deferClose");
if (s != null) {
try {
deferredCloseTimeout = (int) (Float.parseFloat(s) * 1000);
} catch (NumberFormatException e) {
deferredCloseTimeout = 60 * 1000; // default: one minute, in millis
options.whenReady(options -> {
String s = options.get("fileManager.deferClose");
if (s != null) {
try {
deferredCloseTimeout = (int) (Float.parseFloat(s) * 1000);
} catch (NumberFormatException e) {
deferredCloseTimeout = 60 * 1000; // default: one minute, in millis
}
}
}
});
}

protected Locations createLocations() {
Expand All @@ -138,12 +134,11 @@ protected Locations createLocations() {

protected Options options;

protected String classLoaderClass;
protected Lint lint;

protected final Locations locations;

// This is non-null when output file clash detection is enabled
private HashSet<Path> outputFilesWritten;
private final HashSet<Path> outputFilesWritten = new HashSet<>();

/**
* A flag for clients to use to indicate that this file manager should
Expand Down Expand Up @@ -198,6 +193,7 @@ protected ClassLoader getClassLoader(URL[] urls) {
// other than URLClassLoader.

// 1: Allow client to specify the class to use via hidden option
String classLoaderClass = options.get("procloader");
if (classLoaderClass != null) {
try {
Class<? extends ClassLoader> loader =
Expand Down Expand Up @@ -243,6 +239,11 @@ public void remove(String name) {
public boolean handleFileManagerOption(Option option, String value) {
return handleOption(option, value);
}

@Override
public void initialize() {
options.initialize();
}
};

Option o = Option.lookup(current, javacFileManagerOptions);
Expand Down Expand Up @@ -457,8 +458,7 @@ public void flushCache(JavaFileObject file) {
}

public synchronized void resetOutputFilesWritten() {
if (outputFilesWritten != null)
outputFilesWritten.clear();
outputFilesWritten.clear();
}

protected final Map<JavaFileObject, ContentCacheEntry> contentCache = new HashMap<>();
Expand Down Expand Up @@ -515,7 +515,7 @@ protected static <T> Collection<T> nullCheck(Collection<T> it) {
synchronized void newOutputToPath(Path path) throws IOException {

// Is output file clash detection enabled?
if (outputFilesWritten == null)
if (!lint.isEnabled(LintCategory.OUTPUT_FILE_CLASH))
return;

// Get the "canonical" version of the file's path; we are assuming
Expand Down
Loading

0 comments on commit 644d154

Please sign in to comment.