<div dir="ltr">On 8 October 2013 23:25, Jordan Justen <span dir="ltr"><<a href="mailto:jljusten@gmail.com" target="_blank">jljusten@gmail.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

Patches 1, 2 & 4 - 10:<br>
Reviewed-by: Jordan Justen <<a href="mailto:jordan.l.justen@intel.com" target="_blank">jordan.l.justen@intel.com</a>><br>
<br>
For 8, I replied with a non-essential question, and the same question<br>
is relevant in patch 10.<br>
<br>
For patch 3 I had that question (mentioned on irc) about ir having<br>
ast/parser knowledge. I guess I'd like to think about this one a<br>
little more.<br></blockquote><div><br></div><div>I've been thinking about this more too, and I'm of two minds about it.<br><br>On the one hand, I think we're going to wind up polluting the IR with ast/parser knowledge one of these days anyhow, since it would be nice for (a) linker errors to be able to refer to source line numbers, and (b) the output of INTEL_DEBUG={vs,gs,fs} to refer to source code lines rather than IR, and in order for either of those to happen, the IR needs to be able to store source locations, which are currently knowledge belonging to the ast/parser.<br>

<br></div><div>One the other hand, I didn't really get as much benefit out of using virtual method dispatch in patch 3 as I was expecting, and I got a fair bit of headache (which is why patch 2 was necessary).  It would probably be just as easy to make update_max_array_access() an ordinary function in ast_array_index.cpp, and implement its dynamic behaviour using functions like as_dereference_record() and as_dereference_variable().  I'll try doing that this morning and let you know how it goes.<br>
</div></div></div></div>