[PATCH] D54356: Convert CheckICE into a statment visitor

Richard Smith - zygoloid via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 13 12:59:22 PST 2018


rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D54356#1297506, @void wrote:

> This code is called in over 90 places, so it's hard to tell if they all are in a constant context. Though I suppose that what this code is supposed to check for would work the same in a constant context as without one. I can revert this if you want, but to be honest the original function was terrible--it's huge and hard to understand what's going on. As for adding new expressions, this isn't the only place where a `StmtVisitor` is used. One still needs to go through all of those visitors to see if they need to add their expression.


Thinking about this some more: in the case where adding a new `Stmt` node without considering this code is likely to result in a silent and initially-unnoticed bug, I think it's useful to use one of our covered-switch-like patterns. But I don't think this actually is such a case; the C ICE rules are pretty conservative in what they allow, and new `Stmt` nodes should nearly always be treated as non-ICE. Thank you!


Repository:
  rC Clang

https://reviews.llvm.org/D54356





More information about the cfe-commits mailing list