Skip to content

Commit 644d154

Browse files
committedJan 19, 2025
8347474: Options singleton is used before options are parsed
Reviewed-by: vromero
1 parent 3804082 commit 644d154

File tree

16 files changed

+364
-171
lines changed

16 files changed

+364
-171
lines changed
 

‎src/jdk.compiler/share/classes/com/sun/tools/javac/code/Lint.java

+46-18
Original file line numberDiff line numberDiff line change
@@ -108,22 +108,44 @@ public Lint suppress(LintCategory... lc) {
108108
}
109109

110110
private final Context context;
111+
private final Options options;
111112

112113
// These are initialized lazily to avoid dependency loops
113114
private Symtab syms;
114115
private Names names;
115116

116117
// Invariant: it's never the case that a category is in both "values" and "suppressedValues"
117-
private final EnumSet<LintCategory> values;
118-
private final EnumSet<LintCategory> suppressedValues;
118+
private EnumSet<LintCategory> values;
119+
private EnumSet<LintCategory> suppressedValues;
119120

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

122123
@SuppressWarnings("this-escape")
123124
protected Lint(Context context) {
124-
// initialize values according to the lint options
125-
Options options = Options.instance(context);
125+
this.context = context;
126+
context.put(lintKey, this);
127+
options = Options.instance(context);
128+
}
129+
130+
// Instantiate a non-root ("symbol scoped") instance
131+
protected Lint(Lint other) {
132+
other.initializeRootIfNeeded();
133+
this.context = other.context;
134+
this.options = other.options;
135+
this.syms = other.syms;
136+
this.names = other.names;
137+
this.values = other.values.clone();
138+
this.suppressedValues = other.suppressedValues.clone();
139+
}
140+
141+
// Process command line options on demand to allow use of root Lint early during startup
142+
private void initializeRootIfNeeded() {
126143

144+
// Already initialized?
145+
if (values != null)
146+
return;
147+
148+
// Initialize enabled categories based on "-Xlint" flags
127149
if (options.isSet(Option.XLINT) || options.isSet(Option.XLINT_CUSTOM, "all")) {
128150
// If -Xlint or -Xlint:all is given, enable all categories by default
129151
values = EnumSet.allOf(LintCategory.class);
@@ -162,21 +184,11 @@ protected Lint(Context context) {
162184
}
163185

164186
suppressedValues = LintCategory.newEmptySet();
165-
166-
this.context = context;
167-
context.put(lintKey, this);
168-
}
169-
170-
protected Lint(Lint other) {
171-
this.context = other.context;
172-
this.syms = other.syms;
173-
this.names = other.names;
174-
this.values = other.values.clone();
175-
this.suppressedValues = other.suppressedValues.clone();
176187
}
177188

178189
@Override
179190
public String toString() {
191+
initializeRootIfNeeded();
180192
return "Lint:[enable" + values + ",suppress" + suppressedValues + "]";
181193
}
182194

@@ -404,6 +416,7 @@ public static EnumSet<LintCategory> newEmptySet() {
404416
* the SuppressWarnings annotation.
405417
*/
406418
public boolean isEnabled(LintCategory lc) {
419+
initializeRootIfNeeded();
407420
return values.contains(lc);
408421
}
409422

@@ -414,11 +427,26 @@ public boolean isEnabled(LintCategory lc) {
414427
* current entity being itself deprecated.
415428
*/
416429
public boolean isSuppressed(LintCategory lc) {
430+
initializeRootIfNeeded();
417431
return suppressedValues.contains(lc);
418432
}
419433

420434
/**
421435
* Helper method. Log a lint warning if its lint category is enabled.
436+
*
437+
* @param log warning destination
438+
* @param warning key for the localized warning message
439+
*/
440+
public void logIfEnabled(Log log, LintWarning warning) {
441+
logIfEnabled(log, null, warning);
442+
}
443+
444+
/**
445+
* Helper method. Log a lint warning if its lint category is enabled.
446+
*
447+
* @param log warning destination
448+
* @param pos source position at which to report the warning
449+
* @param warning key for the localized warning message
422450
*/
423451
public void logIfEnabled(Log log, DiagnosticPosition pos, LintWarning warning) {
424452
if (isEnabled(warning.getLintCategory())) {
@@ -450,7 +478,7 @@ public EnumSet<LintCategory> suppressionsFrom(Symbol symbol) {
450478
* @return set of lint categories, possibly empty but never null
451479
*/
452480
private EnumSet<LintCategory> suppressionsFrom(JCAnnotation annotation) {
453-
initializeIfNeeded();
481+
initializeSymbolsIfNeeded();
454482
if (annotation == null)
455483
return LintCategory.newEmptySet();
456484
Assert.check(annotation.attribute.type.tsym == syms.suppressWarningsType.tsym);
@@ -459,7 +487,7 @@ private EnumSet<LintCategory> suppressionsFrom(JCAnnotation annotation) {
459487

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

483-
private void initializeIfNeeded() {
511+
private void initializeSymbolsIfNeeded() {
484512
if (syms == null) {
485513
syms = Symtab.instance(context);
486514
names = Names.instance(context);

‎src/jdk.compiler/share/classes/com/sun/tools/javac/code/Preview.java

+16-12
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2018, 2024, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2018, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -68,6 +68,9 @@ public class Preview {
6868
/** flag: are preview features enabled */
6969
private final boolean enabled;
7070

71+
/** flag: is the "preview" lint category enabled? */
72+
private final boolean verbose;
73+
7174
/** the diag handler to manage preview feature usage diagnostics */
7275
private final MandatoryWarningHandler previewHandler;
7376

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

8285
private final Names names;
83-
private final Lint lint;
8486
private final Log log;
8587
private final Source source;
8688

@@ -101,10 +103,9 @@ protected Preview(Context context) {
101103
names = Names.instance(context);
102104
enabled = options.isSet(PREVIEW);
103105
log = Log.instance(context);
104-
lint = Lint.instance(context);
105106
source = Source.instance(context);
106-
this.previewHandler =
107-
new MandatoryWarningHandler(log, source, lint.isEnabled(LintCategory.PREVIEW), true, "preview", LintCategory.PREVIEW);
107+
verbose = Lint.instance(context).isEnabled(LintCategory.PREVIEW);
108+
previewHandler = new MandatoryWarningHandler(log, source, verbose, true, "preview", LintCategory.PREVIEW);
108109
forcePreview = options.isSet("forcePreview");
109110
majorVersionToSource = initMajorVersionToSourceMap();
110111
}
@@ -176,12 +177,10 @@ public void warnPreview(int pos, Feature feature) {
176177
public void warnPreview(DiagnosticPosition pos, Feature feature) {
177178
Assert.check(isEnabled());
178179
Assert.check(isPreview(feature));
179-
if (!lint.isSuppressed(LintCategory.PREVIEW)) {
180-
sourcesWithPreviewFeatures.add(log.currentSourceFile());
181-
previewHandler.report(pos, feature.isPlural() ?
182-
LintWarnings.PreviewFeatureUsePlural(feature.nameFragment()) :
183-
LintWarnings.PreviewFeatureUse(feature.nameFragment()));
184-
}
180+
markUsesPreview(pos);
181+
previewHandler.report(pos, feature.isPlural() ?
182+
LintWarnings.PreviewFeatureUsePlural(feature.nameFragment()) :
183+
LintWarnings.PreviewFeatureUse(feature.nameFragment()));
185184
}
186185

187186
/**
@@ -191,12 +190,17 @@ public void warnPreview(DiagnosticPosition pos, Feature feature) {
191190
*/
192191
public void warnPreview(JavaFileObject classfile, int majorVersion) {
193192
Assert.check(isEnabled());
194-
if (lint.isEnabled(LintCategory.PREVIEW)) {
193+
if (verbose) {
195194
log.mandatoryWarning(null,
196195
LintWarnings.PreviewFeatureUseClassfile(classfile, majorVersionToSource.get(majorVersion).name));
197196
}
198197
}
199198

199+
/**
200+
* Mark the current source file as using a preview feature. The corresponding classfile
201+
* will be generated with minor version {@link ClassFile#PREVIEW_MINOR_VERSION}.
202+
* @param pos the position at which the preview feature was used.
203+
*/
200204
public void markUsesPreview(DiagnosticPosition pos) {
201205
sourcesWithPreviewFeatures.add(log.currentSourceFile());
202206
}

‎src/jdk.compiler/share/classes/com/sun/tools/javac/file/BaseFileManager.java

+23-23
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
import javax.tools.JavaFileObject;
5454
import javax.tools.JavaFileObject.Kind;
5555

56+
import com.sun.tools.javac.code.Lint;
5657
import com.sun.tools.javac.code.Lint.LintCategory;
5758
import com.sun.tools.javac.main.Option;
5859
import com.sun.tools.javac.main.OptionHelper;
@@ -87,18 +88,11 @@ protected BaseFileManager(Charset charset) {
8788
*/
8889
public void setContext(Context context) {
8990
log = Log.instance(context);
91+
lint = Lint.instance(context);
9092
options = Options.instance(context);
91-
classLoaderClass = options.get("procloader");
9293

93-
// Detect Lint options, but use Options.isLintSet() to avoid initializing the Lint class
94-
boolean warn = options.isLintSet(LintCategory.PATH.option);
95-
boolean fileClashOption = options.isLintSet(LintCategory.OUTPUT_FILE_CLASH.option);
96-
locations.update(log, warn, FSInfo.instance(context));
97-
98-
// Only track file clashes if enabled
99-
synchronized (this) {
100-
outputFilesWritten = fileClashOption ? new HashSet<>() : null;
101-
}
94+
// Initialize locations
95+
locations.update(log, lint, FSInfo.instance(context));
10296

10397
// Setting this option is an indication that close() should defer actually closing
10498
// the file manager until after a specified period of inactivity.
@@ -112,14 +106,16 @@ public void setContext(Context context) {
112106
// in seconds, of the period of inactivity to wait for, before the file manager
113107
// is actually closed.
114108
// See also deferredClose().
115-
String s = options.get("fileManager.deferClose");
116-
if (s != null) {
117-
try {
118-
deferredCloseTimeout = (int) (Float.parseFloat(s) * 1000);
119-
} catch (NumberFormatException e) {
120-
deferredCloseTimeout = 60 * 1000; // default: one minute, in millis
109+
options.whenReady(options -> {
110+
String s = options.get("fileManager.deferClose");
111+
if (s != null) {
112+
try {
113+
deferredCloseTimeout = (int) (Float.parseFloat(s) * 1000);
114+
} catch (NumberFormatException e) {
115+
deferredCloseTimeout = 60 * 1000; // default: one minute, in millis
116+
}
121117
}
122-
}
118+
});
123119
}
124120

125121
protected Locations createLocations() {
@@ -138,12 +134,11 @@ protected Locations createLocations() {
138134

139135
protected Options options;
140136

141-
protected String classLoaderClass;
137+
protected Lint lint;
142138

143139
protected final Locations locations;
144140

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

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

200195
// 1: Allow client to specify the class to use via hidden option
196+
String classLoaderClass = options.get("procloader");
201197
if (classLoaderClass != null) {
202198
try {
203199
Class<? extends ClassLoader> loader =
@@ -243,6 +239,11 @@ public void remove(String name) {
243239
public boolean handleFileManagerOption(Option option, String value) {
244240
return handleOption(option, value);
245241
}
242+
243+
@Override
244+
public void initialize() {
245+
options.initialize();
246+
}
246247
};
247248

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

459460
public synchronized void resetOutputFilesWritten() {
460-
if (outputFilesWritten != null)
461-
outputFilesWritten.clear();
461+
outputFilesWritten.clear();
462462
}
463463

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

517517
// Is output file clash detection enabled?
518-
if (outputFilesWritten == null)
518+
if (!lint.isEnabled(LintCategory.OUTPUT_FILE_CLASH))
519519
return;
520520

521521
// Get the "canonical" version of the file's path; we are assuming

0 commit comments

Comments
 (0)
Please sign in to comment.