Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8240658: Code completion not working for lambdas in method invocations that require type inference #50

Closed
wants to merge 4 commits into from
+699 −36
Closed

8240658: Code completion not working for lambdas in method invocations that require type inference #50

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7f4e903
8240658: Code completion not working for lambdas in method invocation…
lahodaj Sep 2, 2020
a2faddb
Merge branch 'master' into JDK-8240658
lahodaj Sep 10, 2020
32845dd
Improving behavior for vararg method invocations.
lahodaj Sep 10, 2020
988f85b
Moving the TestGetTypeMirrorReference test into a separate folder.
lahodaj Sep 11, 2020
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
14 changes: 10 additions & 4 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/comp/AttrRecover.java
View file
Original file line number Diff line number Diff line change
@@ -29,6 +29,7 @@
import com.sun.tools.javac.code.Symbol.TypeSymbol;
import com.sun.tools.javac.code.Symtab;
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.code.Type.ArrayType;
import com.sun.tools.javac.code.Type.ErrorType;
import com.sun.tools.javac.code.TypeTag;
import com.sun.tools.javac.code.Types;
@@ -98,23 +99,26 @@ public void doRecovery() {
boolean repaired = false;
RECOVER: if (todo.env.tree.hasTag(Tag.APPLY)) {
JCMethodInvocation mit = (JCMethodInvocation) todo.env.tree;
if ((todo.candSym.flags() & Flags.VARARGS) == 0 &&
boolean vararg = (todo.candSym.flags() & Flags.VARARGS) != 0;
if (!vararg &&
mit.args.length() > todo.candSym.type.getParameterTypes().length()) {
break RECOVER; //too many actual parameters, skip
Copy link
Contributor

Choose a reason for hiding this comment

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

question: why not covering varargs too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note this is a case where the invoked method is not a varargs method, and there are too many actual parameters. But it is true the varags handling could be improved, which I tried to do in a recent patch (32845dd).

Copy link
Contributor

@vicente-romero-oracle vicente-romero-oracle Sep 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I like the patch. I think it is great. My only comment would be about test: TestGetTypeMirrorReference.java which does a golden file like comparison but I understand that there are not many options. I would suggest though to place this test along with file: TestGetTypeMirrorReferenceData.java in a separate folder and add some doc to the test so that it would be easier to modify it in the future

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!

I've tried to improve the text (moved to a separate directory, added comment) in 988f85b.

}
List<JCExpression> args = mit.args;
List<Type> formals = todo.candSym.type.getParameterTypes();
while (args.nonEmpty() && formals.nonEmpty()) {
JCExpression arg = args.head;
Type formal = formals.tail.nonEmpty() || !vararg
? formals.head : ((ArrayType) formals.head).elemtype;
if (arg.hasTag(JCTree.Tag.LAMBDA)) {
final JCTree.JCLambda lambda = (JCLambda) arg;
if (lambda.paramKind == JCLambda.ParameterKind.IMPLICIT) {
for (JCVariableDecl var : lambda.params) {
var.vartype = null; //reset type
}
}
if (types.isFunctionalInterface(formals.head)) {
Type functionalType = types.findDescriptorType(formals.head);
if (types.isFunctionalInterface(formal)) {
Type functionalType = types.findDescriptorType(formal);
boolean voidCompatible = functionalType.getReturnType().hasTag(TypeTag.VOID);
lambda.body = new TreeTranslator() {
@Override
@@ -163,7 +167,9 @@ public void visitClassDef(JCClassDecl tree) {
repaired = true;
}
args = args.tail;
formals = formals.tail;
if (formals.tail.nonEmpty() || !vararg) {
formals = formals.tail;
}
}
List<JCExpression> prevArgs = mit.args;
while (formals.nonEmpty()) {
17 changes: 17 additions & 0 deletions test/langtools/tools/javac/api/TestGetTypeMirrorReference.java
View file
Original file line number Diff line number Diff line change
@@ -88,6 +88,16 @@ private static void test() {
consumeVarArgs(0, (c7)->{
Object o = c7;
}, 1, 2, 3, 4);
convertVarArgs2(0, (c8)->{
Object o = c8;
}, (c8)->{
Object o = c8;
});
consumeVarArgs2(0, (c9)->{
Object o = c9;
}, (c9)->{
Object o = c9;
});
}

public <T, R>R convert(T t, Function<T, R> f, int i) {
@@ -104,6 +114,13 @@ public <T, R>R convertVarArgs(T t, Function<T, R> c, int... i) {
public <T>void consumeVarArgs(T t, Consumer<T> c, int... i) {
}

public <T, R>R convertVarArgs2(T t, Function<T, R>... c) {
return null;
}

public <T>void consumeVarArgs2(T t, Consumer<T>... c) {
}

public static class Test<T> {

public Test() {
7 changes: 7 additions & 0 deletions test/langtools/tools/javac/api/TestGetTypeMirrorReferenceData.java
View file
Original file line number Diff line number Diff line change
@@ -33,6 +33,8 @@ private static void test() {
consume(0, c5 -> {Object o = c5/*getTypeMirror:DECLARED:java.lang.Integer*/;});
Copy link

Choose a reason for hiding this comment

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

Jackpot:
warning: Variable o is never read

convertVarArgs(0, c6 -> {Object o = c6/*getTypeMirror:DECLARED:java.lang.Integer*/;}, 1, 2, 3, 4);
Copy link

Choose a reason for hiding this comment

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

Jackpot:
warning: Variable o is never read

consumeVarArgs(0, c7 -> {Object o = c7/*getTypeMirror:DECLARED:java.lang.Integer*/;}, 1, 2, 3, 4);
Copy link

Choose a reason for hiding this comment

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

Jackpot:
warning: Variable o is never read

convertVarArgs2(0, c8 -> {Object o = c8/*getTypeMirror:DECLARED:java.lang.Integer*/;}, c8 -> {Object o = c8/*getTypeMirror:DECLARED:java.lang.Integer*/;});
consumeVarArgs2(0, c9 -> {Object o = c9/*getTypeMirror:DECLARED:java.lang.Integer*/;}, c9 -> {Object o = c9/*getTypeMirror:DECLARED:java.lang.Integer*/;});
}
public <T, R> R convert(T t, Function<T, R> f, int i) {
return null;
@@ -44,6 +46,11 @@ public <T, R> R convertVarArgs(T t, Function<T, R> c, int... i) {
}
public <T> void consumeVarArgs(T t, Consumer<T> c, int... i) {
}
public <T, R> R convertVarArgs2(T t, Function<T, R>... c) {
return null;
}
public <T> void consumeVarArgs2(T t, Consumer<T>... c) {
}
public static class Test<T> {
public static <T> Test<T> of(T t) {
return new Test<>();