[PATCH] D54356: Convert CheckICE into a statment visitor

void added a comment. In https://reviews.llvm.org/D54356#1297543, @rsmith wrote: > In https://reviews.llvm.org/D54356#1297522, @void wrote: > > > Okay. I'll revert this then. > > > I don't think we necessarily need the change in the other patch that's based > on this one, but I still think this

[PATCH] D54356: Convert CheckICE into a statment visitor

void abandoned this revision. void added a comment. This isn't necessary. We can assume it's in a constant context because it's checking for an ICE. Repository: rC Clang https://reviews.llvm.org/D54356 ___ cfe-commits mailing list cfe-commits@li

[PATCH] D54356: Convert CheckICE into a statment visitor

rsmith added a comment. In https://reviews.llvm.org/D54356#1297522, @void wrote: > Okay. I'll revert this then. I don't think we necessarily need the change in the other patch that's based on this one, but I still think this refactoring is an improvement :) Repository: rC Clang https://re

[PATCH] D54356: Convert CheckICE into a statment visitor

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

[PATCH] D54356: Convert CheckICE into a statment visitor

void added a comment. Okay. I'll revert this then. Repository: rC Clang https://reviews.llvm.org/D54356 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54356: Convert CheckICE into a statment visitor

rsmith added a comment. In https://reviews.llvm.org/D54356#1297470, @rsmith wrote: > Can you explain more about the justification for this? The code today has a > covered switch, which is useful for maintainability -- anyone adding a new > `Expr` node gets told they need to think about and upda

[PATCH] D54356: Convert CheckICE into a statment visitor

void added a comment. 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 or

[PATCH] D54356: Convert CheckICE into a statment visitor

rsmith added a comment. Can you explain more about the justification for this? The code today has a covered switch, which is useful for maintainability -- anyone adding a new `Expr` node gets told they need to think about and update this code. Are there any cases where we check for an ICE and a

[PATCH] D54356: Convert CheckICE into a statment visitor

void added a comment. Ping? Repository: rC Clang https://reviews.llvm.org/D54356 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54356: Convert CheckICE into a statment visitor

void created this revision. void added reviewers: rsmith, shafik. Herald added a subscriber: jfb. This cleans up the code somewhat and allows us conditionally to act on different types of nodes depending on their context. E.g., if we're checking for an ICE in a constant context. Repository: rC