barannikov88 added inline comments.
================
Comment at: llvm/include/llvm/Support/CodeGen.h:57
+ /// Code generation optimization level.
+ enum Level : IDType {
+ None = 0, ///< -O0
----------------
scott.linder wrote:
> barannikov88 wrote:
> > arsenm wrote:
> > > scott.linder wrote:
> > > > This is ABI breaking, so maybe we don't want/need to define the
> > > > underlying type?
> > > This isn't in the C API, and this isn't for a release branch so I don't
> > > think it matters
> > Why did you need to restrict the underlying type?
> >
> There is no strict need, it just:
>
> * Avoids potential UB (casting an out-of-range value to an enumeration type
> //without a fixed underlying type// is undefined); in practice there is
> plenty of UB in LLVM, and a bug here wouldn't be a particularly pernicious
> kind of UB anyway.
> * Means users of the type might pack better (and we won't run out of 255 opt
> levels anytime soon).
>
> I originally intended to change this to an `enum class` rather than a `class`
> in a `namespace`, but that is slightly more disruptive and should probably be
> done for every other enumeration defined here at the same time.
Thanks for the explanation.
> Means users of the type might pack better (and we won't run out of 255 opt
> levels anytime soon).
The downside is that `int` is usually more efficient.
> Avoids potential UB [...]
cppreference [[ https://en.cppreference.com/w/cpp/language/enum | says ]]
```
Values of unscoped enumeration type are implicitly-convertible to integral
types. If the underlying type is not fixed, the value is convertible to the
first type from the following list able to hold their entire value range: int,
unsigned int, ...
```
so casting to int should be safe.
Anyways, this is just a nit, feel free to ignore.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D141968/new/
https://reviews.llvm.org/D141968
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
