Skip to content
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

[Solidity] Tuple Support #1647

Merged
merged 4 commits into from
May 28, 2024
Merged

[Solidity] Tuple Support #1647

merged 4 commits into from
May 28, 2024

Conversation

JacobYiu
Copy link

@JacobYiu JacobYiu commented Feb 6, 2024

No description provided.

// e.g. "sol:@C@BASE@tuple#14"
std::string label = std::to_string(struct_def["id"].get<int>());
name = "tuple#" + label;
id = "sol:@C@" + current_contractName + "@" + name;
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe add an assertion assert(!current_contractName.empty());


//Components consist of our variables used in our tuple
nlohmann::json ast_nodes;
ast_nodes = struct_def["components"];
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe check assert(struct_def.contains("components"));

// }

for (nlohmann::json::iterator itr = ast_nodes.begin(); itr != ast_nodes.end();
++itr)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel like you should use get_expr() to parse each component. See CallExprClass => structConstructorCall:

// popluate components
    for (unsigned int i = 0; i < inits.operands().size() && i < args.size();
         i++)
    {
      exprt init;
      if (get_expr(args.at(i), members.at(i)["typeDescriptions"], init))
        return true;

      const struct_union_typet::componentt *c =
        &to_struct_type(t).components().at(i);
      typet elem_type = c->type();

      solidity_gen_typecast(ns, init, elem_type);
      inits.operands().at(i) = init;
    }

Copy link
Author

Choose a reason for hiding this comment

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

I think this is more suited towards RHS, since its calling a previous initialized struct and populating the values. But this is definitely useful for my current implementation, which is LHS. Thank you for the help!

@@ -2602,7 +2830,7 @@ bool solidity_convertert::get_func_decl_ref(
exprt &new_expr)
{
// Function to configure new_expr that has a +ve referenced id, referring to a function declaration
// This allow to get func symbol before we add it to the symbol table
// This allow to get func syfmbol before we add it to the symbol table
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo

return output;
}

std::vector<nlohmann::json> solidity_convertert::make_struct_elementary_types(
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you can obtain the type info in the typeDescriptions node in each component, right?

You can do something like:

   if (get_expr(args.at(i), members.at(i)["typeDescriptions"], init))
        return true;

Copy link
Author

Choose a reason for hiding this comment

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

I have yet to delete this function in case I may use it in the future for an unknown reason. I will delete it shortly

std::string label = std::to_string(expr["id"].get<int>());
std::string name, id;
name = "tuple#" + label;
id = "sol:@C@" + current_contractName + "@" + name;
Copy link
Collaborator

Choose a reason for hiding this comment

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

same as above

// e.g. (uint age, uint money) = (....);
if(context.find_symbol(id) != nullptr)
{
std::cout << "Could not find symbol" << std::endl;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe just log_error

@@ -40,7 +41,7 @@ solidity_convertert::solidity_convertert(

bool solidity_convertert::convert()
{
// This function consists of two parts:
// This function consists of two output:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we keep the word "parts"

@ChenfengWei0
Copy link
Collaborator

Overall, it looks good. Just some small comments:

  1. As said, for each tuple comment, we can obtain the type info from typeDescriptions nodes. However, I am also okay with the current implementation. You can stick to the make_struct_elementary_types if you are unfamiliar with the get_expr solution I mentioned.
  2. tupleQueueIds. Do you know if it will work when handling interaction between contracts e.g. inheritance? Will there be an ID conflict?
contract Base
{
  uint x;
  bool y;
  constructor() { (x,y) = (1,true); }
}

contract Derived is Base
{
  function test() public 
  {
    assert(y == true);
  }
}

Copy link
Contributor

@lucasccordeiro lucasccordeiro left a comment

Choose a reason for hiding this comment

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

I have just minor comments.

src/solidity-frontend/solidity_convert.cpp Outdated Show resolved Hide resolved
src/solidity-frontend/solidity_convert.cpp Outdated Show resolved Hide resolved
src/solidity-frontend/solidity_convert.cpp Outdated Show resolved Hide resolved
src/solidity-frontend/solidity_convert.cpp Outdated Show resolved Hide resolved
src/solidity-frontend/solidity_convert.cpp Outdated Show resolved Hide resolved
src/solidity-frontend/solidity_convert.cpp Show resolved Hide resolved
@ChenfengWei0
Copy link
Collaborator

Changed as suggested. Thanks for the advice, @lucasccordeiro !

@lucasccordeiro lucasccordeiro merged commit 09653b2 into esbmc:master May 28, 2024
9 of 11 checks passed
@lucasccordeiro
Copy link
Contributor

Thanks for submitting this PR, @JacobYiu and @ChenfengWei0. Great work!

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.

None yet

4 participants