adamjstewart / prolog

An implementation of Prolog written in OCaml
MIT License
2 stars 3 forks source link

Get rid of BoolConst #6

Closed adamjstewart closed 6 years ago

adamjstewart commented 6 years ago

The purpose of this PR is to remove BoolConst from the AST. This allows for a uniform treatment of "true" and "false" in parsing and evaluating. The following expressions:

cat.
cat :- true.

should have the same representation in the AST now.

ZaidQureshi commented 6 years ago

I know I sort of made a big deal out of this last night, but I was thinking and I don't think we need to do this as long as we can justify the way we do it in the master branch. The justification that we have for what we have in the master branch is that by having the two ways represented slightly differently we can evaluate the following stuff the same way swi-prolog does it. db: a(true). b(X) :- a(X).

?- b(a(true)). false.

With making them both the same thing, the evaluator would evaluate that to true.

Also, it is going to be a pain to fix all the code and tests and I would rather focus on the report.

ZaidQureshi commented 6 years ago

Fixed all evaluator and all tests to remove boolconst. although since you made the pr request you should have fixed all the code, like i did for the previous pr request but its ok.

adamjstewart commented 6 years ago

When I run make test I see the following warning message:

File "src/evaluator.ml", line 250, characters 34-63:
Warning 11: this match case is unused.

It looks like this block is duplicated. Or should one of them be for false?

ZaidQureshi commented 6 years ago

fixed

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.09%) to 92.739% when pulling 08e7a66c5a1c4493b27e63e9a95434c8e696bf34 on features/bool into 1d58fa3ff77c072573030ca3212521ec8d2378a1 on master.

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-0.04%) to 92.612% when pulling 08e7a66c5a1c4493b27e63e9a95434c8e696bf34 on features/bool into 1d58fa3ff77c072573030ca3212521ec8d2378a1 on master.