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

Replacing C-like input/output with C++ style code, some other changes #22

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

eigold
Copy link
Contributor

@eigold eigold commented Jul 3, 2017

No description provided.

@eigold eigold requested a review from mgudemann July 3, 2017 08:07
@kroening
Copy link
Member

kroening commented Jul 3, 2017

  • Is there a reason why the BDD2 test got removed?
  • Also, note that we use a custom stream for status and error messages, to be found in util/message.h. Do not use std::cout and std::cerr.
  • Please squash the 3 commits related to replacing fprintf into one.
  • The exception throwing does not need parentheses.

@eigold
Copy link
Contributor Author

eigold commented Jul 3, 2017 via email

@mgudemann
Copy link
Contributor

mgudemann commented Jul 3, 2017 via email

Copy link
Contributor

@mgudemann mgudemann left a comment

Choose a reason for hiding this comment

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

there are some recurring changes, in particular for what was char[...] before and is std::string now that can be simplified. It seems that this was mainly to format some intergal values, this can be don easily using std::to_string(...) to get a string, no need to create an std::ostringstream and call .str() on it.

also, all ic3_? regression tests fail, e.g., ic3_1 with

~/s/d/h/r/e/ic3_1 (new_ic3_bugs) $ ebmc pdtvispeterson.sv --ic3
Parsing pdtvispeterson.sv
Converting
Type-checking Verilog::pdtvispeterson
Using module `pdtvispeterson'
Generating Netlist
fish: “ebmc pdtvispeterson.sv --ic3” terminated by signal SIGSEGV (Address boundary error)

print_var_indexes(fname1);
fname1 = fname;
fname1 += ".ind";
print_var_indexes(fname1.c_str());
Copy link
Contributor

Choose a reason for hiding this comment

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

should be print_var_indexes(fname+".ind") and print_var_indexes should take const std::string & argument and also be renamed to print_var_indices

@@ -108,11 +108,11 @@ void CompInfo::gen_out_fun(DNF &H,int shift,bool short_version)
add_truth_table_gate_cubes(H,gate_ind,shift);
break;
case COMPLEX:
printf("complex gates are not allowed\n");
exit(1);
std::cout << "complex gates are not allowed\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

please use custom error stream

@@ -24,8 +25,8 @@ Author: Eugene Goldberg, eu.goldberg@gmail.com
==================================*/
void CompInfo::add_constrs()
{
printf("adding %d unit constraints\n",(int) (Constr_ilits.size() +
Constr_nilits.size()));
std::cout << "adding " << Constr_ilits.size() + Constr_nilits.size()
Copy link
Contributor

Choose a reason for hiding this comment

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

please use custom status output stream

{


FILE *fp = fopen(fname,"w");
assert(fp != NULL);
std::ofstream Out_str(fname,std::ios::out);
Copy link
Contributor

Choose a reason for hiding this comment

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

variables should be lowercase

Out_str << "topological level " << i << "\n";
CUBE &Gates = topol_levels[i];
for (size_t j=0; j < Gates.size(); j++) {
int gate_ind = Gates[j];
Copy link
Contributor

Choose a reason for hiding this comment

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

should be unsigned

src/ic3/s1tat.cc Outdated
Tf.num_bnd_cls,tf_lind+1,
Time_frames[tf_lind+1].num_bnd_cls);
std::cout << "new derived clauses Bnd[" << tf_lind <<"]=" << Tf.num_bnd_cls
<< ", Bnd[" << tf_lind+1 << "]="
Copy link
Contributor

Choose a reason for hiding this comment

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

is this a \tab?

}

for(size_t i=0;i < Buff.str().size();i++)
fake_name.push_back(Buff.str()[i]);
Copy link
Contributor

Choose a reason for hiding this comment

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

fake_name+=std::to_string(ind), remove Buff

Gate &G = N->get_gate(gate_ind);

assert(G.spec_buff_ind >= 0);

sprintf(buff,"%d",G.spec_buff_ind);
Buff << G.spec_buff_ind;
Copy link
Contributor

Choose a reason for hiding this comment

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

use const cstd::string s(std::to_string(G.spec_buff_ind))

Buff << G.spec_buff_ind;

for(size_t i=0;i < Buff.str().size();i++)
Name.push_back(Buff.str()[i]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Buff.str() -> s


std::ostringstream Buff;
Buff << "Tf_sat" << tf_ind;
std::string Slv_name = Buff.str();
Copy link
Contributor

Choose a reason for hiding this comment

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

use std::string slv_name("Tf_sat"+std::to_string(tf_ind));

@eigold
Copy link
Contributor Author

eigold commented Jul 3, 2017 via email

@eigold
Copy link
Contributor Author

eigold commented Jul 18, 2017 via email

@mgudemann
Copy link
Contributor

@eigold starts looking good, please remove the changes to BDD test cases from commit and squash commits according to nature of changes, e.g., printf->std::cout, std::cout -> messaget etc.

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

3 participants