[PATCH] D155441: [ADT] Remove SFINAE constraint from llvm::iterator_range ctor for gcc-7

[Balázs Benics via Phabricator via llvm-commits]

steakhal added a comment.

In D155441#4508583 <https://reviews.llvm.org/D155441#4508583>, @immolo wrote:

> In D155441#4508400 <https://reviews.llvm.org/D155441#4508400>, @crobeck wrote:
>
>> In D155441#4507897 <https://reviews.llvm.org/D155441#4507897>, @steakhal wrote:
>>
>>> In D155441#4507877 <https://reviews.llvm.org/D155441#4507877>, @MaskRay wrote:
>>>
>>>>> Allegedly, GCC-8 and above builds just fine.
>>>>
>>>> It would be nice to test this:)
>>>
>>> I'll check that tomorrow.
>>
>> FWIW, the lit checks were failing for me with with GCC 8.3 so that GCC version check may need to be more inclusive.
>
> For some added information I used gcc-8.5 for this test.

I tested it this morning.
gcc-8 (Ubuntu 8.4.0-3ubuntu2) 8.4.0
gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
All these builds this just fine.

In D155441#4509003 <https://reviews.llvm.org/D155441#4509003>, @mehdi_amini wrote:

> Can we land this quickly? I'd like to restore the gcc7 build and I'll change my bot to target it.

Pushed. Thank you all for the reviews!



================
Comment at: llvm/include/llvm/ADT/iterator_range.h:47
+#if __GNUC__ == 7
+  // Be careful no to break gcc-7 on the mlir target.
+  // See https://github.com/llvm/llvm-project/issues/63843
----------------
MaskRay wrote:
> steakhal wrote:
> > MaskRay wrote:
> > > If you know what things are specifically broken, better to use something like:
> > > 
> > > Work around a bug that ...
> > The thing is that usually I don't write generic code. So I gave up after 4 hours, thus I don't really know why it doesn't work for gcc-8.
> > So I cannot be more specific.
> > I had a quick look at gcc-8 release notes, but I couldn't find anything related to templates.
> immolo from Gentoo reports a build success with GCC 8, so only GCC 7 needs the workaround.
> 
> ... and reported a typo (no => not): "Be careful not to break gcc-7 on the mlir target."
AH, I just noticed now. If you don't mind, I won't fix that typo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155441