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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65591/new/ https://reviews.llvm.org/D65591 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
