[PATCH] D134020: [clang][Interp] Handle enums

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added inline comments.


================
Comment at: clang/test/AST/Interp/enums.cpp:25
+  SIX = FIVE + 2,
+
+};
----------------
aaron.ballman wrote:
> tbaeder wrote:
> > aaron.ballman wrote:
> > > tbaeder wrote:
> > > > aaron.ballman wrote:
> > > > > tbaeder wrote:
> > > > > > shafik wrote:
> > > > > > > Maybe some edge case values for enumerators like `__INT_MAX__  *2U +1U` (UINT_MAX) 
> > > > > > > 
> > > > > > > and 
> > > > > > > 
> > > > > > > ```
> > > > > > > enum E { // warning: enumeration values exceed range of largest integer [-Wenum-too-large]
> > > > > > >   E1 = -__LONG_MAX__ -1L,
> > > > > > >   E2 = __LONG_MAX__ *2UL+1UL 
> > > > > > > };
> > > > > > > ```
> > > > > > Hm, looks like that test broke one of the windows builders: https://lab.llvm.org/buildbot/#/builders/123/builds/13424 - are enums larger by default on Windows? What do you suggest to fix the test?
> > > > > If we're trying to be compatible with MSVC, we use their rules for picking the underlying type of an enumeration is which not fixed. One way to handle this is to add more RUN lines with explicit triples, but I think we lose too much interesting coverage that way. I'd probably use a `#ifndef _MSC_VER` block to control the expected diagnostics with a comment as to why the diagnostic is not expected on Windows.
> > > > The second builder that broke was  a hexagon builder: https://lab.llvm.org/buildbot/#/builders/38/builds/6231 - this would still fail with the `_MSC_VER` change, right?
> > > Nope, drat.
> > > 
> > > I would recommend breaking that specific test out into a separate file where we can add various RUN lines with triples, and the rest of the test can remain targetless.
> > Shame, but alright. So, what triples should I test? :)
> The ones from the failing bots, and I'd say maybe `x86_64-pc-linux` and `i686-pc-linux` would be reasonable.
Heh, the warning doesn't happen with the i686 triple either (with the old interpreter). Is that expected?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134020