jgorbe added inline comments.

================
Comment at: lldb/include/lldb/lldb-enumerations.h:841
+
+  kNumFormatterMatchTypes,
+};
----------------
labath wrote:
> I see the enums in this file use wildly inconsistent styles for the "largest 
> value" enumerator. However, you've picked the one I like the least, because 
> this tends to produce `unhandled switch case "kNumFormatterMatchTypes"` 
> warnings. If you feel like you need to have a sentinel, I'd recommend going 
> with the `eLastFormatterMatchType = <last enum value>` pattern (used e.g. in 
> the StateType enum).
I //do// want a sentinel because the plan is to replace some hardcoded 
instances of "check for an exact match and if nothing is found then check for a 
regex match" with a class that has an array of size `kNumFormatterMatchTypes` 
formatter containers, and knows how to do a lookup in priority order. So then I 
can add another "priority tier" for callback matchers and not add extra logic 
in a bunch of places.

But I have no strong preference between having a `std::array<Container, 
kNumFormatterMatchTypes>` and a `std::array<Container, eLastFormatterMatchType 
+ 1>`, and I agree the `unhandled switch case` warnings are annoying. I'll 
change it shortly.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133240/new/

https://reviews.llvm.org/D133240

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to