search

nitnelave / hopper

Programming language
Other
2 stars 0 forks source link

Simplify the IfStatement node #57

Closed nitnelave closed 7 years ago

nitnelave commented 7 years ago

By having a simple structure, the code generation, printing, and general handling is simplified.

This change is Reviewable

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.2%) to 98.687% when pulling 59fdb905dd73048e6e6f1b1b78f7f18fead0cfc1 on simplify_if into 0b61428d135243554c5cbcb09930487811a2f97e on master.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.2%) to 98.689% when pulling 328bbef372876b48f746c8bbd54c0175594d63a8 on simplify_if into 0b61428d135243554c5cbcb09930487811a2f97e on master.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.09%) to 98.998% when pulling e9325cc3eb138f12dda832b76c627650176d333a on simplify_if into 0b61428d135243554c5cbcb09930487811a2f97e on master.

bloody76 commented 7 years ago

Reviewed 1 of 1 files at r1, 1 of 1 files at r2, 1 of 1 files at r3, 20 of 20 files at r4, 1 of 1 files at r5. Review status: all files reviewed at latest revision, 5 unresolved discussions.

src/codegen/codegen_statement.cc, line 76 at r5 (raw file):

log(ERROR)

Why ERROR ? Why not TRACE or DEBUG ?

src/codegen/codegen_statement.cc, line 97 at r5 (raw file):

  ir_builder_.SetInsertPoint(if_start_block);
  ir_builder_.CreateCondBr(equality, if_block,
                           else_block.is_ok() ? else_block.value_or_die()

else_block.value_or(ifend_block.value_or_die()) ?

test/resources/ir/if_elseif_statement.gh, line 5 at r5 (raw file):

    1;
  } else {
  if (2) {

why changing the test ? Identation anyway

test/resources/ir/if_statement2.gh, line 5 at r5 (raw file):

    1;
  }

same, why changing the test ?

test/resources/ir/if_statement3.ref, line 0 at r5 (raw file): why deleting ?

Comments from Reviewable

nitnelave commented 7 years ago

Review status: 21 of 23 files reviewed at latest revision, 5 unresolved discussions.

src/codegen/codegen_statement.cc, line 76 at r5 (raw file):

Previously, bloody76 (Bd76) wrote…
> log(ERROR) Why ERROR ? Why not TRACE or DEBUG ?

Debugging leftovers, I wanted it to display during test execution. Removed.

src/codegen/codegen_statement.cc, line 97 at r5 (raw file):

Previously, bloody76 (Bd76) wrote…
else_block.value_or(ifend_block.value_or_die()) ?

no, because the argument of the function is greedily evaluated, leading to the or_die() part.

test/resources/ir/if_statement2.gh, line 5 at r5 (raw file):

Previously, bloody76 (Bd76) wrote…
same, why changing the test ?

Here, the test is the if_statement3 that I just moved to if_statement2 after deleting this one. I deleted it because it was the same test as if_statement1, except that the if did not have braces. This is a syntax difference, not a semantic one, so it should be checked at the pretty-printer level.

test/resources/ir/if_statement3.ref, line at r5 (raw file):

Previously, bloody76 (Bd76) wrote…
why deleting ?

See above.

test/resources/ir/if_elseif_statement.gh, line 5 at r5 (raw file):

Previously, bloody76 (Bd76) wrote…
why changing the test ? Identation anyway

debugging leftovers, reverted.

Comments from Reviewable

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.08%) to 98.995% when pulling bb520024fbb3b811cd1d4a728a510ca986f9aecf on simplify_if into 0b61428d135243554c5cbcb09930487811a2f97e on master.

bloody76 commented 7 years ago

Reviewed 1 of 3 files at r6. Review status: 22 of 23 files reviewed at latest revision, 1 unresolved discussion.

Comments from Reviewable

bloody76 commented 7 years ago
:lgtm:

Review status: 22 of 23 files reviewed at latest revision, all discussions resolved.

Comments from Reviewable