<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">Thanks for reviewing this, David!</div><div class="gmail_quote"><br></div><div class="gmail_quote">On Fri, Sep 5, 2014 at 2:22 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank" onclick="window.open('https://mail.google.com/mail/?view=cm&tf=1&to=dblaikie@gmail.com&cc=&bcc=&su=&body=','_blank');return false;" class="cremed">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Looks mostly reasonable.<br><br>The changes to include/clang/Sema/SemaInternal.h seem unrelated to this patch, though - perhaps they're meant for some other patch? The TypoDiagnosticGenerator too, is an isolated/unrelated change.<br></div></blockquote><div><br></div><div>Yup, those look like a few pieces that got added to the wrong patch when I was cleaning up and shuffling around and squashing commits into more reasonable patches. They should be in patch #5 that starts adding the infrastructure for handling a TypoExpr (of which TypoDiagnosticGenerator is a part).</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br>It might be worth adding a test case for statement printing (StmtPrinter.cpp) with typos - if you can cause the TypoExpr to still be there when you do an AST Dump? (perhaps on invalid code? Or do you always transform the TypoExpr into /something/ else, even if you can't suggest a specific function to call?)<br></div></blockquote><div><br></div><div>I'm not sure how to test that, even ignoring the fact that nothing in this patch generates a TypoExpr. Also, the TypoExpr is a placeholder that should either be replaced with a correction or an ExprError (with appropriate diagnostic) before anything is truly done with the full Expr, so I don't think it is possible to dump the AST--even with invalid input--before all of the TypoExprs have been replaced (or at least without it being a bug in the code where a path was hit that failed to handle TypoExprs). While it may change in the future for use with something like code completion, as of now TypoExprs shouldn't ever be exposed and could not meaningfully be dumped and re-read as most of their state is saved in the specific Sema instance (e.g. all of the namespaces currently visible to the translation units, and all of the possible corrections that couldn't be immediately discarded e.g. by virtue of the identifier being way too different).</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br>I'm assuming all the other changes are just necessary bits & Pieces to adding a new expr type? (eg: if I removed some of them, things would break/fail to compile/etc)<br></div></blockquote><div><br></div><div>Yeah, much of this patch is just the boilerplate associated with keeping compilers happy when adding a new Expr type.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Aug 26, 2014 at 11:04 AM, Kaelyn Takata <span dir="ltr"><<a href="mailto:rikka@google.com" target="_blank" onclick="window.open('https://mail.google.com/mail/?view=cm&tf=1&to=rikka@google.com&cc=&bcc=&su=&body=','_blank');return false;" class="cremed">rikka@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">+dblaikie</div><div><div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Jul 31, 2014 at 1:20 PM, Kaelyn Takata <span dir="ltr"><<a href="mailto:rikka@google.com" target="_blank" onclick="window.open('https://mail.google.com/mail/?view=cm&tf=1&to=rikka@google.com&cc=&bcc=&su=&body=','_blank');return false;" class="cremed">rikka@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">---<br>
include/clang/AST/DataRecursiveASTVisitor.h | 1 +<br>
include/clang/AST/Expr.h | 18 ++++++++++++++++++<br>
include/clang/AST/RecursiveASTVisitor.h | 1 +<br>
include/clang/Basic/StmtNodes.td | 1 +<br>
include/clang/Sema/SemaInternal.h | 3 +++<br>
include/clang/Sema/TypoCorrection.h | 5 +++++<br>
lib/AST/Expr.cpp | 1 +<br>
lib/AST/ExprClassification.cpp | 3 ++-<br>
lib/AST/ExprConstant.cpp | 1 +<br>
lib/AST/ItaniumMangle.cpp | 1 +<br>
lib/AST/StmtPrinter.cpp | 5 +++++<br>
lib/AST/StmtProfile.cpp | 4 ++++<br>
lib/Sema/SemaExceptionSpec.cpp | 1 +<br>
lib/Sema/TreeTransform.h | 6 ++++++<br>
lib/Serialization/ASTReaderStmt.cpp | 4 ++++<br>
lib/Serialization/ASTWriterStmt.cpp | 6 ++++++<br>
lib/StaticAnalyzer/Core/ExprEngine.cpp | 1 +<br>
tools/libclang/CXCursor.cpp | 1 +<br>
18 files changed, 62 insertions(+), 1 deletion(-)<br>
<br>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div></div>