[PATCH] D24686: [support] Some improvements to StringSwitch

Chandler Carruth via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 3 01:04:24 PDT 2016


chandlerc added a comment.

Could you split this appart? I think it will be much easier to review.

Suggested split and ordering:

1. Add basic unit tests. Should be super easy patch.
2. Switch to variadic templates with associated magic. Should have a nice simple unit test that showcases what is going on with this patch. And the unit tests from #1 should clearly show that it doesn't break things.
3. Add the case insensitive case methods.
4. Add the variant of `Default`. However, I wouldn't use `Error` and `Expected` here. Those seem both heavyweight and indicative of an *error* as opposed to just detecting whether or not the switch fell off the end. I feel like `Optional` would be a better fit?

Clearly #3 and #4 can be done in parallel. Happy to discuss any high level issues here to avoid re-writing stuff again and again in rebased patches just to change the underlying design.


https://reviews.llvm.org/D24686





More information about the llvm-commits mailing list