-
Notifications
You must be signed in to change notification settings - Fork 275
Replace use of deprecated nil_typet in java_type_from_string [blocks: #3800] #3929
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 |
---|---|---|
|
@@ -259,21 +259,26 @@ java_method_typet member_type_lazy( | |
|
||
messaget message(message_handler); | ||
|
||
typet member_type_from_descriptor=java_type_from_string(descriptor); | ||
INVARIANT(member_type_from_descriptor.id()==ID_code, "Must be code type"); | ||
auto member_type_from_descriptor = java_type_from_string(descriptor); | ||
INVARIANT( | ||
member_type_from_descriptor.has_value() && | ||
member_type_from_descriptor->id() == ID_code, | ||
"Must be code type"); | ||
if(signature.has_value()) | ||
{ | ||
try | ||
{ | ||
typet member_type_from_signature=java_type_from_string( | ||
signature.value(), | ||
class_name); | ||
INVARIANT(member_type_from_signature.id()==ID_code, "Must be code type"); | ||
auto member_type_from_signature = | ||
java_type_from_string(signature.value(), class_name); | ||
INVARIANT( | ||
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 actually delete this, we're about to check both those things by deref'ing and casting |
||
member_type_from_signature.has_value() && | ||
member_type_from_signature->id() == ID_code, | ||
"Must be code type"); | ||
if( | ||
to_java_method_type(member_type_from_signature).parameters().size() == | ||
to_java_method_type(member_type_from_descriptor).parameters().size()) | ||
to_java_method_type(*member_type_from_signature).parameters().size() == | ||
to_java_method_type(*member_type_from_descriptor).parameters().size()) | ||
{ | ||
return to_java_method_type(member_type_from_signature); | ||
return to_java_method_type(*member_type_from_signature); | ||
} | ||
else | ||
{ | ||
|
@@ -294,7 +299,7 @@ java_method_typet member_type_lazy( | |
<< message.eom; | ||
} | ||
} | ||
return to_java_method_type(member_type_from_descriptor); | ||
return to_java_method_type(*member_type_from_descriptor); | ||
} | ||
|
||
/// Retrieves the symbol of the lambda method associated with the given | ||
|
@@ -460,24 +465,10 @@ void java_bytecode_convert_methodt::convert( | |
if(!is_parameter(v)) | ||
continue; | ||
|
||
// Construct a fully qualified name for the parameter v, | ||
// e.g. my.package.ClassName.myMethodName:(II)I::anIntParam, and then a | ||
// symbol_exprt with the parameter and its type | ||
typet t; | ||
if(v.signature.has_value()) | ||
{ | ||
t=java_type_from_string_with_exception( | ||
v.descriptor, | ||
v.signature, | ||
id2string(class_symbol.name)); | ||
} | ||
else | ||
t=java_type_from_string(v.descriptor); | ||
|
||
std::ostringstream id_oss; | ||
id_oss << method_id << "::" << v.name; | ||
irep_idt identifier(id_oss.str()); | ||
symbol_exprt result(identifier, t); | ||
symbol_exprt result = symbol_exprt::typeless(identifier); | ||
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. ❓ This change looks a little suspicious? Previously the type of the symbol was derived from the signature or descriptor, now is a typeless identifier? 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 type was sometimes 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. In that case could I suggest using an 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. That would necessitate some refactoring of this code as the expression is also used to communicate |
||
result.set(ID_C_base_name, v.name); | ||
|
||
// Create a new variablet in the variables vector; in fact this entry will | ||
|
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.
Are we sure this has a value? (and other places)
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.
As above: no idea, but it was previously used unchecked. There are a couple of places that failed this while running the regression tests, and those I have now guarded appropriately. I still think that this makes
optionalt<typet>
much better as at least you'd have a failing assertion instead of silently passing around anil_typet
that some later stage won't know how to deal with.