<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><div><div>On Oct 20, 2014, at 14:58 , Daniel Fahlgren <<a href="mailto:daniel@fahlgren.se">daniel@fahlgren.se</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">Hi Jordan,<br><br>On tor, 2014-10-16 at 09:21 -0700, Jordan Rose wrote:<br><blockquote type="cite">On Oct 14, 2014, at 15:12 , Daniel Fahlgren <<a href="mailto:daniel@fahlgren.se">daniel@fahlgren.se</a>> wrote:<br><br><blockquote type="cite">Hi Jordan,<br><br>On ons, 2014-10-08 at 20:05 -0700, Jordan Rose wrote:<br><br><blockquote type="cite">+ // First only check for errors. That way we will find problems even<br>if one of<br>+ // the dimensions is unknown.<br>+ const Expr *SE;<br>+ QualType QT;<br>+ do {<br>+ // Check size expression.<br>+ SE = VLA->getSizeExpr();<br>+ State = checkSizeExpr(SE, State, C);<br>+ if (!State)<br>+ return;<br></blockquote><br><blockquote type="cite">Seems like a worthy goal. If we're finding problems even if one<br>dimension is unknown, though, is it worth finding problems in every<br>size expression, rather than exiting early when we find a problem in<br>the first one?<br></blockquote><br>Good point. I'm not sure I solved this the correct way. It seems like<br>it only is possible to create one sink node so I had to call<br>getPredecessor(). Is that the right way or how should I do to emit<br>multiple errors? <br></blockquote><br>It's more just the normal uniquing of identical nodes kicking in—if<br>we've somehow already gotten to this state, that's supposed to mean<br>we've already emitted any errors. I think that means you should collect<br>all the errors up front and then emit them all on the same sink node.<br>Emitting them on the predecessor would be valid but not really what we<br>want, because it could have the wrong location.<br></blockquote><br><br>Ah, thanks for the explanation. I've updated the patch to store all<br>errors in a vector, allowing to emit all of them on the same node.<br></blockquote></div><br><div>I like this better. :-) A few small comments remaining:</div><div><br></div><div><div>+ typedef std::vector<VLABug> VLABugs;</div></div><div><br></div><div>This should probably use SmallVector at the call site and SmallVectorImpl in the function signature. It's not likely we'll have <i>that</i> many VLA bugs in a single declaration, so let's avoid the heap allocation.</div><div><br></div><div><div>+ for (unsigned I = 0, E = Bugs.size(); I != E; ++I)</div><div>+ reportBug(Bugs[I], N, State, C);</div></div><div><br></div><div>Please use a C++11 for-each loop here.</div><div><br></div><div>I think that's it, though!</div><div>Jordan</div><div><br></div></body></html>