-
Notifications
You must be signed in to change notification settings - Fork 274
Store the final status of fields in componentt
#2976
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
|
||
class ClassWithFields { | ||
final boolean final1 = true; | ||
final Boolean final2 = false; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 I'd have probably picked different accessibility and/or different ways of setting them (i.e. in the constructor). You might also like to add tests for fields that hide inherited fields |
||
final Object final3 = null; | ||
|
||
boolean nonFinal1 = true; | ||
Boolean nonFinal2 = false; | ||
Object nonFinal3 = null; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
/*******************************************************************\ | ||
|
||
Module: Unit tests for converting fields | ||
|
||
Author: Diffblue Ltd. | ||
|
||
\*******************************************************************/ | ||
|
||
#include <java-testing-utils/load_java_class.h> | ||
#include <java_bytecode/java_bytecode_convert_class.h> | ||
#include <java_bytecode/java_bytecode_parse_tree.h> | ||
#include <java_bytecode/java_types.h> | ||
#include <testing-utils/catch.hpp> | ||
|
||
SCENARIO( | ||
"java_bytecode_parse_field", | ||
"[core][java_bytecode][java_bytecode_parser]") | ||
{ | ||
GIVEN("Some class with final and non final fields") | ||
{ | ||
const symbol_tablet &symbol_table = load_java_class( | ||
"ClassWithFields", "./java_bytecode/java_bytecode_parser"); | ||
|
||
WHEN("Parsing the class file structure") | ||
{ | ||
THEN("The the final status of the classes fields should be correct.") | ||
{ | ||
const symbolt &class_symbol = | ||
symbol_table.lookup_ref("java::ClassWithFields"); | ||
const java_class_typet &java_class = | ||
to_java_class_type(class_symbol.type); | ||
REQUIRE(java_class.get_component("final1").get_is_final()); | ||
REQUIRE(java_class.get_component("final2").get_is_final()); | ||
REQUIRE(java_class.get_component("final3").get_is_final()); | ||
REQUIRE(!java_class.get_component("nonFinal1").get_is_final()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ⛏️ Prefer |
||
REQUIRE(!java_class.get_component("nonFinal1").get_is_final()); | ||
REQUIRE(!java_class.get_component("nonFinal1").get_is_final()); | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -189,6 +189,16 @@ class struct_union_typet:public typet | |
{ | ||
return set(ID_C_is_padding, is_padding); | ||
} | ||
|
||
bool get_is_final() const | ||
{ | ||
return get_bool(ID_final); | ||
} | ||
|
||
void set_is_final(const bool is_final) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The name makes me cry. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So ... what would you recommend instead? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FYI we already use |
||
{ | ||
set(ID_final, is_final); | ||
} | ||
}; | ||
|
||
typedef std::vector<componentt> componentst; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 If the class is opaque the
is_final
flag should be set to true.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After discussion with @thomasspriggs, we worked out that dealing with fields of opaque classes may not be as straightforward than I thought. So my comment can be ignored.