Skip to content

Java frontend: add support for lambdas that require generating boxing, unboxing, widening casts and upcasts #5359

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.

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

Merged
merged 6 commits into from
May 28, 2020

Conversation

smowton
Copy link
Contributor

@smowton smowton commented May 27, 2020

The Java lambda metafactory, per https://docs.oracle.com/javase/8/docs/api/java/lang/invoke/LambdaMetafactory.html, can require adapting the type of the referenced method to that of the target functional interface by introducing boxing, unboxing, primitive type widening and pointer upcasting conversions. This adds tests for a wide variety of such cases and implements the required suppport.

smowton added 3 commits May 27, 2020 16:33
Turns out the usual lambda metafactory can apply boxing/unboxing, so you might see a situation like

int myMethod(int i) { ... }

interface MyLambdaType { Integer f(Integer i); }

MyLambdaType mlt = SomeType::myMethod;

And contrary to my prior expectations, the compiler doesn't generate a method stub to do the type conversion,
it just directly requests an invokestatic lambda pointed at myMethod, regardless of the int vs. Integer clash.
The lambda metafactory (which re-implement in JBMC) is then responsible for adding the necessary conversions.
This broadens our support for lambdas with implicit boxing/unboxing conversions
to include those that use a method-reference to a method with type Object -> x
to satisfy a functional interface of type primitive -> x (requiring boxing the
parameter and then upcasting to Object), or a method with type x -> primitive
satisfying an interface of type x -> Object (requiring the same conversion on
the return value).
…type widening

For example, an int -> int method is a valid instantiation of a functional interface
that expects a byte -> int, or one that expects an int -> long. In both cases it's down
to the metafactory / our code that emulates its behaviour to add the widening cast.
smowton added 2 commits May 27, 2020 17:37
…from a symbol-table id

These users start with an irep_idt, so it's cheaper to look up by id than to create a derived
string and then look up by that.
@smowton smowton force-pushed the smowton/feature/lambda-boxing branch from 829d5e9 to aaa5d28 Compare May 27, 2020 16:37
@smowton smowton force-pushed the smowton/feature/lambda-boxing branch from aaa5d28 to 70f9729 Compare May 27, 2020 21:07
@codecov
Copy link

codecov bot commented May 27, 2020

Codecov Report

Merging #5359 into develop will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #5359      +/-   ##
===========================================
+ Coverage    68.17%   68.19%   +0.01%     
===========================================
  Files         1170     1171       +1     
  Lines        96540    96590      +50     
===========================================
+ Hits         65819    65866      +47     
- Misses       30721    30724       +3     
Flag Coverage Δ
#cproversmt2 42.47% <ø> (ø)
#regression 65.39% <100.00%> (+0.01%) ⬆️
#unit 31.82% <54.38%> (+<0.01%) ⬆️
Impacted Files Coverage Δ
jbmc/src/java_bytecode/assignments_from_json.cpp 97.99% <100.00%> (-0.01%) ⬇️
jbmc/src/java_bytecode/java_utils.cpp 91.07% <100.00%> (-0.60%) ⬇️
jbmc/src/java_bytecode/java_utils.h 100.00% <100.00%> (ø)
jbmc/src/java_bytecode/lambda_synthesis.cpp 97.84% <100.00%> (+0.27%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d58ebd8...70f9729. Read the comment docs.

Copy link
Contributor

@thk123 thk123 left a comment

Choose a reason for hiding this comment

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

⛏️ Commit messages frequently exceed 72 (80?) chars causing github to wrap them, otherwise looks perfect to me

}

const java_primitive_type_infot *
get_java_primitive_type_info(const typet &maybe_primitive_type)
Copy link
Contributor

Choose a reason for hiding this comment

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

⛏️ optionalt rather than a pointer

Comment on lines 631 to 638
if(original_is_pointer == required_is_pointer)
{
if(original_is_pointer && original_type != required_type)
return typecast_exprt{expr, required_type};
else
return expr;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

What's this case for?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the answer is byte to int and Derived -> Base?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gains a comment in a later commit (it's for pointer or integer type clashes without boxing, where plain casting always does the trick (forcing pointers and widening integers, so long as only valid Java casts are fed in in the first place)

@smowton smowton merged commit 60ba633 into diffblue:develop May 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants