Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Fix not working --no-missing-ricardian-clause #1197

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

conr2d
Copy link
Contributor

@conr2d conr2d commented Aug 30, 2021

Change Description

This PR makes --no-missing-ricardian-clause compile option work correctly.

API Changes

  • API Changes

Documentation Additions

  • Documentation Additions

@@ -729,6 +721,7 @@ namespace eosio { namespace cdt {
public:
explicit eosio_abigen_visitor(CompilerInstance *CI) {
get_error_emitter().set_compiler_instance(CI);
set_suppress_ricardian_warning(ag.suppress_ricardian_warnings);

Choose a reason for hiding this comment

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

I don't think you need this one. suppress_ricardian_warnings above is taken from abigen

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, when I wrote this patch, eosio_abigen_visitor called eosio_abigen_visitor::parse_contracts instead of abigen::parse_contracts. This is obsolete now.

Choose a reason for hiding this comment

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

got it. Yes, After some refactoring ricardian contracts generation was broken and recently it was fixed including this warning suppression.
So now other then correcting warning message there is nothing to do. Anyway if you'll add toolchain tests for this warning it will be helpful for future

// TODO
std::cout << "Warning, action <"+get_action_name(decl)+"> does not have a ricardian contract\n";
if (!suppress_ricardian_warnings)
CDT_CHECK_WARN(!rcs[get_action_name(decl)].empty(), "abigen_warning", decl->getLocation(), "Action <"+get_action_name(decl)+"> does not have a ricardian contract");
Copy link

@dimas1185 dimas1185 Sep 2, 2021

Choose a reason for hiding this comment

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

what is the difference? Does it now outputs to stderr instead of stdout?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Duplicated code. It seems original author intended replacing stdout warning message with CDT_CHECK_WARN macro when macro is implemented, but just added rather than replace it.

@dimas1185
Copy link

dimas1185 commented Sep 2, 2021

thanks for your contribution.
do you mind to add toolchain tests, please?
you can take an example from here:
https://github.com/EOSIO/eosio.cdt/blob/4ff779217c54dccc8fb9a59463c0e1c6cb78e82b/tests/toolchain/compile-pass/warn_action_read_only.json

basically you need just to create dummy contract with same name and check stderr/stdout in json. you can use regexp there.
to run toolchain tests use ./build/tools/toolchain-tester/toolchain-tester

@conr2d conr2d force-pushed the fix/suppress-ricardian-warning branch from 5accc84 to ada19a0 Compare September 10, 2021 10:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants