Jumpstart java8 by juliandolby · Pull Request #1507 · wala/WALA

juliandolby · 2025-05-14T01:07:07Z

We have been working on Java 8 supports in the source front end, and this is work on lambda expressions, complete with two tests.

github-actions · 2025-05-14T01:30:45Z

Test Results

713 files + 6 713 suites +6 4h 54m 14s ⏱️ + 33m 21s
773 tests + 8 754 ✅ + 8 19 💤 ±0 0 ❌ ±0
4 522 runs +48 4 406 ✅ +48 116 💤 ±0 0 ❌ ±0

Results for commit 57eaf3d. ± Comparison against base commit 9665ec2.

♻️ This comment has been updated with latest results.

msridhar

This is awesome @juliandolby! It will take me a little while to review this; I hope that's ok. A couple of initial slightly nit-picky comments are below.

High-level questions:

Is the lambda support here intended to be complete? Or are there cases with lambdas that are still not handled?
Are method references handled at all, or is that a separate effort?

msridhar · 2025-05-14T01:59:42Z

cast/js/src/testFixtures/java/com/ibm/wala/cast/js/test/TestSimpleCallGraphShape.java

    CAstCallGraphUtil.dumpCG(B.getCFAContextInterpreter(), B.getPointerAnalysis(), CG);
  }

