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

[move] Add CFGIR Ast Visitor. Clean up rough edges of T::Program. #17787

Merged
merged 2 commits into from
May 29, 2024

Conversation

tnowacki
Copy link
Contributor

Description

  • Some lints probably want to use the CFGIR, since they are easier to do post optimizations
  • Cleaned up the linter APIs

Test plan

  • internal change

Release notes

Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.

For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.

  • Protocol:
  • Nodes (Validators and Full nodes):
  • Indexer:
  • JSON-RPC:
  • GraphQL:
  • CLI:
  • Rust SDK:

Copy link

vercel bot commented May 17, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
sui-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 20, 2024 6:34pm

Copy link

vercel bot commented May 17, 2024

@tnowacki is attempting to deploy a commit to the Mysten Labs Team on Vercel.

A member of the Team first needs to authorize it.

}
}

fn visit_module(
fn absint_visit_module(
Copy link
Contributor

@cgswords cgswords May 17, 2024

Choose a reason for hiding this comment

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

Could we eliminate this by using the new visitor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh thats fun. Stuffing all of the absint visitors in a visitor?

self.visit_exp(el);
self.visit_exp(er);
}
Command_::Break(_) | Command_::Continue(_) | Command_::Jump { .. } => (),
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if these could report an error somehow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure

if self.visit_constant_custom(module, constant_name, cdef) {
self.pop_warning_filter_scope();
return;
}
Copy link
Contributor

@cgswords cgswords May 17, 2024

Choose a reason for hiding this comment

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

Do we want to add visit_value and call that here (plus in visit_exp)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, seems helpful in the CFGIR

Copy link
Contributor

@cgswords cgswords left a comment

Choose a reason for hiding this comment

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

LGTM modulo a few small questions.

@tnowacki tnowacki merged commit 214719e into MystenLabs:main May 29, 2024
42 of 45 checks passed
@tnowacki tnowacki deleted the visitor branch May 29, 2024 00:18
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

2 participants