aaron.ballman added inline comments.

================
Comment at: clang/test/AST/Interp/enums.cpp:25
+  SIX = FIVE + 2,
+
+};
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134020

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

Reply via email to