+  /*


Can we put this change in a separate PR if we really want it? Doesn't seem to relate to Java 8. On Gradle if you want to skip these slow tests you can add the flag -PexcludeSlowTests

i didn't know about that option... in that case, we likely don't need it.

Ok, sounds good can we revert to the version in the master branch then?

msridhar · 2025-05-14T02:08:25Z

cast/java/ecj/.settings/org.eclipse.jdt.core.prefs

 org.eclipse.jdt.core.compiler.codegen.inlineJsrBytecode=enabled
 org.eclipse.jdt.core.compiler.codegen.methodParameters=do not generate
-org.eclipse.jdt.core.compiler.codegen.targetPlatform=11
+org.eclipse.jdt.core.compiler.codegen.targetPlatform=17


I'm not sure if all these changes to the Eclipse config files were intended to be part of this PR. But specifically, we might want to revert these changes that target JDK 17 instead of JDK 11, as WALA still supports running on JDK 11. Otherwise, an Eclipse developer might erroneously not see errors when using Java 17 constructs (though the Gradle build should still fail).

Ah, that is an oversight due to the fact that much work here requires Java 17. Most likely my Eclipse updated that without my asking. Sorry about that.

No worries, could you revert the changes to all the Eclipse config files then for this PR?

juliandolby · 2025-05-14T12:58:03Z

This is awesome @juliandolby! It will take me a little while to review this; I hope that's ok. A couple of initial
slightly nit-picky comments are below.

High-level questions:

Is the lambda support here intended to be complete? Or are there cases with lambdas that are still not handled?

It is intended to be complete, but whether it really is complete and bug free is another question. We do have two tests of basic functionality, and the implementation relies on the same machinery that supports inner classes. However, more rigorous testing would be a good thing.

Are method references handled at all, or is that a separate effort?

This request is focused on lambda expressions. But aren't method references and their friends mostly a library thing anyway? The existing support for them in bytecode might be sufficient...

juliandolby · 2025-05-16T01:23:36Z

This is awesome @juliandolby! It will take me a little while to review this; I hope that's ok. A couple of initial
slightly nit-picky comments are below.
High-level questions:

Is the lambda support here intended to be complete? Or are there cases with lambdas that are still not handled?

It is intended to be complete, but whether it really is complete and bug free is another question. We do have two tests of basic functionality, and the implementation relies on the same machinery that supports inner classes. However, more rigorous testing would be a good thing.

Are method references handled at all, or is that a separate effort?

This request is focused on lambda expressions. But aren't method references and their friends mostly a library thing anyway? The existing support for them in bytecode might be sufficient...

And now we have a third test, and needed fixes...

msridhar · 2025-05-16T01:26:24Z

This request is focused on lambda expressions. But aren't method references and their friends mostly a library thing anyway? The existing support for them in bytecode might be sufficient...

A method reference is something like A::foo and it will often occur in source code. But, completely fine to handle that in a separate PR. It might already work; under the hood it desugars to a lambda. But worth testing, eventually.

juliandolby · 2025-05-16T01:30:11Z

This request is focused on lambda expressions. But aren't method references and their friends mostly a library thing anyway? The existing support for them in bytecode might be sufficient...

A method reference is something like A::foo and it will often occur in source code. But, completely fine to handle that in a separate PR. It might already work; under the hood it desugars to a lambda. But worth testing, eventually.

Ah yes. We have not (yet) attempted to handle those.

codecov · 2025-05-18T21:31:25Z

Codecov Report

❌ Patch coverage is 88.14815% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.20%. Comparing base (9665ec2) to head (57eaf3d).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
...st/java/translator/jdt/JDTJava2CAstTranslator.java	86.86%	4 Missing and 9 partials ⚠️
...la/cast/java/translator/jdt/JDTTypeDictionary.java	70.00%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #1507      +/-   ##
============================================
+ Coverage     50.15%   50.20%   +0.05%     
- Complexity    12632    12652      +20     
============================================
  Files          1365     1365              
  Lines         85113    85204      +91     
  Branches      14717    14732      +15     
============================================
+ Hits          42691    42780      +89     
+ Misses        37630    37628       -2     
- Partials       4792     4796       +4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

msridhar

Hoping we can clean up a couple high-level things and then I can glance through the code more easily

msridhar · 2025-05-18T21:39:25Z

cast/java/ecj/.settings/org.eclipse.jdt.core.prefs

 org.eclipse.jdt.core.compiler.codegen.inlineJsrBytecode=enabled
 org.eclipse.jdt.core.compiler.codegen.methodParameters=do not generate
-org.eclipse.jdt.core.compiler.codegen.targetPlatform=11
+org.eclipse.jdt.core.compiler.codegen.targetPlatform=17


No worries, could you revert the changes to all the Eclipse config files then for this PR?

msridhar · 2025-05-18T21:45:19Z

cast/js/src/testFixtures/java/com/ibm/wala/cast/js/test/TestSimpleCallGraphShape.java

    CAstCallGraphUtil.dumpCG(B.getCFAContextInterpreter(), B.getPointerAnalysis(), CG);
  }

+  /*


Ok, sounds good can we revert to the version in the master branch then?

msridhar · 2025-05-18T21:46:26Z

java/ecj/src/main/java/com/ibm/wala/cast/java/translator/jdt/JDT2CAstUtils.java

@@ -0,0 +1,265 @@
+/*


It looks like a top-level java directory was added erroneously in this PR. Assuming that's correct could we delete it?

It seems this comment has still not been addressed? I still see an unneeded top-level java directory

msridhar · 2025-05-18T21:47:03Z

ide/tests/build.properties

 output.. = bin/test
-bin.includes = META-INF/,\
-               .,\
+bin.includes = .,\


Is this change important for this PR? If not could we undo it?

All of your changes sound good to me. I am not sure how the top-level java dir got there, rather than cast/java, but it was not deliberate.

I think the change to ide/tests/build.properties made a warning go away in Eclipse, which I still use, but it is not vital.

reverting the .prefs files and TestSimpleCallGraphShape is all fine.

Thanks, @juliandolby. Please let me know once you've had time to make all these changes (no hurry on my end), and then after that I'll take another look. I expect we'll be able to land the PR soon after.

If we could also undo changes to build.properties files that'd be great; we can put them in a separate PR maybe? If it's too painful to undo it's ok.

reverted that file too

TestSimpleCallGraphShape, java17

juliandolby · 2025-07-09T00:02:36Z

Took a while to get back to this, but we finally did. @patricklapgar and I have attempted to address the comments on this pull request: we have put the Java version back to 11 and undone our change on the slow JS test. We have also merged the latest WALA. More comments, suggestions and requests are welcome.

msridhar · 2025-08-25T15:33:30Z

@juliandolby let me know when this is ready for another look 🙂

juliandolby · 2025-08-25T16:55:26Z

@juliandolby let me know when this is ready for another look 🙂

I think it is ready now...

…-java8

msridhar

LGTM now! So great to see source frontend lambda support finally landing!

msridhar · 2025-08-25T20:37:52Z

@juliandolby would it be helpful to have a WALA release that includes this support?

juliandolby added 5 commits April 16, 2025 14:00

first cut with passing test

a35382f

changes for java 8

1985f88

work on lambda

d108e4c

Merge branch 'master' of https://github.com/wala/WALA

02779d9

minor tweaks

994392b

juliandolby added the enhancement label May 14, 2025

juliandolby linked an issue May 14, 2025 that may be closed by this pull request

JDTJava2CAstTranslator.visitNode Unhandled JDT node type org.eclipse.jdt.core.dom.LambdaExpression #1479

Closed

juliandolby requested review from liblit and msridhar May 14, 2025 01:10

msridhar reviewed May 14, 2025

View reviewed changes

juliandolby added 2 commits May 15, 2025 17:29

fixes to make lambdas work with lexical scoping

0fdcc9c

test for lambdas and lexical scoping

9633da8

Merge branch 'master' into jumpstart-java8

ee12b9b

juliandolby and others added 3 commits May 15, 2025 21:36

fixes with spotless

2d3953c

Merge branch 'master' into jumpstart-java8

4356086

Merge branch 'master' into jumpstart-java8

5021485

msridhar requested changes May 18, 2025

View reviewed changes

Merge branch 'master' into jumpstart-java8

162ba34

liblit mentioned this pull request Jul 8, 2025

Resolving PR comments for PR #1507 #1551

Merged

Patrick Apgar and others added 4 commits July 8, 2025 18:52

Resolving PR comments:

653ba87

TestSimpleCallGraphShape, java17

Resolved undefined method error

606128a

merge deletion

5a009f7

move lambda tests to new expected place

6dd1e27

juliandolby added 7 commits August 21, 2025 22:31

unused

ee20135

spotless

da05455

explain why this class exists

6564b9f

fix added type-o

00d9d6f

spotless

e05140b

describe checkCaller

d34f9db

spotless

88746a4

juliandolby added 3 commits August 25, 2025 11:42

remove hacks special casing Function

13b247d

spotless

4fd0cd6

Merge branch 'master' into jumpstart-java8

72808ac

juliandolby and others added 8 commits August 25, 2025 13:05

remove return for void

85828c5

Merge branch 'jumpstart-java8' of github.com:wala/WALA into jumpstart…

14d9747

…-java8

use throw

eafe459

spotless

4ab2769

Merge branch 'master' into jumpstart-java8

85ea7a9

make safe for multithreading

8b8ccf9

rename method

f1193dd

Merge branch 'master' into jumpstart-java8

60cd371

msridhar previously approved these changes Aug 25, 2025

View reviewed changes

msridhar enabled auto-merge August 25, 2025 19:24

spotless

57eaf3d

msridhar dismissed their stale review via 57eaf3d August 25, 2025 19:29

msridhar approved these changes Aug 25, 2025

View reviewed changes

msridhar added this pull request to the merge queue Aug 25, 2025

Merged via the queue into master with commit 69e3035 Aug 25, 2025
20 checks passed

msridhar deleted the jumpstart-java8 branch August 25, 2025 20:30

Conversation

juliandolby commented May 14, 2025

Uh oh!

github-actions bot commented May 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

msridhar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

juliandolby May 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

juliandolby commented May 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

juliandolby commented May 16, 2025

Uh oh!

msridhar commented May 16, 2025

Uh oh!

juliandolby commented May 16, 2025

Uh oh!

codecov bot commented May 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

msridhar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

msridhar Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

juliandolby commented Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

msridhar commented Aug 25, 2025

Uh oh!

juliandolby commented Aug 25, 2025

Uh oh!

msridhar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

msridhar commented Aug 25, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

github-actions bot commented May 14, 2025 •

edited

Loading

juliandolby May 14, 2025 •

edited

Loading

juliandolby commented May 14, 2025 •

edited

Loading

codecov bot commented May 18, 2025 •

edited

Loading

msridhar Jul 11, 2025 •

edited

Loading

juliandolby commented Jul 9, 2025 •

edited

Loading