[PATCH] D65591: [AST] Add a flag indicating if any subexpression had errors
Ilya Biryukov via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Aug 13 01:25:47 PDT 2019
ilya-biryukov added a comment.
In D65591#1625744 <https://reviews.llvm.org/D65591#1625744>, @aaron.ballman wrote:
> The problem is: those bits are not infinite and we only have a handful left until bumping the allocation size; is this use case critical enough to use one of those bits? I don't think it will be -- it seems like premature optimization. Also, by calculating rather than using a bit, you don't have to touch every `Expr` constructor, which reduces the complexity of the patch.
Alternatively, we could keep an equivalent of `set<Expr*> InvalidExprs` in `ASTContext`. Gives the same computational complexity, but has a higher constant overhead factor.
Does that look reasonable?
> Some other things I think are missing from the patch (regardless of whether you go with a bit or calculate on the fly):
> - Do you need some changes to AST serialization and deserialization?
Good point, will update the patch.
> - Does anything special need to happen for modules?
Not sure. What are the potential problems you foresee?
> - I would expect to see this information reflected in an AST dump
Good point. Will do. Although it's a little hard to test in this patch, since it's hard to catch a `TypoExpr` in the AST dump.
> - How should this impact AST matching interfaces?
We could add a matcher that filters on this flag, but I would start with adding more expressions first (something similar to `ErrorExpr`);
For the purposes of this patch, I'd keep the matcher interfaces untouched.
> - Test cases
Again, since it's hard to catch a `TypoExpr` in the final AST dump, it's hard to catch this bit. See the dependent revision for a bogus diagnostic not being emitted anymore.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the cfe-commits