Skip to content

Commit

Permalink
8292275: javac does not emit SYNTHETIC and MANDATED flags for paramet…
Browse files Browse the repository at this point in the history 
…ers by default

Co-authored-by: Chen Liang <liach@openjdk.org>
Reviewed-by: vromero, jwaters
  • Loading branch information
@SirYwell @liach @TheShermanTanker
2 people authored and TheShermanTanker committed Apr 30, 2023
1 parent 6d6d00b commit b3dbf28
Show file tree
Hide file tree
Showing 9 changed files with 604 additions and 107 deletions.
5 changes: 3 additions & 2 deletions make/langtools/src/classes/build/tools/symbolgenerator/CreateSymbols.java
View file
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2006, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2006, 2023, 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 @@ -2384,7 +2384,8 @@ private boolean readAttribute(ClassFile cf, FeatureDescription feature, Attribut
MethodDescription method = (MethodDescription) feature;
method.methodParameters = new ArrayList<>();
for (MethodParameters_attribute.Entry e : params.method_parameter_table) {
String name = cf.constant_pool.getUTF8Value(e.name_index);
String name = e.name_index == 0 ? null
: cf.constant_pool.getUTF8Value(e.name_index);
MethodDescription.MethodParam param =
new MethodDescription.MethodParam(e.flags, name);
method.methodParameters.add(param);
Expand Down
159 changes: 80 additions & 79 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java
View file
Expand Up @@ -202,7 +202,13 @@ public class ClassReader {
/** A table to hold the constant pool indices for method parameter
* names, as given in LocalVariableTable attributes.
*/
int[] parameterNameIndices;
int[] parameterNameIndicesLvt;

/**
* A table to hold the constant pool indices for method parameter
* names, as given in the MethodParameters attribute.
*/
int[] parameterNameIndicesMp;

/**
* A table to hold the access flags of the method parameters.
Expand All @@ -229,18 +235,6 @@ void add(List<CompoundAnnotationProxy> newAnnotations) {
}
}

/**
* Whether or not any parameter names have been found.
*/
boolean haveParameterNameIndices;

/** Set this to false every time we start reading a method
* and are saving parameter names. Set it to true when we see
* MethodParameters, if it's set when we see a LocalVariableTable,
* then we ignore the parameter names from the LVT.
*/
boolean sawMethodParameters;

/**
* The set of attribute names for which warnings have been generated for the current class
*/
Expand Down Expand Up @@ -939,7 +933,7 @@ protected void read(Symbol sym, int attrLen) {
new AttributeReader(names.LocalVariableTable, V45_3, CLASS_OR_MEMBER_ATTRIBUTE) {
protected void read(Symbol sym, int attrLen) {
int newbp = bp + attrLen;
if (saveParameterNames && !sawMethodParameters) {
if (saveParameterNames) {
// Pick up parameter names from the variable table.
// Parameter names are not explicitly identified as such,
// but all parameter name entries in the LocalVariableTable
Expand All @@ -958,14 +952,13 @@ protected void read(Symbol sym, int attrLen) {
int register = nextChar();
if (start_pc == 0) {
// ensure array large enough
if (register >= parameterNameIndices.length) {
if (register >= parameterNameIndicesLvt.length) {
int newSize =
Math.max(register + 1, parameterNameIndices.length + 8);
parameterNameIndices =
Arrays.copyOf(parameterNameIndices, newSize);
Math.max(register + 1, parameterNameIndicesLvt.length + 8);
parameterNameIndicesLvt =
Arrays.copyOf(parameterNameIndicesLvt, newSize);
}
parameterNameIndices[register] = nameIndex;
haveParameterNameIndices = true;
parameterNameIndicesLvt[register] = nameIndex;
}
}
}
Expand Down Expand Up @@ -1116,19 +1109,17 @@ protected void read(Symbol sym, int attrLen) {
protected void read(Symbol sym, int attrlen) {
int newbp = bp + attrlen;
if (saveParameterNames) {
sawMethodParameters = true;
int numEntries = nextByte();
parameterNameIndices = new int[numEntries];
parameterNameIndicesMp = new int[numEntries];
parameterAccessFlags = new int[numEntries];
haveParameterNameIndices = true;
int index = 0;
for (int i = 0; i < numEntries; i++) {
int nameIndex = nextChar();
int flags = nextChar();
if ((flags & (Flags.MANDATED | Flags.SYNTHETIC)) != 0) {
continue;
}
parameterNameIndices[index] = nameIndex;
parameterNameIndicesMp[index] = nameIndex;
parameterAccessFlags[index] = flags;
index++;
}
Expand Down Expand Up @@ -2396,13 +2387,11 @@ void initParameterNames(MethodSymbol sym) {
final int excessSlots = 4;
int expectedParameterSlots =
Code.width(sym.type.getParameterTypes()) + excessSlots;
if (parameterNameIndices == null
|| parameterNameIndices.length < expectedParameterSlots) {
parameterNameIndices = new int[expectedParameterSlots];
if (parameterNameIndicesLvt == null
|| parameterNameIndicesLvt.length < expectedParameterSlots) {
parameterNameIndicesLvt = new int[expectedParameterSlots];
} else
Arrays.fill(parameterNameIndices, 0);
haveParameterNameIndices = false;
sawMethodParameters = false;
Arrays.fill(parameterNameIndicesLvt, 0);
}

/**
Expand All @@ -2417,46 +2406,47 @@ void initParameterNames(MethodSymbol sym) {
* anonymous synthetic parameters.
*/
void setParameters(MethodSymbol sym, Type jvmType) {
// If we get parameter names from MethodParameters, then we
// don't need to skip.
int firstParam = 0;
if (!sawMethodParameters) {
firstParam = ((sym.flags() & STATIC) == 0) ? 1 : 0;
// the code in readMethod may have skipped the first
// parameter when setting up the MethodType. If so, we
// make a corresponding allowance here for the position of
// the first parameter. Note that this assumes the
// skipped parameter has a width of 1 -- i.e. it is not
// a double width type (long or double.)
if (sym.name == names.init && currentOwner.hasOuterInstance()) {
// Sometimes anonymous classes don't have an outer
// instance, however, there is no reliable way to tell so
// we never strip this$n
if (!currentOwner.name.isEmpty())
firstParam += 1;
}

if (sym.type != jvmType) {
// reading the method attributes has caused the
// symbol's type to be changed. (i.e. the Signature
// attribute.) This may happen if there are hidden
// (synthetic) parameters in the descriptor, but not
// in the Signature. The position of these hidden
// parameters is unspecified; for now, assume they are
// at the beginning, and so skip over them. The
// primary case for this is two hidden parameters
// passed into Enum constructors.
int skip = Code.width(jvmType.getParameterTypes())
- Code.width(sym.type.getParameterTypes());
firstParam += skip;
}
int firstParamLvt = ((sym.flags() & STATIC) == 0) ? 1 : 0;
// the code in readMethod may have skipped the first
// parameter when setting up the MethodType. If so, we
// make a corresponding allowance here for the position of
// the first parameter. Note that this assumes the
// skipped parameter has a width of 1 -- i.e. it is not
// a double width type (long or double.)
if (sym.name == names.init && currentOwner.hasOuterInstance()) {
// Sometimes anonymous classes don't have an outer
// instance, however, there is no reliable way to tell so
// we never strip this$n
if (!currentOwner.name.isEmpty())
firstParamLvt += 1;
}

if (sym.type != jvmType) {
// reading the method attributes has caused the
// symbol's type to be changed. (i.e. the Signature
// attribute.) This may happen if there are hidden
// (synthetic) parameters in the descriptor, but not
// in the Signature. The position of these hidden
// parameters is unspecified; for now, assume they are
// at the beginning, and so skip over them. The
// primary case for this is two hidden parameters
// passed into Enum constructors.
int skip = Code.width(jvmType.getParameterTypes())
- Code.width(sym.type.getParameterTypes());
firstParamLvt += skip;
}
Set<Name> paramNames = new HashSet<>();
ListBuffer<VarSymbol> params = new ListBuffer<>();
int nameIndex = firstParam;
// we maintain two index pointers, one for the LocalVariableTable attribute
// and the other for the MethodParameters attribute.
// This is needed as the MethodParameters attribute may contain
// name_index = 0 in which case we want to fall back to the LocalVariableTable.
// In such case, we still want to read the flags from the MethodParameters with that index.
int nameIndexLvt = firstParamLvt;
int nameIndexMp = 0;
int annotationIndex = 0;
for (Type t: sym.type.getParameterTypes()) {
VarSymbol param = parameter(nameIndex, t, sym, paramNames);
VarSymbol param = parameter(nameIndexMp, nameIndexLvt, t, sym, paramNames);
params.append(param);
if (parameterAnnotations != null) {
ParameterAnnotations annotations = parameterAnnotations[annotationIndex];
Expand All @@ -2465,7 +2455,8 @@ void setParameters(MethodSymbol sym, Type jvmType) {
annotate.normal(new AnnotationCompleter(param, annotations.proxies));
}
}
nameIndex += sawMethodParameters ? 1 : Code.width(t);
nameIndexLvt += Code.width(t);
nameIndexMp++;
annotationIndex++;
}
if (parameterAnnotations != null && parameterAnnotations.length != annotationIndex) {
Expand All @@ -2474,24 +2465,34 @@ void setParameters(MethodSymbol sym, Type jvmType) {
Assert.checkNull(sym.params);
sym.params = params.toList();
parameterAnnotations = null;
parameterNameIndices = null;
parameterNameIndicesLvt = null;
parameterNameIndicesMp = null;
parameterAccessFlags = null;
}


// Returns the name for the parameter at position 'index', either using
// names read from the MethodParameters, or by synthesizing a name that
// is not on the 'exclude' list.
private VarSymbol parameter(int index, Type t, MethodSymbol owner, Set<Name> exclude) {
/**
* Creates the parameter at the position {@code mpIndex} in the parameter list of the owning method.
* Flags are optionally read from the MethodParameters attribute.
* Names are optionally read from the MethodParameters attribute. If the constant pool index
* of the name is 0, then the name is optionally read from the LocalVariableTable attribute.
* @param mpIndex the index of the parameter in the MethodParameters attribute
* @param lvtIndex the index of the parameter in the LocalVariableTable attribute
*/
private VarSymbol parameter(int mpIndex, int lvtIndex, Type t, MethodSymbol owner, Set<Name> exclude) {
long flags = PARAMETER;
Name argName;
if (parameterAccessFlags != null && index < parameterAccessFlags.length
&& parameterAccessFlags[index] != 0) {
flags |= parameterAccessFlags[index];
}
if (parameterNameIndices != null && index < parameterNameIndices.length
&& parameterNameIndices[index] != 0) {
argName = optPoolEntry(parameterNameIndices[index], poolReader::getName, names.empty);
if (parameterAccessFlags != null && mpIndex < parameterAccessFlags.length
&& parameterAccessFlags[mpIndex] != 0) {
flags |= parameterAccessFlags[mpIndex];
}
if (parameterNameIndicesMp != null
// if name_index is 0, then we might still get a name from the LocalVariableTable
&& parameterNameIndicesMp[mpIndex] != 0) {
argName = optPoolEntry(parameterNameIndicesMp[mpIndex], poolReader::getName, names.empty);
flags |= NAME_FILLED;
} else if (parameterNameIndicesLvt != null && lvtIndex < parameterNameIndicesLvt.length
&& parameterNameIndicesLvt[lvtIndex] != 0) {
argName = optPoolEntry(parameterNameIndicesLvt[lvtIndex], poolReader::getName, names.empty);
flags |= NAME_FILLED;
} else {
String prefix = "arg";
Expand Down
48 changes: 39 additions & 9 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassWriter.java
View file
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1999, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1999, 2023, 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 @@ -40,7 +40,6 @@
import com.sun.tools.javac.code.*;
import com.sun.tools.javac.code.Attribute.RetentionPolicy;
import com.sun.tools.javac.code.Directive.*;
import com.sun.tools.javac.code.Source.Feature;
import com.sun.tools.javac.code.Symbol.*;
import com.sun.tools.javac.code.Type.*;
import com.sun.tools.javac.code.Types.SignatureGenerator.InvalidSignatureException;
Expand Down Expand Up @@ -383,7 +382,7 @@ int writeMemberAttrs(Symbol sym, boolean isRecordComponent) {
/**
* Write method parameter names attribute.
*/
int writeMethodParametersAttr(MethodSymbol m) {
int writeMethodParametersAttr(MethodSymbol m, boolean writeParamNames) {
MethodType ty = m.externalType(types).asMethodType();
final int allparams = ty.argtypes.size();
if (m.params != null && allparams != 0) {
Expand All @@ -394,23 +393,32 @@ int writeMethodParametersAttr(MethodSymbol m) {
final int flags =
((int) s.flags() & (FINAL | SYNTHETIC | MANDATED)) |
((int) m.flags() & SYNTHETIC);
databuf.appendChar(poolWriter.putName(s.name));
if (writeParamNames)
databuf.appendChar(poolWriter.putName(s.name));
else
databuf.appendChar(0);
databuf.appendChar(flags);
}
// Now write the real parameters
for (VarSymbol s : m.params) {
final int flags =
((int) s.flags() & (FINAL | SYNTHETIC | MANDATED)) |
((int) m.flags() & SYNTHETIC);
databuf.appendChar(poolWriter.putName(s.name));
if (writeParamNames)
databuf.appendChar(poolWriter.putName(s.name));
else
databuf.appendChar(0);
databuf.appendChar(flags);
}
// Now write the captured locals
for (VarSymbol s : m.capturedLocals) {
final int flags =
((int) s.flags() & (FINAL | SYNTHETIC | MANDATED)) |
((int) m.flags() & SYNTHETIC);
databuf.appendChar(poolWriter.putName(s.name));
if (writeParamNames)
databuf.appendChar(poolWriter.putName(s.name));
else
databuf.appendChar(0);
databuf.appendChar(flags);
}
endAttr(attrIndex);
Expand Down Expand Up @@ -1009,9 +1017,12 @@ void writeMethod(MethodSymbol m) {
endAttr(alenIdx);
acount++;
}
if (target.hasMethodParameters() && (options.isSet(PARAMETERS) || m.isConstructor() && (m.flags_field & RECORD) != 0)) {
if (!m.isLambdaMethod()) // Per JDK-8138729, do not emit parameters table for lambda bodies.
acount += writeMethodParametersAttr(m);
if (target.hasMethodParameters()) {
if (!m.isLambdaMethod()) { // Per JDK-8138729, do not emit parameters table for lambda bodies.
boolean requiresParamNames = requiresParamNames(m);
if (requiresParamNames || requiresParamFlags(m))
acount += writeMethodParametersAttr(m, requiresParamNames);
}
}
acount += writeMemberAttrs(m, false);
if (!m.isLambdaMethod())
Expand All @@ -1020,6 +1031,25 @@ void writeMethod(MethodSymbol m) {
endAttrs(acountIdx, acount);
}

private boolean requiresParamNames(MethodSymbol m) {
if (options.isSet(PARAMETERS))
return true;
if (m.isConstructor() && (m.flags_field & RECORD) != 0)
return true;
return false;
}

private boolean requiresParamFlags(MethodSymbol m) {
if (!m.extraParams.isEmpty()) {
return m.extraParams.stream().anyMatch(p -> (p.flags_field & (SYNTHETIC | MANDATED)) != 0);
}
if (m.params != null) {
// parameter is stored in params for Enum#valueOf(name)
return m.params.stream().anyMatch(p -> (p.flags_field & (SYNTHETIC | MANDATED)) != 0);
}
return false;
}

/** Write code attribute of method.
*/
void writeCode(Code code) {
Expand Down
2 changes: 1 addition & 1 deletion src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java
View file
Expand Up @@ -4153,7 +4153,7 @@ protected JCClassDecl recordDeclaration(JCModifiers mods, Comment dc) {
for (JCVariableDecl param : headerFields) {
tmpParams.add(F.at(param)
// we will get flags plus annotations from the record component
.VarDef(F.Modifiers(Flags.PARAMETER | Flags.GENERATED_MEMBER | param.mods.flags & Flags.VARARGS,
.VarDef(F.Modifiers(Flags.PARAMETER | Flags.GENERATED_MEMBER | Flags.MANDATED | param.mods.flags & Flags.VARARGS,
param.mods.annotations),
param.name, param.vartype, null));
}
Expand Down

1 comment on commit b3dbf28

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Issues

Please sign in to comment.