This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [Patch] [libstdc++] [constexpr] use macros in place of 'constexpr' to allow for disabling constexpr until fully supported by compiler


On Thu, February 17, 2011 6:49 pm, Jonathan Wakely wrote:
> On 17 February 2011 16:11, Paolo Carlini wrote:
>> On 02/17/2011 03:06 PM, Adam Butcher wrote:
>>>
>>> The attached patch replaces occurrences of the plain word 'constexpr'
>>> with either the symbol _GLIBCXX_CONSTEXPR or the symbol
>>> _GLIBCXX_USE_CONSTEXPR, depending on whether the disabled replacement
>>> should be '' or 'const' respectively.
>>>
>>> This allows for disabling constexpr usage by the standard library in
>>> C++0x mode until issues such as
>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47774 /
>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46472 are resolved in the
>>> compiler.
>>>
>> Please always send library patches to the libstdc++ mailing list too
>> (and of course, at some point you will need a Copyright Assignment on
>> file...)
Sure no probs, will CC to libstdc++ if I make future library patches.
(PS. I already have copyright assignment on file for gcc but it seem moot
for this anyway)

>> Anyway, about the technical point: currently the macros are just an
>> implementation detail, are *not* meant to be changed at will [by]
>> the user
Ah ok.  I only intended this as a workaround rather than a solution --
the solution is to fix the above compiler bugs.

>> (among other things, note that they appear in *.cc files too...).
>>
It is a moot point but I think in the case of constexpr vs static const
this should be okay as differences could safely be translation unit
specific.  It should work as if one unit was built in C++03 mode and one
in C++0x which is supported by libstdc++ I think -- the patch would just
allow that a unit could be built in C++0x mode but with constexpr
disabled in the library.

>> Anyway, if we are going to do what you are suggesting we have to do it
>> consistently everywhere and document it.
>>
Certainly.  However, the consensus of this thread, which I agree with,
seems to be that a library-based 'workaround' is ugly.  It would be
preferable to fix the compiler; though if it turns out to be too
difficult within 4.6 timescales then maybe user-configurable
_GLIBCXX_DISABLE_CONSTEXPR or some such might be an option.  But, of
course, applied consistently.

>> At the moment, I don't know *exactly* how serious are the front-end
>> issues you mentioned above (in the specific sense: how much C++03 code
>> using <limits>, <complex>, etc, which we care about, would break in
>> C++0x mode only because of the bugs), or if in principle could be
>> fixed in time for 4.6.0.
>>
>> Missing further clarifications, I vote against the library-only
>> changes.
>>
>
> I don't much like the idea of patching the library and having to ship
> those changes in 4.6.0 to workaround that bug (I think the two PRs are
> the same issue.)  Especially as neither of those PRs shows code which
> is affected by using constexpr in the library (I'm sure such code
> exists, but the testcases provided don't show a library-related
> problem.)
>
I didn't like the idea of patching the library either.  I assumed I
could workaround the compiler issue by specifying
   -D_GLIBCXX_CONSTEXPR= -D_GLIBCXX_USE_CONSTEXPR=const
when building our software in C++0x mode.  This worked with some files
that used std::pair only but broke on files that used limits/complex
due to constexpr vs non-constexpr mismatches.  It was then that I
changed the library to 'uniformly' use the preprocessor symbols rather
than use the plain word constexpr in C++0x blocks.

> Adam, if the constexpr additions to the library have broken code that
> worked with GCC 4.5 please provide a small testcase showing the
> problem (your self-executing script is clever and comprehensive but
> not very easy to parse quickly to identify the issue!)
>
I don't believe this to be a library issue (the library code looks
conforming to me) but I guess it could be considered as premature
usage of a partially completed feature.  My issue was primarily with
std::pair as I quoted in bug 47774:
>>> I originally came across this when attempting to instantiate a
>>> std::array<std::pair<F,S>,N> where objects of either F or S were
>>> not usable as constants; pair's default constructor is specified
>>> as constexpr yielding the same problem)

The code quoted in
  http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47774#c0
is a reduced test case illustrating the same bug.  Since I was
submitting a compiler bug I avoided any library code and attempted to
reduce the problem to a minimal example.

The attachment on the PR goes into more detail with various
combinations of declarations and member types which I thought might
help deduce the cause.  The issue seems to be due to constexpr
declarations in templates not 'devolving' to simple non-constexpr
declarations if the requirements for constexpr are not met for a
particular instantiation (7.1.5.4).  Std::pair's constructor exhibits
this problem if either template argument causes a break of constexpr
requirements.

> If you can show a valid program (possibly using <limits> or <complex>,
> ideally one which is valid in C++03 and C++0x) which compiles using
> -std=c++0x with GCC 4.5 but not with 4.6 then the bugs can be marked
> as regressions, which means they're more likely to be fixed for 4.6
>
> If you have code examples which don't explicitly use constexpr, but
> break because libstdc++ code you make use of has been changed to use
> constexpr, that's more likely to get given a higher priority by the
> 4.6 release managers.
>
Marc's latest example, added to bug 46472, demonstrates the problem
from the library/end-user point-of-view.

Since 46472 has now been confirmed as a regression hopefully the issue
will be resolved for 4.6 and no library hacks will be necessary.

FWIW, the patch to disable constexpr in [some parts of] the library
has allowed a full build of our code base with 4.6 in C++0x mode so
I'll keep it in my GCC build for now.

Thanks for the feedback.
Regards,
Adam



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]