Dan Hecht has posted comments on this change.

Change subject: IMPALA-5860: upgrade to LLVM 3.9.1
......................................................................


Patch Set 2: Code-Review+2

(3 comments)

Please decide if you want kwho to take a look as well.

http://gerrit.cloudera.org:8080/#/c/7974/2/be/src/codegen/codegen-symbol-emitter.cc
File be/src/codegen/codegen-symbol-emitter.cc:

PS2, Line 98: getType
what type does that thing return?  oh, maybe it's an "Expected<>"? Maybe pull 
that into a variable to make this operator bool() overloading clearer?


PS2, Line 104: *addr_or_err
how about using get() like you did on line 98?


PS2, Line 106: ->
same


-- 
To view, visit http://gerrit.cloudera.org:8080/7974
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ida873ddb15e393b0bd37486db24add8a32f43ad0
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com>
Gerrit-HasComments: Yes

Reply via email to