Skip to content

Conversation

@celinval
Copy link
Contributor

Users don't usually set their main function to be public and we should enable them to verify their main function. For now, users will have to manually set the create type when running RMC directly. For cargo rmc, this should be automatically set up by cargo.

Resolved issues:

Resolves #655

Call-outs:

We should figure out what is the default behavior of RMC and we might want to add a mechanism to figure out which one to use based on the user configuration. For example, maybe we can check what is the target function for the verification.

Testing:

  • How is this change tested?

  • Is this a refactor change?

Checklist

  • Each commit message has a non-empty body, explaining why the change was made
  • Methods or procedures are documented
  • Regression or unit tests are included, or existing tests cover the modified code
  • My PR is restricted to a single feature or bugfix

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.

Users don't usually set their main function to be public and we should
enable them to verify their main function. For now, users will have to
manually set the create type when running RMC directly. For cargo rmc,
this should be automatically set up by cargo.
@celinval celinval requested a review from adpaco-aws November 19, 2021 00:30
@celinval celinval requested a review from a team as a code owner November 19, 2021 00:30
Copy link
Contributor

@adpaco-aws adpaco-aws left a comment

Choose a reason for hiding this comment

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

Does not look ready to me, see comments below. Also, how it comes we are able to codegen binary crates now? The changes in RMC scripts remove all lib-related flags, I do not see any major change anywhere in this PR.

// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0 OR MIT
pub fn main() {
fn main() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this change needed for each test as in #523?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. It's not needed as in #523. With this change RMC will accept both public and non public main.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I understand it is not strictly need but nice to have. Anyway, let me take care of that in #659

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks

.args(&self.props.cbmc_flags);
self.add_rmc_dir_to_path(&mut rmc);
let proc_res = self.compose_and_run_compiler(rmc, None);
let proc_res = self.run_rmc();
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for refactoring this!

@celinval
Copy link
Contributor Author

Does not look ready to me, see comments below. Also, how it comes we are able to codegen binary crates now? The changes in RMC scripts remove all lib-related flags, I do not see any major change anywhere in this PR.

Previous fixes is what enabled us to be able to codegen binary crates. This PR is just to remove the hack that we were enforcing the crate type to be a lib.

@adpaco-aws adpaco-aws requested a review from danielsn November 22, 2021 15:16
@adpaco-aws adpaco-aws merged commit 55c78bb into model-checking:main Nov 22, 2021
@celinval celinval deleted the issue-655 branch November 22, 2021 22:52
.args(&self.props.cbmc_flags);
self.add_rmc_dir_to_path(&mut rmc);
let proc_res = self.compose_and_run_compiler(rmc, None);
let proc_res = self.run_rmc();
Copy link
Contributor

Choose a reason for hiding this comment

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

💯

tedinski pushed a commit to tedinski/rmc that referenced this pull request Apr 26, 2022
…ng#656)

* Re-enable RMC to allow users to verify non-public main.

Users don't usually set their main function to be public and we should
enable them to verify their main function. For now, users will have to
manually set the create type when running RMC directly. For cargo rmc,
this should be automatically set up by cargo.

* Update src/tools/dashboard/src/books.rs

Co-authored-by: Adrian Palacios <[email protected]>

Co-authored-by: Adrian Palacios <[email protected]>
tedinski pushed a commit that referenced this pull request Apr 27, 2022
* Re-enable RMC to allow users to verify non-public main.

Users don't usually set their main function to be public and we should
enable them to verify their main function. For now, users will have to
manually set the create type when running RMC directly. For cargo rmc,
this should be automatically set up by cargo.

* Update src/tools/dashboard/src/books.rs

Co-authored-by: Adrian Palacios <[email protected]>

Co-authored-by: Adrian Palacios <[email protected]>
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.

Re-enable support to binary crates verification

4 participants