Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
829eeca
always write MethodParameters attribute for methods with mandated par…
SirYwell May 22, 2022
65b1f46
only write param names when needed
SirYwell May 27, 2022
aa80719
name index might be 0
SirYwell May 28, 2022
12d10d2
split name and flag requirements
SirYwell Jun 7, 2022
09bb0c4
adapt change to constant pool in test
SirYwell Jun 7, 2022
cbc3e5d
revert valueOf hack
SirYwell Jun 8, 2022
3cc4c8e
also require flags for synthetic
SirYwell Jun 19, 2022
7c7c193
combine parameter names from LVT and MethodParameters
SirYwell Jun 19, 2022
ba7986f
add javac testcase for implicit parameters
SirYwell Aug 7, 2022
bac6aec
set mandated flag for parameters of compact constructor
SirYwell May 22, 2022
8eb3939
update constant pool indexes in AnnoTest
SirYwell Aug 8, 2022
17df27f
copyright
SirYwell Aug 12, 2022
21c547d
add bug number to test
SirYwell Aug 12, 2022
c9ba84c
remove spaces from empty lines in text block
SirYwell Aug 12, 2022
e453380
add access flag based test
SirYwell Aug 13, 2022
b393150
fix line breaks
SirYwell Aug 13, 2022
55b81f2
add annotation processing test
SirYwell Aug 19, 2022
ebb09f8
cleanup
SirYwell Aug 19, 2022
883a2dc
address comments
SirYwell Aug 23, 2022
3bdc996
Merge remote-tracking branch 'upstream/master' into fix/enforce-metho…
SirYwell Jan 8, 2023
4378093
Update copyright year
SirYwell Jan 9, 2023
cc5a049
Merge branch 'master' into methodparameters-flags
liach Mar 22, 2023
6183083
Updated RequiredMethodParameterFlagTest, checks against Local classes…
liach Mar 22, 2023
9879b82
Parity for the parameter flag tests in javac and reflection
liach Mar 22, 2023
c42e9f3
Update tests
liach Mar 23, 2023
5ac3a21
Merge pull request #1 from liachmodded/methodparameters-flags
SirYwell Mar 23, 2023
400e1ec
Use ternary operator
SirYwell Apr 29, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,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 @@ -220,18 +226,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 @@ -896,7 +890,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 @@ -915,14 +909,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 @@ -1073,19 +1066,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 @@ -2343,13 +2334,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 @@ -2364,46 +2353,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 @@ -2412,7 +2402,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 @@ -2421,24 +2412,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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1999, 2022, 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 @@ -4132,7 +4132,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,
Copy link
Member

Choose a reason for hiding this comment

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

Should the use of MANDATED be conditional on the target class file version?

Copy link
Member

Choose a reason for hiding this comment

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

I advise also writing a core reflection test that uses access flags (java.lang.reflect.AccessFlag).

Copy link
Member Author

Choose a reason for hiding this comment

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

Should the use of MANDATED be conditional on the target class file version?

I don't think this is needed. The JLS mentions the parameters of canonical record constructors in Java 14 already. Previous versions don't have records and shouldn't enter that method at all.

I advise also writing a core reflection test that uses access flags (java.lang.reflect.AccessFlag).

Will do. I assume the jdk/java/lang/reflect/AccessFlag folder is a good place for that?

Copy link
Member

Choose a reason for hiding this comment

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

Should the use of MANDATED be conditional on the target class file version?

I don't think this is needed. The JLS mentions the parameters of canonical record constructors in Java 14 already. Previous versions don't have records and shouldn't enter that method at all.

Agreed; I double-checked and the ACC_MANDATED flag was defined in that location since before JDK 14's format.

I advise also writing a core reflection test that uses access flags (java.lang.reflect.AccessFlag).

Will do. I assume the jdk/java/lang/reflect/AccessFlag folder is a good place for that?

Yes; that an appropriate home for such tests.

param.mods.annotations),
param.name, param.vartype, null));
}
Expand Down
Loading