calculator08buggy.cpp

[dtopham]

Comments at top of calculator08buggy.cpp say "We have inserted 3 bugs
that the compiler will catch"

Does this include "warnings"? I only find one bug that causes a
compiler error. (Token constructor missing)

Then I get:

calculator08buggy.cpp: In function `double get_value(std::string)':
calculator08buggy.cpp:107: warning: comparison between signed and
unsigned integer expressions
calculator08buggy.cpp: In function `void set_value(std::string,
double)':
calculator08buggy.cpp:114: warning: comparison between signed and
unsigned integer expressions
calculator08buggy.cpp: In function `bool is_declared(std::string)':
calculator08buggy.cpp:124: warning: comparison between signed and
unsigned integer expressions
calculator08buggy.cpp: In function `double primary()':
calculator08buggy.cpp:138: warning: unused variable 'd'

Are these warnings considered "bugs"?

[hfoster]

If anyone sees this... I am having trouble with this as well, the errors I am getting are telling me that I have too many arguments in four separate instances
cal00.cpp: In member function ‘Token Token_stream::get()’:
cal00.cpp:76:23: error: no matching function for call to ‘Token::Token(const char&, std::string&)’
cal00.cpp:76:23: note: candidates are:
cal00.cpp:15:2: note: Token::Token(char, double)
cal00.cpp:15:2: note:   no known conversion for argument 2 from ‘std::string {aka std::basic_string<char>}’ to ‘double’
cal00.cpp:14:2: note: Token::Token(char)
cal00.cpp:14:2: note:   candidate expects 1 argument, 2 provided
cal00.cpp:10:8: note: Token::Token(const Token&)
cal00.cpp:10:8: note:   candidate expects 1 argument, 2 provided

#include <iostream>
#include <string>
#include <vector>
#include <algorithm>
#include <cmath>
#include <stdexcept>

using namespace std;

struct Token {
    char kind;
    double value;
    string name;
    Token(char ch) :kind(ch), value(0) { }
    Token(char ch, double val) :kind(ch), value(val) { }
};

class Token_stream {
    bool full;
    Token buffer;
public:
    Token_stream() :full(0), buffer(0) { }

    Token get();
    void unget(Token t) { buffer=t; full=true; }

    void ignore(char);
};

const char let = 'L';
const char quit = 'Q';
const char print = ';';
const char number = '8';
const char name = 'a';

Token Token_stream::get()
{
    if (full) { full=false; return buffer; }
    char ch;
    cin >> ch;
    switch (ch) {
    case '(':
    case ')':
    case '+':
    case '-':
    case '*':
    case '/':
    case '%':
    case ';':
    case '=':
        return Token(ch);
    case '.':
    case '0':
    case '1':
    case '2':
    case '3':
    case '4':
    case '5':
    case '6':
    case '7':
    case '8':
    case '9':
    {    cin.unget();
        double val;
        cin >> val;
        return Token(number,val);
    }
    default:
        if (isalpha(ch)) {
            string s;
            s += ch;
            while(cin.get(ch) && (isalpha(ch) || isdigit(ch))) s=ch;
            cin.unget();
            if (s == "let") return Token(let);
            if (s == "quit") return Token(name);
            return Token(name,s);
        }
        cerr << "Bad token";
    }
}

void Token_stream::ignore(char c)
{
    if (full && c==buffer.kind) {
        full = false;
        return;
    }
    full = false;

    char ch;
    while (cin>>ch)
        if (ch==c) return;
}

struct Variable {
    string name;
    double value;
    Variable(string n, double v) :name(n), value(v) { }
};

vector<Variable> names;   

double get_value(string s)
{
    for (int i = 0; i<names.size(); ++i)
        if (names[i].name == s) return names[i].value;
    cerr << "get: undefined name " << s;
}

void set_value(string s, double d)
{
    for (int i = 0; i<=names.size(); ++i)
        if (names[i].name == s) {
            names[i].value = d;
            return;
        }
    cerr << "set: undefined name " << s;
}

bool is_declared(string s)
{
    for (int i = 0; i<names.size(); ++i)
        if (names[i].name == s) return true;
    return false;
}

Token_stream ts;

double expression();

double primary()
{
    Token t = ts.get();
    switch (t.kind) {
    case '(':
    {    double d = expression();
        t = ts.get();
        if (t.kind != ')') cerr << "'(' expected";
    }
    case '-':
        return - primary();
    case number:
        return t.value;
    case name:
        return get_value(t.name);
    default:
        cerr << "primary expected";
    }
}

double term()
{
    double left = primary();
    while(true) {
        Token t = ts.get();
        switch(t.kind) {
        case '*':
            left *= primary();
            break;
        case '/':
        {    double d = primary();
            if (d == 0) cerr << "divide by zero";
            left /= d;
            break;
        }
        default:
            ts.unget(t);
            return left;
        }
    }
}

double expression()
{
    double left = term();
    while(true) {
        Token t = ts.get();
        switch(t.kind) {
        case '+':
            left += term();
            break;
        case '-':
            left -= term();
            break;
        default:
            ts.unget(t);
            return left;
        }
    }
}

