Java frontend: get qualified generic types of static fields #4137
Conversation
smowton
requested review from
cesaro,
cristina-david,
NathanJPhillips,
peterschrammel,
pkesseli and
thk123
as code owners
There was a problem hiding this comment.
✔️
Passed Diffblue compatibility checks (cbmc commit: 81b3148).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/100277615
| const symbol_exprt symbol_expr( | ||
| get_static_field(arg0.get_string(ID_class), field_name), arg0.type()); | ||
|
|
||
| INVARIANT( |
There was a problem hiding this comment.
❓ This looks like useful context for this symbol not existing that goes away by folding it into lookup_ref - do you disagree?
There was a problem hiding this comment.
Seems pointless to have an invariant that can never fail, and I think a symbol table lookup returning a reference is widely recognised as find-or-die?
There was a problem hiding this comment.
My point is "getstatic symbol should have been created before method conversion" is more useful than simply a symbol not existing. Like without that message you might (wrongly) conclude that this conversion should actually be adding to the symbol table
I'm suggesting the invariant goes about the call to lookup_ref in case that wasn't clear. Like,
There was a problem hiding this comment.
Added a clarifying comment
Before it would unintentionally erase SomeClass.field from A<B> to just the unqualified A type.
|
@thk123 test added |
smowton
force-pushed
the
smowton/fix/java-static-field-generic-types
branch
from
81b3148 to
a208cce
Compare
| const symbol_exprt symbol_expr( | ||
| get_static_field(arg0.get_string(ID_class), field_name), arg0.type()); | ||
|
|
||
| INVARIANT( |
There was a problem hiding this comment.
My point is "getstatic symbol should have been created before method conversion" is more useful than simply a symbol not existing. Like without that message you might (wrongly) conclude that this conversion should actually be adding to the symbol table
I'm suggesting the invariant goes about the call to lookup_ref in case that wasn't clear. Like,
There was a problem hiding this comment.
✔️
Passed Diffblue compatibility checks (cbmc commit: a208cce).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/100610260
Avoid the suggestion that `get_static_field` might be expected to create them -- it doesn't, java_bytecode_convert_class does and we should be able to safely assume they exist during convert_method.
smowton
force-pushed
the
smowton/fix/java-static-field-generic-types
branch
from
a19d819 to
35213c4
Compare
There was a problem hiding this comment.
✔️
Passed Diffblue compatibility checks (cbmc commit: 35213c4).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/100644186
Before it would unintentionally erase SomeClass.field from
A<B>to just the unqualified A type.Needs tests adding, of course.