<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Jul 14, 2014 at 4:55 PM, Kaelyn Takata <span dir="ltr"><<a href="mailto:rikka@google.com" target="_blank">rikka@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">---<br>
 include/clang/AST/DataRecursiveASTVisitor.h |  1 +<br>
 include/clang/AST/Expr.h                    | 20 ++++++++++++++++++++<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/SemaLookup.cpp                     | 16 ++++++++++++++++<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>
 19 files changed, 80 insertions(+), 1 deletion(-)</blockquote><div><br></div><div>The AST library is supposed to be self-contained; it's a layering violation to put TypoExpr member definitions into Sema. Conversely, it'd be a layering violation to put them into AST, since they currently delete Sema types. To avoid this, I suggest:</div>
<div><br></div><div>* The TypoExpr class should instead store a single opaque 'void*' pointer, and make no attempt to deallocate or otherwise manage it.</div><div>* Sema owns the pointee and is responsible for deleting it (you can delete it and zero out the pointer on the TypoExpr at the point when you handle the TypoExpr)</div>
<div><br></div><div>Alternatively, since you need Sema to track the live TypoExprs anyway, you could have Sema maintain a map from TypoExpr* to whatever data you want to store for the TypoExprs, and have the AST node be essentially stateless.</div>
<div><br></div><div>Incidentally, I wonder whether it'd be useful for TypoExpr to store the typoed identifier, maybe as an IdentifierInfo*? (This'd allow you to produce something more useful from the statement printer and ast dumper, and we really don't care much how big a TypoExpr is, since we'll -- presumably -- never have many of them.) This might also be useful for clients that want to turn off typo correction but keep the TypoExprs in the AST.</div>
</div></div></div>