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
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
