[xlscc] Expression cloning #2057
Conversation
Signed-off-by: Krzysztof Bieganski <[email protected]>
Signed-off-by: Krzysztof Bieganski <[email protected]>
Signed-off-by: Krzysztof Bieganski <[email protected]>
Signed-off-by: Krzysztof Bieganski <[email protected]>
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
xls/contrib/xlscc/expr_clone.h
Outdated
| } | ||
|
|
||
| namespace xlscc { | ||
| const clang::Expr* Clone(clang::ASTContext& ctx, const clang::Expr* expr); |
There was a problem hiding this comment.
What happens when an unsupported type of expression is passed in? Is everything guaranteed to succeed? I imagine this should return absl::StatusOrclang::Expr* (why const?).
There was a problem hiding this comment.
An assert in the visitor is triggered, if asserts are off I guess it crashes on null pointer. I was thinking it would always explicitly abort, but it doesn't seem to. Anyway, sure, I'll make it return StatusOr.
It's const because the original AST is const. If the cloned AST isn't const then we leak non-const access to Decls (see here).
There was a problem hiding this comment.
Added graceful handling of unsupported expressions. Let me know if the const is OK.
Signed-off-by: Krzysztof Bieganski <[email protected]>
Signed-off-by: Krzysztof Bieganski <[email protected]>
Signed-off-by: Krzysztof Bieganski <[email protected]>
kbieganski
force-pushed
the
clone-clang-expr
branch
from
4f00b66 to
5570f91
Compare
xls/contrib/xlscc/expr_clone.cc
Outdated
| expr->requiresZeroInitialization()); | ||
| } | ||
|
|
||
| clang::Expr* VisitExpr(const clang::Expr* expr) { return nullptr; } |
There was a problem hiding this comment.
Could you store the specific clang::Expr* here for error reporting?
There was a problem hiding this comment.
I don't think StatusOr can hold arbitrary data, but now I'm storing the name of the offending expression here which should be good enough for error reporting.
Signed-off-by: Krzysztof Bieganski <[email protected]>
Signed-off-by: Krzysztof Bieganski <[email protected]>
Signed-off-by: Krzysztof Bieganski <[email protected]>
This patch adds a Clang expression tree cloning utility to
xlscc, as well as a unit test for it.It handles most C++ expressions, with one notable exception being lambdas (as they are more complex but probably won't be needed).
It doesn't support non-standard extensions.