ilya-biryukov accepted this revision as: ilya-biryukov. ilya-biryukov added a comment.
In D132918#3773237 <https://reviews.llvm.org/D132918#3773237>, @shafik wrote: > This is helpful information but I am not sure this convinces me that > `EvaluateVarDecl` is the correct place to check. Why not in `EvaluateDecl` or > `EvaluateStmt`? Both locations we could also check. So the options are: 1. `assert(Decl->isValid())` in `EvaluateVarDecl` and checks at call sites in `EvaluateDecl` and `EvaluateStmt`. 2. a single check in `EvaluateVarDecl`. The actual observable behavior of the compiler seems equivalent (`EvaluateDecl` can only fail inside `EvaluateVarDecl` anyway). I think either is fine, neither of the approaches seems to be a big win over the other. I will stamp this as I'm already in the reviewers list, think this change is ok and believe that Aaron's comments were addressed. However, I still suggest to give time for @shafik to respond before landing this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132918/new/ https://reviews.llvm.org/D132918 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