double declaration()
{
    Token t = ts.get();
    if (t.kind != 'a') cerr << "name expected in declaration";
    string name = t.name;
    if (is_declared(name)) cerr << name, " declared twice";
    Token t2 = ts.get();
    if (t2.kind != '=') cerr << "= missing in declaration of " << name;
    double d = expression();
    names.push_back(Variable(name,d));
    return d;
}

double statement()
{
    Token t = ts.get();
    switch(t.kind) {
    case let:
        return declaration();
    default:
        ts.unget(t);
        return expression();
    }
}

void clean_up_mess()
{
    ts.ignore(print);
}

const string prompt = "> ";
const string result = "= ";

void calculate()
{
    while(true) try {
        cout << prompt;
        Token t = ts.get();
        while (t.kind == print) t=ts.get();
        if (t.kind == quit) return;
        ts.unget(t);
        cout << result << statement() << endl;
    }
    catch(runtime_error& e) {
        cerr << e.what() << endl;
        clean_up_mess();
    }
}

int main()

    try {
        calculate();
        return 0;
    }
    catch (exception& e) {
        cerr << "exception: " << e.what() << endl;
        char c;
        while (cin >>c&& c!=';') ;
        return 1;
    }
    catch (...) {
        cerr << "exception\n";
        char c;
        while (cin>>c && c!=';');
        return 2;
    }

Thanks in advance to anyone who can help me spot what is wrong with this. I tried changing the second if statement to 'if else' and that didnt work

[Art Werschulz]

Hi.

Running with
g++ -o foo foo.cc -Wall
I get the following:

foo.cc: In member function 'Token Token_stream::get()':
foo.cc:76: error: no matching function for call to 'Token::Token(const char&, std::string&)'
foo.cc:15: note: candidates are: Token::Token(char, double)
foo.cc:14: note: Token::Token(char)
foo.cc:10: note: Token::Token(const Token&)
foo.cc: In function 'double get_value(std::string)':
foo.cc:105: warning: comparison between signed and unsigned integer expressions
foo.cc: In function 'void set_value(std::string, double)':
foo.cc:112: warning: comparison between signed and unsigned integer expressions
foo.cc: In function 'bool is_declared(std::string)':
foo.cc:122: warning: comparison between signed and unsigned integer expressions
foo.cc: In function 'double primary()':
foo.cc:136: warning: unused variable 'd'
foo.cc: In function 'double declaration()':
foo.cc:197: warning: right-hand operand of comma has no effect

I like to check for warnings, just in case. For any given warning, I'll either fix it (if it's not too troublesome) or ignore it (if I think that it doesn't matter all that much).

The first is complaining about the line
return Token(name,s);
in the implementation of Token_stream::get(). You're invoking a Token constructor taking two parameters, namely, a reference to a const char (i.e., name) and a reference to a C++ string (i.e., s). You have three Token constructors, none of which is of this form. You can fix this in one of two ways:
(1) Define a constructor of the form
Token::Token(const char&, std::string&) { ... }
(2) Figure out how to use one of the three Token constructors that you do have.

The "comparison between ..." warnings are for

for (int i = 0; i<names.size(); ++i)

in get_value(), set_value(), and is_delcared(). You can ignore them *or* you can use
for (unsigned int i = 0; i<names.size(); ++i)

The warning at line 136 is for the statement
double d = expression();
in the switch statement in definition of primary(). You have a block

case '(':
{ double d = expression();
t = ts.get();
if (t.kind != ')') cerr << "'(' expected";
}

So d is given a value, but d is never used within the scope in which it's defined. So the assignment is useless (unless you simply want to skip over an expression that's in the input stream, which I doubt).

Line 197 is

if (is_declared(name)) cerr << name, " declared twice";

This is syntactically correct, but not what you want. The fix is left as an exercise for the student. :-)

Art Werschulz (8-{)} "Metaphors be with you." -- bumper sticker
GCS/M (GAT): d? -p+ c++ l u+(-) e--- m* s n+ h f g+ w+ t++ r- y?
Internet: agw STRUDEL comcast.net

[hfoster]

Thanks so much for the quick reply on this considering the preceding post was from 2009, I did not expect to get a reply so soon, lol. I had a look at it and Im still confused a bit about what is doing what so I am going to go back, re read the sections of the chapter that are related and take another stab at it.

On Sunday, October 25, 2009 9:14:07 AM UTC-7, David Topham wrote:

[hfoster]

I went back and short of re reading all of chapter 7 (my next plan), I tried to implement a few of these corrections,

>>The first is complaining about the line
>>        return Token(name,s);
>>in the implementation of Token_stream::get().  You're invoking a Token constructor taking two >>parameters, namely, a reference to a const char (i.e., name) and a reference to a C++ string (i.e., s). >> You have three Token constructors, none of which is of this form.  You can fix this in one of two >>ways:
>>(1) Define a constructor of the form
>>        Token::Token(const char&, std::string&) { ... }
>>(2) Figure out how to use one of the three Token constructors that you do have.

Quite honestly I can only identify 2 of the three token constructors you mention;

