[PATCH v3 2/9] Add the initial TypoExpr AST node for delayed typo correction.

Richard Smith richard at metafoo.co.uk
Fri Jul 25 13:57:46 PDT 2014


On Mon, Jul 14, 2014 at 4:55 PM, Kaelyn Takata <rikka at google.com> wrote:

> ---
>  include/clang/AST/DataRecursiveASTVisitor.h |  1 +
>  include/clang/AST/Expr.h                    | 20 ++++++++++++++++++++
>  include/clang/AST/RecursiveASTVisitor.h     |  1 +
>  include/clang/Basic/StmtNodes.td            |  1 +
>  include/clang/Sema/SemaInternal.h           |  3 +++
>  include/clang/Sema/TypoCorrection.h         |  5 +++++
>  lib/AST/Expr.cpp                            |  1 +
>  lib/AST/ExprClassification.cpp              |  3 ++-
>  lib/AST/ExprConstant.cpp                    |  1 +
>  lib/AST/ItaniumMangle.cpp                   |  1 +
>  lib/AST/StmtPrinter.cpp                     |  5 +++++
>  lib/AST/StmtProfile.cpp                     |  4 ++++
>  lib/Sema/SemaExceptionSpec.cpp              |  1 +
>  lib/Sema/SemaLookup.cpp                     | 16 ++++++++++++++++
>  lib/Sema/TreeTransform.h                    |  6 ++++++
>  lib/Serialization/ASTReaderStmt.cpp         |  4 ++++
>  lib/Serialization/ASTWriterStmt.cpp         |  6 ++++++
>  lib/StaticAnalyzer/Core/ExprEngine.cpp      |  1 +
>  tools/libclang/CXCursor.cpp                 |  1 +
>  19 files changed, 80 insertions(+), 1 deletion(-)


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:

* The TypoExpr class should instead store a single opaque 'void*' pointer,
and make no attempt to deallocate or otherwise manage it.
* 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)

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.

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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140725/4c5df1bd/attachment.html>


More information about the cfe-commits mailing list