Eric Fiselier via llvm-dev
2019-Mar-08 22:57 UTC
[llvm-dev] [cfe-dev] 8.0 Regression with __builtin_constant_p.
Hi Bill, I committed a fix already as r355743 [1], and it fixes `test2` as well. I believe we should merge this into 8.0. /Eric [1] https://github.com/llvm/llvm-project/commit/680e865c313a80b6ec329abde61e1f0c66bdc103 On Fri, Mar 8, 2019 at 5:46 PM Bill Wendling <isanbard at gmail.com> wrote:> Hi Eric, > > The attached patch may help matters, though it now fails because "test2" > doesn't exist (it's not enabled). > > -bw > > > > On Fri, Mar 8, 2019 at 11:38 AM Eric Fiselier via cfe-dev < > cfe-dev at lists.llvm.org> wrote: > >> Hi All, >> >> I know we're late in the release process, but I've discovered a >> regression I believe is serious enough to hold the release [1]. >> >> The regression is in usages of `__builtin_constant_p` applied to a >> dereferenced pointer in a constant expression. In certain cases Clang now >> rejects this as a non-constant expression [2]. >> >> This regression will have a larger impact than it initially appears. >> >> libstdc++ 7.1 and newer use `__builtin_constant_p` to implement C++17 >> `std::char_traits` [4]. This means that any usage of `std::string_view` in >> a constant expression has the possibility to break. `__builtin_constant_p` >> may be rare, but `std::string_view` is not. >> >> This regression was first found because it breaks `absl::StrFormat` [4]. >> This bug will break Abseil's LTS w/ libstdc++. Abseil cannot fully work >> around this bug. >> >> I would implore us to fix this bug in the upcoming 8 release. >> >> /Eric >> >> [1] https://reviews.llvm.org/D59038 >> [2] https://godbolt.org/z/VOCCJF >> [3] https://github.com/abseil/abseil-cpp/issues/271 >> [4] >> https://github.com/gcc-mirror/gcc/blob/gcc-8_1_0-release/libstdc++-v3/include/bits/char_traits.h#L229 >> _______________________________________________ >> cfe-dev mailing list >> cfe-dev at lists.llvm.org >> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev >> >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190308/1efab7c5/attachment.html>
Bill Wendling via llvm-dev
2019-Mar-08 23:07 UTC
[llvm-dev] [cfe-dev] 8.0 Regression with __builtin_constant_p.
Ah! Cool. I'll let the release manager make that decision. :-) On Fri, Mar 8, 2019 at 2:57 PM Eric Fiselier <eric at efcs.ca> wrote:> Hi Bill, > > I committed a fix already as r355743 [1], and it fixes `test2` as well. > > I believe we should merge this into 8.0. > > /Eric > > [1] > https://github.com/llvm/llvm-project/commit/680e865c313a80b6ec329abde61e1f0c66bdc103 > > On Fri, Mar 8, 2019 at 5:46 PM Bill Wendling <isanbard at gmail.com> wrote: > >> Hi Eric, >> >> The attached patch may help matters, though it now fails because "test2" >> doesn't exist (it's not enabled). >> >> -bw >> >> >> >> On Fri, Mar 8, 2019 at 11:38 AM Eric Fiselier via cfe-dev < >> cfe-dev at lists.llvm.org> wrote: >> >>> Hi All, >>> >>> I know we're late in the release process, but I've discovered a >>> regression I believe is serious enough to hold the release [1]. >>> >>> The regression is in usages of `__builtin_constant_p` applied to a >>> dereferenced pointer in a constant expression. In certain cases Clang now >>> rejects this as a non-constant expression [2]. >>> >>> This regression will have a larger impact than it initially appears. >>> >>> libstdc++ 7.1 and newer use `__builtin_constant_p` to implement C++17 >>> `std::char_traits` [4]. This means that any usage of `std::string_view` in >>> a constant expression has the possibility to break. `__builtin_constant_p` >>> may be rare, but `std::string_view` is not. >>> >>> This regression was first found because it breaks `absl::StrFormat` [4]. >>> This bug will break Abseil's LTS w/ libstdc++. Abseil cannot fully work >>> around this bug. >>> >>> I would implore us to fix this bug in the upcoming 8 release. >>> >>> /Eric >>> >>> [1] https://reviews.llvm.org/D59038 >>> [2] https://godbolt.org/z/VOCCJF >>> [3] https://github.com/abseil/abseil-cpp/issues/271 >>> [4] >>> https://github.com/gcc-mirror/gcc/blob/gcc-8_1_0-release/libstdc++-v3/include/bits/char_traits.h#L229 >>> _______________________________________________ >>> cfe-dev mailing list >>> cfe-dev at lists.llvm.org >>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev >>> >>-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190308/d7037891/attachment.html>
Hans Wennborg via llvm-dev
2019-Mar-09 13:39 UTC
[llvm-dev] [cfe-dev] 8.0 Regression with __builtin_constant_p.
Okay, sounds like this is important enough, and now that there's a fix committed I'll make sure it gets into the release. Let's have it bake in trunk a little bit first though. On Sat, Mar 9, 2019 at 12:08 AM Bill Wendling via llvm-dev <llvm-dev at lists.llvm.org> wrote:> > Ah! Cool. I'll let the release manager make that decision. :-) > > On Fri, Mar 8, 2019 at 2:57 PM Eric Fiselier <eric at efcs.ca> wrote: >> >> Hi Bill, >> >> I committed a fix already as r355743 [1], and it fixes `test2` as well. >> >> I believe we should merge this into 8.0. >> >> /Eric >> >> [1] https://github.com/llvm/llvm-project/commit/680e865c313a80b6ec329abde61e1f0c66bdc103 >> >> On Fri, Mar 8, 2019 at 5:46 PM Bill Wendling <isanbard at gmail.com> wrote: >>> >>> Hi Eric, >>> >>> The attached patch may help matters, though it now fails because "test2" doesn't exist (it's not enabled). >>> >>> -bw >>> >>> >>> >>> On Fri, Mar 8, 2019 at 11:38 AM Eric Fiselier via cfe-dev <cfe-dev at lists.llvm.org> wrote: >>>> >>>> Hi All, >>>> >>>> I know we're late in the release process, but I've discovered a regression I believe is serious enough to hold the release [1]. >>>> >>>> The regression is in usages of `__builtin_constant_p` applied to a dereferenced pointer in a constant expression. In certain cases Clang now rejects this as a non-constant expression [2]. >>>> >>>> This regression will have a larger impact than it initially appears. >>>> >>>> libstdc++ 7.1 and newer use `__builtin_constant_p` to implement C++17 `std::char_traits` [4]. This means that any usage of `std::string_view` in a constant expression has the possibility to break. `__builtin_constant_p` may be rare, but `std::string_view` is not. >>>> >>>> This regression was first found because it breaks `absl::StrFormat` [4]. This bug will break Abseil's LTS w/ libstdc++. Abseil cannot fully work around this bug. >>>> >>>> I would implore us to fix this bug in the upcoming 8 release. >>>> >>>> /Eric >>>> >>>> [1] https://reviews.llvm.org/D59038 >>>> [2] https://godbolt.org/z/VOCCJF >>>> [3] https://github.com/abseil/abseil-cpp/issues/271 >>>> [4] https://github.com/gcc-mirror/gcc/blob/gcc-8_1_0-release/libstdc++-v3/include/bits/char_traits.h#L229 >>>> _______________________________________________ >>>> cfe-dev mailing list >>>> cfe-dev at lists.llvm.org >>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev