Skip to content

Commit

Permalink
8344148: Add an explicit compiler phase for warning generation
Browse files Browse the repository at this point in the history
Reviewed-by: vromero
  • Loading branch information
archiecobbs committed Jan 6, 2025
1 parent 8d388cc commit 27646e5
Show file tree
Hide file tree
Showing 19 changed files with 219 additions and 30 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2013, 2024, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2013, 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 @@ -58,11 +58,12 @@ public enum CompileState {
PROCESS(3),
ATTR(4),
FLOW(5),
TRANSTYPES(6),
TRANSPATTERNS(7),
LOWER(8),
UNLAMBDA(9),
GENERATE(10);
WARN(6),
TRANSTYPES(7),
TRANSPATTERNS(8),
LOWER(9),
UNLAMBDA(10),
GENERATE(11);

CompileState(int value) {
this.value = value;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1999, 2024, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1999, 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 @@ -229,7 +229,6 @@ public void analyzeTree(Env<AttrContext> env, TreeMaker make) {
new AssignAnalyzer().analyzeTree(env, make);
new FlowAnalyzer().analyzeTree(env, make);
new CaptureAnalyzer().analyzeTree(env, make);
new ThisEscapeAnalyzer(names, syms, types, rs, log, lint).analyzeTree(env);
}

public void analyzeLambda(Env<AttrContext> env, JCLambda that, TreeMaker make, boolean speculative) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2023, 2024, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2023, 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 @@ -58,6 +58,7 @@
import com.sun.tools.javac.tree.TreeInfo;
import com.sun.tools.javac.tree.TreeScanner;
import com.sun.tools.javac.util.Assert;
import com.sun.tools.javac.util.Context;
import com.sun.tools.javac.util.JCDiagnostic;
import com.sun.tools.javac.util.JCDiagnostic.DiagnosticPosition;
import com.sun.tools.javac.util.List;
Expand Down Expand Up @@ -140,8 +141,17 @@
* <li>We assume that native methods do not leak.
* <li>We don't try to follow {@code super()} invocations; that's for the superclass analysis to handle.
* </ul>
*
* <p><b>This is NOT part of any supported API.
* If you write code that depends on this, you do so at your own risk.
* This code and its internal interfaces are subject to change or
* deletion without notice.</b>
*/
class ThisEscapeAnalyzer extends TreeScanner {
public class ThisEscapeAnalyzer extends TreeScanner {

protected static final Context.Key<ThisEscapeAnalyzer> contextKey = new Context.Key<>();

// Other singletons we utilize

private final Names names;
private final Symtab syms;
Expand Down Expand Up @@ -211,22 +221,49 @@ class ThisEscapeAnalyzer extends TreeScanner {
*/
private RefSet<Ref> refs;

// Constructor
// Access

ThisEscapeAnalyzer(Names names, Symtab syms, Types types, Resolve rs, Log log, Lint lint) {
this.names = names;
this.syms = syms;
this.types = types;
this.rs = rs;
this.log = log;
this.lint = lint;
public static ThisEscapeAnalyzer instance(Context context) {
ThisEscapeAnalyzer instance = context.get(contextKey);
if (instance == null)
instance = new ThisEscapeAnalyzer(context);
return instance;
}

@SuppressWarnings("this-escape")
protected ThisEscapeAnalyzer(Context context) {
context.put(contextKey, this);
names = Names.instance(context);
log = Log.instance(context);
syms = Symtab.instance(context);
types = Types.instance(context);
rs = Resolve.instance(context);
lint = Lint.instance(context);
}

//
// Main method
//

public void analyzeTree(Env<AttrContext> env) {
try {
doAnalyzeTree(env);
} finally {
attrEnv = null;
methodMap.clear();
nonPublicOuters.clear();
targetClass = null;
warningList.clear();
methodClass = null;
callStack.clear();
invocations.clear();
pendingWarning = null;
depth = -1;
refs = null;
}
}

private void doAnalyzeTree(Env<AttrContext> env) {

// Sanity check
Assert.check(checkInvariants(false, false));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1999, 2024, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1999, 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 @@ -1168,7 +1168,7 @@ private Type erasure(Type t) {
private Env<AttrContext> env;

private static final String statePreviousToFlowAssertMsg =
"The current compile state [%s] of class %s is previous to FLOW";
"The current compile state [%s] of class %s is previous to WARN";

void translateClass(ClassSymbol c) {
Type st = types.supertype(c.type);
Expand All @@ -1189,15 +1189,15 @@ void translateClass(ClassSymbol c) {
* 1) has no compile state being it the most outer class.
* We accept this condition for inner classes.
*
* 2) has a compile state which is previous to Flow state.
* 2) has a compile state which is previous to WARN state.
*/
boolean envHasCompState = compileStates.get(myEnv) != null;
if (!envHasCompState && c.outermostClass() == c) {
Assert.error("No info for outermost class: " + myEnv.enclClass.sym);
}

if (envHasCompState &&
CompileState.FLOW.isAfter(compileStates.get(myEnv))) {
CompileState.WARN.isAfter(compileStates.get(myEnv))) {
Assert.error(String.format(statePreviousToFlowAssertMsg,
compileStates.get(myEnv), myEnv.enclClass.sym));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
* Copyright (c) 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
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation. Oracle designates this
* particular file as subject to the "Classpath" exception as provided
* by Oracle in the LICENSE file that accompanied this code.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

package com.sun.tools.javac.comp;

import com.sun.tools.javac.util.Context;
import com.sun.tools.javac.util.Log;

/** This pass checks for various things to warn about.
* It runs after attribution and flow analysis.
*
* <p><b>This is NOT part of any supported API.
* If you write code that depends on this, you do so at your own risk.
* This code and its internal interfaces are subject to change or
* deletion without notice.</b>
*/
public class WarningAnalyzer {

protected static final Context.Key<WarningAnalyzer> contextKey = new Context.Key<>();

private final Log log;
private final ThisEscapeAnalyzer thisEscapeAnalyzer;

public static WarningAnalyzer instance(Context context) {
WarningAnalyzer instance = context.get(contextKey);
if (instance == null)
instance = new WarningAnalyzer(context);
return instance;
}

@SuppressWarnings("this-escape")
protected WarningAnalyzer(Context context) {
context.put(contextKey, this);
log = Log.instance(context);
thisEscapeAnalyzer = ThisEscapeAnalyzer.instance(context);
}

public void analyzeTree(Env<AttrContext> env) {
thisEscapeAnalyzer.analyzeTree(env);
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1999, 2024, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1999, 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 @@ -306,6 +306,10 @@ else if (option.equals("class"))
*/
protected Flow flow;

/** The warning analyzer.
*/
protected WarningAnalyzer warningAnalyzer;

/** The modules visitor
*/
protected Modules modules;
Expand Down Expand Up @@ -419,6 +423,7 @@ public JavaCompiler(Context context) {
chk = Check.instance(context);
gen = Gen.instance(context);
flow = Flow.instance(context);
warningAnalyzer = WarningAnalyzer.instance(context);
transTypes = TransTypes.instance(context);
lower = Lower.instance(context);
annotate = Annotate.instance(context);
Expand Down Expand Up @@ -962,20 +967,20 @@ public void compile(Collection<JavaFileObject> sourceFileObjects,
if (!CompileState.ATTR.isAfter(shouldStopPolicyIfNoError)) {
switch (compilePolicy) {
case SIMPLE:
generate(desugar(flow(attribute(todo))));
generate(desugar(warn(flow(attribute(todo)))));
break;

case BY_FILE: {
Queue<Queue<Env<AttrContext>>> q = todo.groupByFile();
while (!q.isEmpty() && !shouldStop(CompileState.ATTR)) {
generate(desugar(flow(attribute(q.remove()))));
generate(desugar(warn(flow(attribute(q.remove())))));
}
}
break;

case BY_TODO:
while (!todo.isEmpty())
generate(desugar(flow(attribute(todo.remove()))));
generate(desugar(warn(flow(attribute(todo.remove())))));
break;

default:
Expand Down Expand Up @@ -1435,6 +1440,56 @@ protected void flow(Env<AttrContext> env, Queue<Env<AttrContext>> results) {
}
}

/**
* Check for various things to warn about.
*
* @return the list of attributed parse trees
*/
public Queue<Env<AttrContext>> warn(Queue<Env<AttrContext>> envs) {
ListBuffer<Env<AttrContext>> results = new ListBuffer<>();
for (Env<AttrContext> env: envs) {
warn(env, results);
}
return stopIfError(CompileState.WARN, results);
}

/**
* Check for various things to warn about in an attributed parse tree.
*/
public Queue<Env<AttrContext>> warn(Env<AttrContext> env) {
ListBuffer<Env<AttrContext>> results = new ListBuffer<>();
warn(env, results);
return stopIfError(CompileState.WARN, results);
}

/**
* Check for various things to warn about in an attributed parse tree.
*/
protected void warn(Env<AttrContext> env, Queue<Env<AttrContext>> results) {
if (compileStates.isDone(env, CompileState.WARN)) {
results.add(env);
return;
}

if (shouldStop(CompileState.WARN))
return;

if (verboseCompilePolicy)
printNote("[warn " + env.enclClass.sym + "]");
JavaFileObject prev = log.useSource(
env.enclClass.sym.sourcefile != null ?
env.enclClass.sym.sourcefile :
env.toplevel.sourcefile);
try {
warningAnalyzer.analyzeTree(env);
compileStates.put(env, CompileState.WARN);
results.add(env);
}
finally {
log.useSource(prev);
}
}

private TaskEvent newAnalyzeTaskEvent(Env<AttrContext> env) {
JCCompilationUnit toplevel = env.toplevel;
ClassSymbol sym;
Expand Down Expand Up @@ -1493,6 +1548,10 @@ protected void desugar(final Env<AttrContext> env, Queue<Pair<Env<AttrContext>,
return;
}

// Ensure the file has reached the WARN state
if (!compileStates.isDone(env, CompileState.WARN))
warn(env);

/**
* Ensure that superclasses of C are desugared before C itself. This is
* required for two reasons: (i) as erasure (TransTypes) destroys
Expand Down Expand Up @@ -1576,8 +1635,8 @@ public void visitSwitchExpression(JCSwitchExpression tree) {
ScanNested scanner = new ScanNested();
scanner.scan(env.tree);
for (Env<AttrContext> dep: scanner.dependencies) {
if (!compileStates.isDone(dep, CompileState.FLOW))
desugaredEnvs.put(dep, desugar(flow(attribute(dep))));
if (!compileStates.isDone(dep, CompileState.WARN))
desugaredEnvs.put(dep, desugar(warn(flow(attribute(dep)))));
}

//We need to check for error another time as more classes might
Expand Down
3 changes: 3 additions & 0 deletions test/langtools/tools/javac/6734819/T6734819a.out
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
[attribute Y]
[flow Y]
[warn Y]
[attribute W]
[flow W]
[warn W]
[attribute Z]
[flow Z]
[warn Z]
[desugar Z]
[desugar W]
[desugar Y]
Expand Down
2 changes: 2 additions & 0 deletions test/langtools/tools/javac/6734819/T6734819b.out
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
[attribute A]
[flow A]
[warn A]
[attribute B]
[flow B]
[warn B]
[desugar B]
[desugar A]
[generate code A]
Expand Down
2 changes: 2 additions & 0 deletions test/langtools/tools/javac/6734819/T6734819c.out
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
[attribute Y]
[flow Y]
[warn Y]
[attribute W]
[flow W]
[warn W]
[attribute Z]
[flow Z]
T6734819c.java:15:11: compiler.err.unreachable.stmt
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[attribute Explicit]
[flow Explicit]
[warn Explicit]
[desugar Explicit]
[generate code Explicit]
Loading

0 comments on commit 27646e5

Please sign in to comment.