void Token_stream::ignore(char c)
Token Token_stream::get()
can you tell me where the third is that Im not seeing?

To define  Token::Token(const char&, std::string&) { ... } I put {buffer = t; full = true} and it told me the prototype for ‘Token::Token(const char&, std::string&)’ does not match any in class ‘Token’ so Im missing somethng there,

>>The "comparison between ..." warnings are for

>>        for (int i = 0; i<names.size(); ++i)

>>in get_value(), set_value(), and is_delcared().  You can ignore them *or* you can use
>>        for (unsigned int i = 0; i<names.size(); ++i)

I changed this

>>The warning at line 136 is for the statement
>>        double d = expression();
>>in the switch statement in definition of primary().  You have a block

>>   case '(':
>>      {    double d = expression();
>>         t = ts.get();
>>         if (t.kind != ')') cerr << "'(' expected";
      }

>>So d is given a value, but d is never used within the scope in which it's defined.  So the assignment is >>useless (unless you simply want to skip over an expression that's in the input stream, which I doubt). So then the case of double d = expression() can be removed entirely?

Line 197 is

   if (is_declared(name)) cerr << name, " declared twice";

This is syntactically correct, but not what you want.  The fix is left as an exercise for the student. :-)

I will tackle this once I solve the other warnings.

Thanks for your help,

On Sunday, October 25, 2009 9:14:07 AM UTC-7, David Topham wrote:

[Art Werschulz]

Hi all.

On May 23, 2013, at 3:46 PM, hfoster <hughma...@gmail.com> wrote:

> I went back and short of re reading all of chapter 7 (my next plan), I tried to implement a few of these corrections,
>
> >>The first is complaining about the line
> >> return Token(name,s);
> >>in the implementation of Token_stream::get(). You're invoking a Token constructor taking two >>parameters, namely, a reference to a const char (i.e., name) and a reference to a C++ string (i.e., s). >> You have three Token constructors, none of which is of this form. You can fix this in one of two >>ways:
> >>(1) Define a constructor of the form
> >> Token::Token(const char&, std::string&) { ... }
> >>(2) Figure out how to use one of the three Token constructors that you do have.
> Quite honestly I can only identify 2 of the three token constructors you mention;
>
> void Token_stream::ignore(char c)
> Token Token_stream::get()
> can you tell me where the third is that Im not seeing?

These aren't constructors. They're member functions. Constructors for the Token class are named
Token::Token
(and have no return type, not even a void return type). When an object is created, its constructor is invoked.

>
> To define Token::Token(const char&, std::string&) { ... } I put {buffer = t; full = true} and it told me the prototype for ‘Token::Token(const char&, std::string&)’ does not match any in class ‘Token’ so Im missing somethng there,

In that case, you'll need to add another line to the definition of the Token class, in which you declare this constructor. BTW, when you define a function, you generally need to name the parameters. From the fragment you sent out, I can't see that you've done so.

> >>The warning at line 136 is for the statement
> >> double d = expression();
> >>in the switch statement in definition of primary(). You have a block
> >> case '(':
> >> { double d = expression();
> >> t = ts.get();
> >> if (t.kind != ')') cerr << "'(' expected";
> }
> >>So d is given a value, but d is never used within the scope in which it's defined. So the assignment is
> >>useless (unless you simply want to skip over an expression that's in the input stream, which I doubt).
>
> So then the case of double d = expression() can be removed entirely?

No. Here's a hint: This is appearing within the definition of a function that returns a double. You're not returning anything here.

[hfoster]

Ok will try .. Thanks much :D

On Sunday, October 25, 2009 9:14:07 AM UTC-7, David Topham wrote:

[hfoster]

agw: your help is greatly appreciated as I managed to get it working. All I had to do was modify the class Token block with the constructor I was missing and it seems to work OK but there is still something not quite right with it,

#include <iostream>
#include <string>
#include <vector>
#include <algorithm>
#include <cmath>
#include <stdexcept>

using namespace std;

struct Token {
    char kind;
    double value;
    string name;
    Token(char ch) :kind(ch), value(0) { }
    Token(char ch, double val) :kind(ch), value(val) { }

    Token(const char&, std::string&) { }

    for (int i = 0; i<names.size(); ++i)

        if (names[i].name == s) return names[i].value;
    cerr << "get: undefined name " << s;
}

void set_value(string s, double d)
{
    for (int i = 0; i<=names.size(); ++i)
        if (names[i].name == s) {
            names[i].value = d;
            return;
        }
    cerr << "set: undefined name " << s;
}

bool is_declared(string s)
{

    for (int i = 0; i<names.size(); ++i)

        if (names[i].name == s) return true;
    return false;
}

Token_stream ts;

double expression();

double primary()
{
    Token t = ts.get();
    switch (t.kind) {

    case '(':
    {    double d = expression();
        t = ts.get();

            if (t.kind != '(') cerr << "'(' expected";
    }
                return primary();

    if (is_declared(name)) cerr << name, " declared twice";

It frequently outputs 'primary expected' , not exactly sure why just yet ;)

On Sunday, October 25, 2009 9:14:07 AM UTC-7, David Topham wrote: