[PATCH] D26013: Change DWARF parser to use enumerations for DWARF tags, attributes and forms.

[Adrian Prantl via llvm-commits]

> On Oct 26, 2016, at 3:11 PM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> 
> 
> On Wed, Oct 26, 2016 at 3:08 PM Adrian Prantl <aprantl at apple.com <mailto:aprantl at apple.com>> wrote:
> aprantl added inline comments.
> 
> 
> ================
> Comment at: lib/DebugInfo/DWARF/DWARFFormValue.cpp:126
>      return (FC == FC_String);
> +  default:
> +    break;
> ----------------
> Here's an idea: How about adding a
> ```
> template<typename EnumType>
> llvm::Optional<EnumType>
> getULEB128AsEnum() {
>   // Decode ULEB.
>   uint64_t Val = ...; // what about larger values?
>   if (Val >= EnumType::getMinValue() && Val <= EnumType::getMaxValue())
>     return static_cast<EnumType>(Val);
>   return None;
> }
> ```
> ?
> 
> This way we could guarantee that on of the enums if it is passed as an enum is always well-formed. Then we would never need a default case in a switch over a dwarf enum and could benefit from the not-all-enum-cases-covered-by-switch warning.
> 
> Is that feasible?
> 
> There's no nice way to get the min/max values of an enum (well, not of the enumerators - of the representable range, yes - which is actually probably what we want, but then we still have the issue with switches, etc).
Something like this seems to at least compile:

enum class A : int { min, a0 = min, a1, a2, max = a2 };
enum class B : int { min, b0 = min, b1, b2, b3, max = b3 };

template <typename EnumType> EnumType cast(int i) {
  if (i >= (int)EnumType::min && i <= (int)EnumType::max)
    return static_cast<EnumType>(i);
  return static_cast<EnumType>(0);
}

void f(int i) {
  auto a = cast<A>(i);
  auto b = cast<B>(i);
}

But, you are right, I completely forgot about holes in the enumerators (which exist in some places for backwards compatibility reasons) and the user space ranges.

> 
> If that makes sense, (see my reply for another way we might be able to deal with the switch-covered-default warning by adding the enumerator for the implementation defined range and just never specifying that value in a switch)
> 

That could work (save for the holes, but we could define placeholder enumerators there).

> - Dave
>  
> 
> 
> Repository:
>   rL LLVM
> 
> https://reviews.llvm.org/D26013 <https://reviews.llvm.org/D26013>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161026/cc21ea77/attachment.html>