jyknight added a comment.

In http://reviews.llvm.org/D18876#402855, @jfb wrote:

> In http://reviews.llvm.org/D18876#399934, @jyknight wrote:
>
> > The large amount of casting to/from integers for AtomicOrderingCABI makes 
> > me think that it probably ought not actually be converted to an enum class 
> > after all.
>
>
> Untrusted user input with enums is a problem: you have to range-check before 
> casting the int to the enum, with or without enum class, otherwise 
> out-of-range is UB. I like it being explicit, but yeah what I had was wordy 
> so I factored out the check.
>
> Casting *out* of the enum to generate constants is also wordy, but I think 
> that's also fine. We could add a version of `ConstantInt` which takes in 
> `is_integral_or_enum` but that seems like a lot of work for little typing? 
> I'm happy to do that if you think it's worthwhile.


Well, my suggestion had been to just leave it as a non-enum-class, like it was 
before -- with no casts to or from the enum type. I think the code as it was 
before was actually safe, too, wasn't it?

I'm just not really sure using strong enums for the CABI type is really buying 
much, since the basically only point of the C ABI kind is to use it as an 
integer. Essentially every uses of it will be converting to/from integers, 
won't it?

I don't really feel strongly though, so if you want to go ahead, I'm okay with 
that.


http://reviews.llvm.org/D18876



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

Reply via email to