Re: [PATCH] libstdc++: Compile std::allocator instantiations as C++20
On Thu, 11 Apr 2024 at 20:22, Jonathan Wakely wrote: > > I'm considering this late patch for gcc-14 to workaround an issue > discovered by a recent Clang change. > > I'm not yet sure if Clang is right to require these symbols. It's not > really clear, because always_inline isn't part of the standard so it's > not clear how it should interact with explicit instantiations and > modules. Exporting these four extra symbols doesn't hurt, even if Clang > ends up reverting or revising its change that requires them. > > Another way to fix it would be to suppress the explicit instantiation > declarations in for C++20, so that the compiler > always instantiates them implicitly as needed. We do similar things for > the explicit instantiations of std::string etc. so that new member > functions that aren't in the .so are implicitly instantiated as needed. > > That would look like this instead: > > --- a/libstdc++-v3/include/bits/allocator.h > +++ b/libstdc++-v3/include/bits/allocator.h > @@ -281,7 +281,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > >// Inhibit implicit instantiations for required instantiations, >// which are defined via explicit instantiations elsewhere. > -#if _GLIBCXX_EXTERN_TEMPLATE > +#if _GLIBCXX_EXTERN_TEMPLATE && __cplusplus <= 201703L >extern template class allocator; >extern template class allocator; > #endif > > But we might want to export the new functions from the library > eventually anyway, so doing it now (before Clang 19 is released) might > be the best option. > > Thoughts? I think the symbol export is a fine solution. Both of these solutions work, so I don't have a strong preference, I have a minor preference for not suppressing explicit instantiations that are otherwise already there, but that is indeed not a strong preference.
Re: [PATCH] libstdc++: Implement P2255R2 dangling checks for std::tuple [PR108822]
On Fri, 12 Jan 2024 at 00:16, Jonathan Wakely wrote: > > I'd like to commit this to trunk for GCC 14. Please take a look. Without looking at it in excruciating detail, it's pretty much along the lines of what I have always envisioned to be a powerful combination of concepts and if-constexpr. My general principle on this is "looks like an improvement, so if it passes all the tests, ship it". :) Sure, I have envisioned going even further with that combination, such as significantly reducing the number of overloads and doing more of it as an if-constexpr ladder, but there's a balance where emulating the effects of overload resolution in something like that can become such a burden that the benefits are no longer there. If the field were green, I'd consider that as the approach from the get-go when initially designing a type like tuple, instead of doing it as an overload set.
Re: [PATCH 2/2] libstdc++: Implement P2278R4 "cbegin should always return a constant iterator"
On Fri, 14 Apr 2023 at 07:03, Patrick Palka via Libstdc++ wrote: > > This also implements the approved follow-up LWG issues 3765, 3766, 3769, > 3770, 3811, 3850, 3853, 3862 and 3872. > > Tested on x86_64-pc-linux-gnu, does this look OK for trunk? Hooray! THANK YOU! No comments on the patch itself, Jonathan will review it. :)
Re: [PATCH] libstdc++: use __bool_constant instead of integral_constant
On Thu, 23 Mar 2023 at 12:53, Ken Matsui wrote: > > DCO sign-off is indeed more light-weight, and sure, it's becoming more > > common > > since it's relatively new as an option. > > Thank you! > > To add a DCO sign-off, do I need to bump up the subject line to [PATCH v2]? No. The format of the subject for patch submission emails is not that strict. :)
Re: [PATCH] libstdc++: use __bool_constant instead of integral_constant
On Thu, 23 Mar 2023 at 12:18, Ken Matsui via Libstdc++ wrote: > > Thank you so much for your review! > > This is my first time contributing to GCC, so I do not have a GCC > copyright assignment. I googled those two ways, but I am still > confused... Is it correct that the DCO sign-off has been getting more > common recently? If so, I will put the sign-off into all my patches. I > would prefer to choose the more common and lightweight way. DCO sign-off is indeed more light-weight, and sure, it's becoming more common since it's relatively new as an option.
Re: [committed] libstdc++: Improve performance of chrono::utc_clock::now()
On Thu, 17 Nov 2022 at 11:57, Daniel Krügler via Libstdc++ wrote: > > Do you really want me to stop working on the missing time zone support to > > test and commit that change? > > I do not. I was reviewing and hoping to make a useful comment. Looks like someone's crunching to make a stage3 deadline, he gets a bit testy during those times. :) He'll return to his normal jolly self soon, don't worry, Daniel. :)
Re: [PATCH] c++: Add __reference_con{struc, ver}ts_from_temporary [PR104477]
Well, is_xible is not is_xible_p because it doesn't need to be both is_*
and *_p. But xes_from_temporary is less obviously a question, so
xes_from_temporary_p would imho be a better name.
On Fri, Jul 15, 2022, 18:33 Marek Polacek via Libstdc++ <
libstd...@gcc.gnu.org> wrote:
> On Thu, Jul 14, 2022 at 11:48:51PM -0400, Jason Merrill wrote:
> > On 7/14/22 13:43, Marek Polacek wrote:
> > > On Tue, Jul 12, 2022 at 04:15:00PM -0400, Jason Merrill wrote:
> > > > On 7/12/22 16:10, Jason Merrill wrote:
> > > > > On 7/8/22 13:41, Marek Polacek wrote:
> > > > > > This patch implements C++23 P2255R2, which adds two new type
> traits to
> > > > > > detect reference binding to a temporary. They can be used to
> detect code
> > > > > > like
> > > > > >
> > > > > > std::tuple t("meow");
> > > > > >
> > > > > > which is incorrect because it always creates a dangling
> reference,
> > > > > > because
> > > > > > the std::string temporary is created inside the selected
> constructor of
> > > > > > std::tuple, and not outside it.
> > > > > >
> > > > > > There are two new compiler builtins,
> > > > > > __reference_constructs_from_temporary
> > > > > > and __reference_converts_from_temporary. The former is used to
> simulate
> > > > > > direct- and the latter copy-initialization context. But I had a
> > > > > > hard time
> > > > > > finding a test where there's actually a difference. Under DR
> 2267, both
> > > > > > of these are invalid:
> > > > > >
> > > > > > struct A { } a;
> > > > > > struct B { explicit B(const A&); };
> > > > > > const B {a};
> > > > > > const B (a);
> > > > > >
> > > > > > so I had to peruse [over.match.ref], and eventually realized
> that the
> > > > > > difference can be seen here:
> > > > > >
> > > > > > struct G {
> > > > > > operator int(); // #1
> > > > > > explicit operator int&&(); // #2
> > > > > > };
> > > > > >
> > > > > > int&& r1(G{}); // use #2 (no temporary)
> > > > > > int&& r2 = G{}; // use #1 (a temporary is created to be bound to
> int&&)
> > > > > >
> > > > > > The implementation itself was rather straightforward because we
> already
> > > > > > have conv_binds_ref_to_prvalue. The main function here is
> > > > > > reference_from_temporary. The renaming to
> ref_conv_binds_to_temporary_p
> > > > > > is because previously the function didn't distinguish between an
> invalid
> > > > > > conversion and one that binds to a prvalue.
> > > > > >
> > > > > > The patch also adds the relevant class and variable templates to
> > > > > > .
> > > > > >
> > > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > > > > >
> > > > > > PR c++/104477
> > > > > >
> > > > > > gcc/c-family/ChangeLog:
> > > > > >
> > > > > > * c-common.cc (c_common_reswords): Add
> > > > > > __reference_constructs_from_temporary and
> > > > > > __reference_converts_from_temporary.
> > > > > > * c-common.h (enum rid): Add
> RID_REF_CONSTRUCTS_FROM_TEMPORARY and
> > > > > > RID_REF_CONVERTS_FROM_TEMPORARY.
> > > > > >
> > > > > > gcc/cp/ChangeLog:
> > > > > >
> > > > > > * call.cc (ref_conv_binds_directly_p): Rename to ...
> > > > > > (ref_conv_binds_to_temporary_p): ... this. Add a new bool
> > > > > > parameter. Return true only if the conversion is valid and
> > > > > > conv_binds_ref_to_prvalue returns true.
> > > > > > * constraint.cc (diagnose_trait_expr): Handle
> > > > > > CPTK_REF_CONSTRUCTS_FROM_TEMPORARY and
> > > > > > CPTK_REF_CONVERTS_FROM_TEMPORARY.
> > > > > > * cp-tree.h (enum cp_trait_kind): Add
> > > > > > CPTK_REF_CONSTRUCTS_FROM_TEMPORARY
> > > > > > and CPTK_REF_CONVERTS_FROM_TEMPORARY.
> > > > > > (ref_conv_binds_directly_p): Rename to ...
> > > > > > (ref_conv_binds_to_temporary_p): ... this.
> > > > > > (reference_from_temporary): Declare.
> > > > > > * cxx-pretty-print.cc (pp_cxx_trait_expression): Handle
> > > > > > CPTK_REF_CONSTRUCTS_FROM_TEMPORARY and
> > > > > > CPTK_REF_CONVERTS_FROM_TEMPORARY.
> > > > > > * method.cc (reference_from_temporary): New.
> > > > > > * parser.cc (cp_parser_primary_expression): Handle
> > > > > > RID_REF_CONSTRUCTS_FROM_TEMPORARY and
> > > > > > RID_REF_CONVERTS_FROM_TEMPORARY.
> > > > > > (cp_parser_trait_expr): Likewise.
> > > > > > (warn_for_range_copy): Adjust to call
> ref_conv_binds_to_temporary_p.
> > > > > > * semantics.cc (trait_expr_value): Handle
> > > > > > CPTK_REF_CONSTRUCTS_FROM_TEMPORARY and
> > > > > > CPTK_REF_CONVERTS_FROM_TEMPORARY.
> > > > > > (finish_trait_expr): Likewise.
> > > > > >
> > > > > > libstdc++-v3/ChangeLog:
> > > > > >
> > > > > > * include/std/type_traits
> (reference_constructs_from_temporary,
> > > > > > reference_converts_from_temporary): New class templates.
> > > > > > (reference_constructs_from_temporary_v,
> > > > > > reference_converts_from_temporary_v): New variable
> templates.
> > > > > >
Re: [PATCH] libstdc++: Allow std::condition_variable waits to be cancelled [PR103382]
On Wed, 8 Dec 2021 at 19:27, Jonathan Wakely via Libstdc++
wrote:
> After resolving a PEBKAC issue, here's an incremental diff that
> preserves the old behaviour for the existing @GLIBCXX_3.4.11 symbol,
> but adds a new @@GLIBCXX_3.4.30 symbol that supports cancellation via
> __forced_unwind.
>
> Maybe we should also do this in the implementation of the old noexcept
> function:
>
> __attribute__((used))
> void
> __nothrow_wait_cv::wait(std::unique_lock& lock) noexcept
> {
> int old;
> int err = pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, );
> this->condition_variable::wait(lock);
> if (!err && old != PTHREAD_CANCEL_DISABLE)
> pthread_setcancelstate(old, );
> }
>
> This would prevent cancellation from terminating a process if it uses
> the old symbol. So we'd have a new symbol that supports cancellation,
> and an old one that safely disables it.
That sounds good to me.
Also, I'm not sure it was pointed out, for the original: changing a
noexcept function to start throwing can leak exceptions
through other noexcept functions, hitting catch-blocks instead of
terminates, or terminates that occur much later
than intended. The compiler will assume that it doesn't need to set up
the LSDA in a noexcept function if everything
you call is noexcept, and then you don't have the LSDA that would
terminate right then and there. That's probably
a lesser problem for the thread cancellation exception than it would
be for some others, but it's a blood-curdling/chilling possibility
that we should just avoid. And you have done that, thanks for that.
Re: [PATCH] libstdc++: Define std::__is_constant_evaluated() for internal use
On Fri, 26 Nov 2021 at 14:29, Jonathan Wakely via Libstdc++ wrote: > > I've bored of having to do preprocessor checks before using > is_constant_evaluated, so I've come up with this approach. Anybody got a > better idea, or objections to this? None here, I like this improvement.
Re: [committed] libstdc++: Make std::jthread support pointers to member functions [PR 100612]
On Fri, 1 Oct 2021 at 23:19, Jonathan Wakely via Libstdc++ wrote: > > This adds a non-standard extension to support initializing a > std::jthread with a pointer to a member function that expects a > stop_token to be added to the arguments. That use case is not supported > by C++20, because the stop_token would get added as the first argument, > which is where the object argument needs to be to invoke a pointer to > member function. Yeah, and the use-case is supported by applying a wrapper that does the right kind of argument binding, like shown in the BZ. Why are we doing this?
Re: [PATCH] assert that deleting by pointer to base in unique_ptr does not cause UB
On Wed, 22 Sept 2021 at 20:49, Antony Polukhin wrote: > > ср, 22 сент. 2021 г. в 20:23, Ville Voutilainen : > > > > On Wed, 22 Sept 2021 at 20:09, Antony Polukhin via Libstdc++ > > wrote: > > > > > > std::unique_ptr allows construction from std::unique_ptr of derived > > > type as per [unique.ptr.single.asgn] and [unique.ptr.single.ctor]. If > > > std::default_delete is used with std::unique_ptr, then after such > > > construction a delete is called on a pointer to base. According to > > > [expr.delete] calling a delete on a non similar object without a > > > virtual destructor is an undefined behavior. > > > > > > This patch turns that undefined behavior into static assertions inside > > > std::unique_ptr. > > > > I don't understand the sizeof(_Tp) == sizeof(_Up) part in the > > static_assert. I fail to see how > > a same-size check suggests that the types are similar enough that a > > delete-expression works. > > I used the following logic: > [unique.ptr.single.*] sections have the constraint that > "unique_ptr::pointer is implicitly convertible to pointer". > There's already a static assert that T in unique_ptr is not void, > so U either has to be the same type T, or a type derived from T. If a > derived type adds members, then size changes and types are not similar > as the decompositions won't have the qualification-decompositions with > the same n. Right, but the delete-expression on a non-polymorphic type where the static type and the dynamic type are different is UB regardless of whether the derived type adds members.
Re: [PATCH] assert that deleting by pointer to base in unique_ptr does not cause UB
On Wed, 22 Sept 2021 at 20:09, Antony Polukhin via Libstdc++ wrote: > > std::unique_ptr allows construction from std::unique_ptr of derived > type as per [unique.ptr.single.asgn] and [unique.ptr.single.ctor]. If > std::default_delete is used with std::unique_ptr, then after such > construction a delete is called on a pointer to base. According to > [expr.delete] calling a delete on a non similar object without a > virtual destructor is an undefined behavior. > > This patch turns that undefined behavior into static assertions inside > std::unique_ptr. I don't understand the sizeof(_Tp) == sizeof(_Up) part in the static_assert. I fail to see how a same-size check suggests that the types are similar enough that a delete-expression works.
Re: [committed] libstdc++: Move attributes that follow requires-clauses [PR101782]
On Thu, 5 Aug 2021 at 17:21, Jonathan Wakely via Libstdc++ wrote: > > On 04/08/21 12:55 +0100, Jonathan Wakely wrote: > >This adds [[nodiscard]] throughout , as proposed by P2377R0 > >(with some minor corrections). > > > >The attribute is added for all modes from C++11 up, using > >[[__nodiscard__]] or _GLIBCXX_NODISCARD where C++17 [[nodiscard]] can't > >be used directly. > > This change causes errors when -fconcepts-ts is used. Fixed like so. But this makes the attribute appertain to the function type, not to the function. It's also ill-formed: "The attribute-token nodiscard may be applied to the declarator-id in a function declaration or to the declaration of a class or enumeration. " Your change makes nodiscard be applied to the function declaration, not to the declarator-id.
Re: [committed 2/2] libstdc++: Add [[nodiscard]] to sequence containers
On Thu, 5 Aug 2021 at 15:11, Christophe Lyon via Libstdc++ wrote: > > Hi Jonathan, > > On Wed, Aug 4, 2021 at 2:04 PM Jonathan Wakely via Gcc-patches < > gcc-patches@gcc.gnu.org> wrote: > > > On 04/08/21 12:56 +0100, Jonathan Wakely wrote: > > >... and container adaptors. > > > > > >This adds the [[nodiscard]] attribute to functions with no side-effects > > >for the sequence containers and their iterators, and the debug versions > > >of those containers, and the container adaptors, > > > > I don't plan to add any more [[nodiscard]] attributes for now, but > > these two commits should demonstrate how to do it for anybody who > > wants to contribute similar patches. > > > > I didn't add tests that verify we do actually warn on each of those > > functions, because there are hundreds of them, and I know they're > > working because I had to alter existing tests to not warn. > > > > > I've noticed a regression on aarch64/arm: > FAIL: g++.old-deja/g++.other/inline7.C -std=gnu++17 (test for excess > errors) > Excess errors: > /gcc/testsuite/g++.old-deja/g++.other/inline7.C:11:11: warning: ignoring > return value of 'std::__cxx11::list<_Tp, _Alloc>::size_type > std::__cxx11::list<_Tp, _Alloc>::size() const [with _Tp = int*; _Alloc = > std::allocator; std::__cxx11::list<_Tp, _Alloc>::size_type = long > unsigned int]', declared with attribute 'nodiscard' [-Wunused-result] > > FAIL: g++.old-deja/g++.other/inline7.C -std=gnu++2a (test for excess > errors) > Excess errors: > /gcc/testsuite/g++.old-deja/g++.other/inline7.C:11:11: warning: ignoring > return value of 'std::__cxx11::list<_Tp, _Alloc>::size_type > std::__cxx11::list<_Tp, _Alloc>::size() const [with _Tp = int*; _Alloc = > std::allocator; std::__cxx11::list<_Tp, _Alloc>::size_type = long > unsigned int]', declared with attribute 'nodiscard' [-Wunused-result] > > Not sure why you didn't see it? That can easily happen when running just the library tests, rather than all of them. :P
Re: A suggestion for going forward from the RMS/FSF debate
On Fri, 16 Apr 2021 at 13:13, Ville Voutilainen wrote: > > The actual suggestion is at the end; skip straight to it if you wish. ..please disregard, that was a send-o, should've have been sent to the patches mailing list.
A suggestion for going forward from the RMS/FSF debate
The actual suggestion is at the end; skip straight to it if you wish. >Im glad there are people like you on the project Eric, because you express exactly what a lot of people see - even if a minority of people chose to ignore it, >To a lot of "non americans", the events on here appear as nothing more than a power grab by a small minority of developers, abusing their position and american corporate ideologies to enact change, ignoring any one who dares question or disagree unless they fit into a clique they have built (and want to maintain by ostracizing people they deem unworthy), brandishing them jerks, trolls, toxic and other childish names. Im glad there are a few devs that can see this, but it feels like they are stepping on egg shells (despite the rhetoric about how well the people in said clique can communicate on technical matters). That's a) incorrect b) beside some rather important points. The "small minority of developers" you speak of sure seems to consist of developers who are not in the minority considering how much they _actually contribute_ to the project. Some of them don't need to perform a "power grab"; they already have all the power fathomable, by virtue of being maintainers and active developers. This whole discussion, again, at least to me, boils down to two things, actually three: 1) is the technical leadership of RMS/GNU/FSF useful for the project? Is it beneficial, or harmful? 2) is the PR/public-face position of RMS/FSF useful for the project? Is it beneficial, or harmful? 3) Who should make decisions related to that? The developers and maintainers, or people who are neither of those, but are certainly vocal in these discussions? On the first part, other people have touched on it already, but the fear of a dreaded non-free software vendor co-opting GCC as a library to a non-free project has resulted in GCC being unsuitable to be used as a library in free software projects. This approach alone made sure that the meteoric rise of LLVM happened; there are recorded statements from LLVM developers trying to talk about this to RMS, and the answer, as they phrased it, "wasn't useful", because RMS decided that GCC shouldn't be a library to make it harder to use it in conjunction with non-free programs. Congratulations, it remains hard to use in conjunction with free programs, and everybody who wants to do something like that looks at LLVM first. RMS made a lofty attempt to promote copyleft software for such purposes, and failed miserably, leading us into a situation where such problems are not solved with copyleft software, but with LLVM instead. On the second part, we can discuss whether the reasons for various people not wanting RMS/FSF to be the PR department of GCC developers are sound, or whether we agree with them, until the cows come home. But that doesn't matter. Bad PR is bad PR, and it seems strikingly simple to consider trying a PR department that doesn't have the baggage of the previous one. And if you ask me, *that* should be a choice of the developers and maintainers, and them alone. It's their work; they should have a say in who and what the public face of the work is to the outside world. Whether their choice is made because they live a pampered and cosseted life is very much secondary. I don't have to agree with every viewpoint of the people who have suggested that RMS shouldn't lead this project, or that this project shouldn't necessarily be tied to FSF any more. I don't even need to "accept" it. I don't consider it something that needs my approval or acceptance, I'm not a maintainer or a major contributor. However, I consider it something that needs even LESS acceptance or approval of ESR or Mr. Dimech or various other people. I happen to have Write-After-Approval permission for this project. They don't. Because they're not members of this project, they don't contribute code to it. Finally, with regards to there existing a power grab or a sinister corporation plot to take GCC away from being "accountable to its community": 1) that's just pure horseshit. The people wanting to disassociate the project from RMS and/or FSF worked on GCC before their current employment, and will work on GCC after their current employment. There is no power grab, and there is no sinister corporation plot, and that wouldn't work anyway due to the license and due to how the project _actually works_. 2) the project isn't accountable that way today, anyway. That concern is pure fantasy. So, I have a moderate suggestion, and I will fairly entertain it being a bold suggestion for some people: A) Detach GCC from FSF (and RMS), have it be hosted on non-gnu sites, make it a project separate from FSF, other than B) Don't drop the copyright assignment requirement, at least not yet. C) Run in that mode for a while, and see what happens. This allows re-attaching GCC to FSF, it that's later deemed like a good idea. The codebases, in case there actually are two, which might not even
Re: [PATCH 2/5] Atomic __platform_wait: accept any 32-bit type, not just int
On Wed, 3 Mar 2021 at 19:25, Jonathan Wakely via Libstdc++ wrote: > Oh, except that is_scalar is surprisingly expensive to instantiate > (its defined in a really expensive way) and since we control all uses I'll be more than happy to write you an __is_scalar for GCC 12. :P
Re: [PATCH 4/5] barrier: use int instead of unsigned char for the phase state
On Tue, 2 Mar 2021 at 00:21, Thiago Macieira wrote: > But the code I posted, if people are careful to use write like I did, would > allow us to have the experimental "we're not sure this is right" > implementation of atomic waits, latches, barriers and semaphores right now. The code assumes that as soon as __cplusplus bumps and a header is present, things are stable. I don't think that's a safe assumption to make. Furthermore, the second #if tries to check a feature-testing macro without including either the corresponding header or . That doesn't work. You need to either include and check a macro, or check that exists, then include and then check the macro. But other than that, sure, as QoI, vendors could already provide the standard macros with numbers that are lower than the standard ever specified. Going forward, if existing facilities are changed, this stops working because now you'd have to track the WPs to see which values are "vendor-bogus". I find it better to just change the macros whose facilities changed during a standard cycle, and in those cases bump the IS macro, that'll then work going forward. In the meanwhile, when vendors can, they could use the technique you describe, but that's barely worth proposing as an SG10 guideline because they would've needed to do it already, but didn't. :P
Re: [PATCH 4/5] barrier: use int instead of unsigned char for the phase state
On Mon, 1 Mar 2021 at 23:54, Thiago Macieira wrote: > But your suggestion does work. We don't need to apply them to all macros, only > those that are new in a given version, like __cpp_lib_atomic_wait or > __cpp_lib_latch in this case. Alternatively, implementations can set the macro > to a given value below the standard-required one (incl. 1) to indicate that > they haven't achieved stability. > > That would make my check: > > #if __cplusplus >= 202002L && __has_include() > # include > #endif > #if __cpp_lib_latch < 201907L > # error "Please upgrade your Standard Library" > #endif Well, this would be different. What I'm suggesting is not quite that; for any *new* facility, we'd make sure that its draft macro and the final IS macro are different, but the minimum value is the first draft version, not anything below it. I don't care too much, that approach and yours would work the same way. Things that already had an IS value for a macro and haven't changed since don't need to be changed. And we don't need to bump all values of existing facilities either, just for those that got changes, so some existing macros would be considered lost causes. Like the ones we're talking about, because the cats are already out of the bag. I'll write a paper. That won't help you now, but it gives us tools in the future.
Re: [PATCH 4/5] barrier: use int instead of unsigned char for the phase state
On Mon, 1 Mar 2021 at 21:44, Thiago Macieira wrote: > But you can see someone doing: > > #if __cplusplus >= 202002L && __has_include() > # include > #else > # error "Please upgrade your compiler & standard library" > #endif > > and using in their inline code. And as you say, if they then mix DSOs > compiled with different versions of GCC, one of them might be the old, > experimental and binary-incompatible code. Remember that before April 2024, > the most recent Ubuntu LTS will be 22.04, which will be released before GCC > 12, which means it will contain GCC 11. > > So, wholly aside from how we fix the inefficiencies, we must decide how to > deal with the ABI break. We can: > > a) not have an ABI break in the first place, by having the above code not >compile with GCC until we promise ABI compatibility > b) cause a non-silent ABI break > c) accept the silent ABI break > > I advocate for (a) and vehemently disagree with (c). Meanwhile, (b) can be > something like (untested!): I can't make the above code work, in any reasonable manner, because it's doing feature detection incorrectly. :) What I can imagine doing, however, is this: 1) a published IS always bumps feature-macro values (this won't help now, it's a future fix, we don't currently do this in WG21) 2) an implementation uses the pre-IS draft values before it deems itself stable 3) users who want stability should require the feature-testing macro to have at least the IS value 4) adventurous users can either use the macro without checking its value, or use at least the value that gives them whatever shiny toy they're interested in With that, we can keep allowing adventurous users to opt in early, and you have a portable way to detect that features are stable, for an implementation-defined definition of "stable".
Re: [PATCH 4/5] barrier: use int instead of unsigned char for the phase state
On Mon, 1 Mar 2021 at 20:09, Thiago Macieira via Libstdc++ wrote: > > On Sunday, 28 February 2021 07:05:47 PST Hans-Peter Nilsson wrote: > > On Fri, 26 Feb 2021, Thiago Macieira via Gcc-patches wrote: > > > ints can be used in futexes. chars can't. > > > > Shouldn't that be an atomic type instead of a bare int then? > > There are a couple of atomic_refs: > > using __atomic_phase_ref_t = std::__atomic_ref<__barrier_phase_t>; > using __atomic_phase_const_ref_t = std::__atomic_ref __barrier_phase_t>; > > And _M_phase, despite being non-atomic, is never accessed without the > atomic_ref, aside from the constructor. Both arrive() and wait() start off by > creating the atomic_ref. > > But I confess I don't understand this code sufficiently to say it is correct. > I'm simply saying that waiting on unsigned chars will not use a futex, at > least until https://lkml.org/lkml/2019/12/4/1373 is merged into the kernel. I do have a question about the intent/concern here, regardless of what your patch technically does. The ABI break _is_ your concern, and the "heisenbugs" you were worried about would in fact be caused by the ABI break? So if you embed these things in your QAtomicThing, the ABI break may mess it up(*)? Is that a correct understanding of the concern here? (*) And inlining might not help because users might compile a different std::atomic_thing in their program than what your DSO has been compiled with, and you may end up using mixtures.
[PATCH] libstdc++: Fix the test for rvalue stream extraction
libstdc++-v3/ChangeLog:
* testsuite/27_io/rvalue_streams.cc: Run the extraction to a char*
for C++17 and lower only.
diff --git a/libstdc++-v3/testsuite/27_io/rvalue_streams.cc b/libstdc++-v3/testsuite/27_io/rvalue_streams.cc
index ad4d11c7cf3..487aa4deedd 100644
--- a/libstdc++-v3/testsuite/27_io/rvalue_streams.cc
+++ b/libstdc++-v3/testsuite/27_io/rvalue_streams.cc
@@ -53,7 +53,9 @@ test02()
VERIFY( x.as_rvalue == true );
char arr[2];
+#if __cplusplus <= 201703L
std::istringstream("x") >> [0];
+#endif
std::istringstream("x") >> arr;
}
Add pretty-printing support for __is_nothrow_{assignable, constructible}. [PR98054]
OK for trunk if full testsuite passes? Should we consider having some sort
of test that catches such omissions?
2020-11-30 Ville Voutilainen
gcc/
PR c++/98054
* cp/cxx-pretty-print.c (pp_cxx_trait_expression):
Add support for __is_nothrow_{assignable,constructible}.
diff --git a/gcc/cp/cxx-pretty-print.c b/gcc/cp/cxx-pretty-print.c
index 058b9c2f4fc..1cdf0772a6b 100644
--- a/gcc/cp/cxx-pretty-print.c
+++ b/gcc/cp/cxx-pretty-print.c
@@ -2666,6 +2666,12 @@ pp_cxx_trait_expression (cxx_pretty_printer *pp, tree t)
case CPTK_IS_CONSTRUCTIBLE:
pp_cxx_ws_string (pp, "__is_constructible");
break;
+case CPTK_IS_NOTHROW_ASSIGNABLE:
+ pp_cxx_ws_string (pp, "__is_nothrow_assignable");
+ break;
+case CPTK_IS_NOTHROW_CONSTRUCTIBLE:
+ pp_cxx_ws_string (pp, "__is_nothrow_constructible");
+ break;
default:
gcc_unreachable ();
Re: [committed] libstdc++: Fix constraints on std::optional comparisons [PR 96269]
On Thu, 5 Nov 2020 at 21:52, Jonathan Wakely via Libstdc++ wrote: > > On 05/11/20 19:09 +, Jonathan Wakely wrote: > >The relational operators for std::optional were using the wrong types > >in the declval expressions used to constrain them. Instead of using > >const lvalues they were using non-const rvalues, which meant that a type > >might satisfy the constraints but then give an error when the function > >body was instantiated. > > > >libstdc++-v3/ChangeLog: > > > > PR libstdc++/96269 > > * include/std/optional (operator==, operator!=, operator<) > > (operator>, operator<=, operator>=): Fix types used in > > SFINAE constraints. > > * testsuite/20_util/optional/relops/96269.cc: New test. > > > >Tested powerpc64le-linux. Committed to trunk. > > When concepts are supported we can make the alias templates > __optional_eq_t et al use a requires-expression instead of SFINAE. > This is potentially faster to compile, given expected improvements > to C++20 compilers. > > I'm testing this patch. It concerns me that we'd have such conditional conceptifying just because it's possibly faster to compile. There's more types where we'd want to conditionally use concepts, but perhaps we want to think a bit more how to do that in our source code, rather than just make them preprocessor-conditionals in the same header. We might entertain conceptifying tuple, when concepts are available. That may end up being fairly verbose if it's done with preprocessor in . That's not to say that I'm objecting to this as such; I merely think we want to be a bit careful with conceptifying, and be rather instantly prepared to entertain doing it with a slightly different source code structure, which may involve splitting things across more files, which would then involve adding more headers that are installed.
Re: [committed] libstdc++: Allow Lemire's algorithm to be used in more cases
On Wed, 4 Nov 2020 at 10:46, Stephan Bergmann via Libstdc++
wrote:
> To me it looks like it boils down to disagreement between g++ and
> clang++ over
>
> > struct S { static constexpr int f() { return 0; } };
> > void f(S & s) { static_assert(s.f(), ""); }
>
> where I think Clang might be right in rejecting it based on [expr.const]
> "An expression e is a core constant expression unless [...] an
> id-expression that refers to a variable or data member of reference type
> unless the reference has a preceding initialization [...]"
There's more to it than that. It's a disagreement over [expr.ref]/1.
For a static
member call, gcc just plain doesn't evaluate the s in s.f(). But [expr.ref]/1
says it's evaluated, and since it's not a constant expression, clang rejects
it, and gcc accepts it. That's why your fix works; it removes the use
of the otherwise-mostly-ignored
object expression for a call to a static member function.
So, I think gcc is accepting-invalid here, and we should just apply
the fix you suggested.
Re: [PATCH] g++, libstdc++: implement __is_nothrow_{constructible, assignable}
On Sat, 24 Oct 2020 at 03:07, Marek Polacek wrote: > > Ha, we have the same thing in is_trivially_xible, so I'll drive-by > > change that one as well. > > Please. Thanks! The tree is also on a separated line in is_trivially_xible and is_nothrow_xible, but not in is_xible. What do we think about line-whitespace changes mixed with 'real' changes?
Re: [PATCH] g++, libstdc++: implement __is_nothrow_{constructible, assignable}
On Sat, 24 Oct 2020 at 03:00, Marek Polacek wrote: > > + tree expr; > > + expr = is_xible_helper (code, to, from, /*trivial*/false); > > tree expr = is_xible_helper (code, to, from, /*trivial*/false); > > would be nicer, otherwise the front-end changes look fine, thanks. Ha, we have the same thing in is_trivially_xible, so I'll drive-by change that one as well.
Re: [PATCH] g++, libstdc++: implement __is_nothrow_{constructible, assignable}
On Sat, 24 Oct 2020 at 02:32, Ville Voutilainen wrote: > * method.c (__is_nothrow_xible): New. ..and this is is_nothrow_xible, without leading underscores.
[PATCH] g++, libstdc++: implement __is_nothrow_{constructible, assignable}
Finishing testing on Linux-PPC64. Ok for trunk if tests pass?
2020-10-24 Ville Voutilainen
gcc/c-family/ChangeLog:
Implement __is_nothrow_{constructible,assignable}
* c-common.c (__is_nothrow_assignable): New.
(__is_nothrow_constructible): Likewise.
* c-common.h (RID_IS_NOTHROW_ASSIGNABLE): New.
(RID_IS_NOTHROW_CONSTRUCTIBLE): Likewise.
gcc/cp/ChangeLog:
Implement __is_nothrow_{constructible,assignable}
* cp-tree.h (CPTK_IS_NOTHROW_ASSIGNABLE): New.
(CPTK_IS_NOTHROW_CONSTRUCTIBLE): Likewise.
(is_nothrow_xible): Likewise.
* method.c (__is_nothrow_xible): New.
* parser.c (cp_parser_primary_expression): Handle the new RID_*.
(cp_parser_trait_expr): Likewise.
* semantics.c (trait_expr_value): Handle the new RID_*.
(finish_trait_expr): Likewise.
libstdc++-v3/ChangeLog:
Implement __is_nothrow_{constructible,assignable}
* include/std/type_traits (__is_nt_constructible_impl): Remove.
(__is_nothrow_constructible_impl): Adjust.
(is_nothrow_default_constructible): Likewise.
(__is_nt_assignable_impl): Remove.
(__is_nothrow_assignable_impl): Adjust.
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index e16ca3894bc..098a36e4d67 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -527,6 +527,8 @@ const struct c_common_resword c_common_reswords[] =
{ "while", RID_WHILE, 0 },
{ "__is_assignable", RID_IS_ASSIGNABLE, D_CXXONLY },
{ "__is_constructible", RID_IS_CONSTRUCTIBLE, D_CXXONLY },
+ { "__is_nothrow_assignable", RID_IS_NOTHROW_ASSIGNABLE, D_CXXONLY },
+ { "__is_nothrow_constructible", RID_IS_NOTHROW_CONSTRUCTIBLE, D_CXXONLY },
/* C++ transactional memory. */
{ "synchronized", RID_SYNCHRONIZED, D_CXX_OBJC | D_TRANSMEM },
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 3d96092a297..24a4a8e7fe3 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -176,6 +176,7 @@ enum rid
RID_IS_TRIVIALLY_COPYABLE,
RID_IS_UNION,RID_UNDERLYING_TYPE,
RID_IS_ASSIGNABLE, RID_IS_CONSTRUCTIBLE,
+ RID_IS_NOTHROW_ASSIGNABLE, RID_IS_NOTHROW_CONSTRUCTIBLE,
/* C++11 */
RID_CONSTEXPR, RID_DECLTYPE, RID_NOEXCEPT, RID_NULLPTR, RID_STATIC_ASSERT,
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 5c06ac3789e..1ce20989e13 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -1323,7 +1323,9 @@ enum cp_trait_kind
CPTK_IS_UNION,
CPTK_UNDERLYING_TYPE,
CPTK_IS_ASSIGNABLE,
- CPTK_IS_CONSTRUCTIBLE
+ CPTK_IS_CONSTRUCTIBLE,
+ CPTK_IS_NOTHROW_ASSIGNABLE,
+ CPTK_IS_NOTHROW_CONSTRUCTIBLE
};
/* The types that we are processing. */
@@ -6752,6 +6754,7 @@ extern void use_thunk(tree, bool);
extern bool trivial_fn_p (tree);
extern tree forward_parm (tree);
extern bool is_trivially_xible (enum tree_code, tree, tree);
+extern bool is_nothrow_xible (enum tree_code, tree, tree);
extern bool is_xible(enum tree_code, tree, tree);
extern tree get_defaulted_eh_spec (tree, tsubst_flags_t = tf_warning_or_error);
extern bool maybe_explain_implicit_delete (tree);
diff --git a/gcc/cp/method.c b/gcc/cp/method.c
index 6e4c5f7e83b..2ffc86cbd81 100644
--- a/gcc/cp/method.c
+++ b/gcc/cp/method.c
@@ -1933,6 +1933,21 @@ is_trivially_xible (enum tree_code code, tree to, tree from)
return !nt;
}
+/* Returns true iff TO is nothrow assignable (if CODE is MODIFY_EXPR) or
+ constructible (otherwise) from FROM, which is a single type for
+ assignment or a list of types for construction. */
+
+bool
+is_nothrow_xible (enum tree_code code, tree to, tree from)
+{
+ tree expr;
+ expr = is_xible_helper (code, to, from, /*trivial*/false);
+
+ if (expr == NULL_TREE || expr == error_mark_node)
+return false;
+ return expr_noexcept_p (expr, tf_none);
+}
+
/* Returns true iff TO is assignable (if CODE is MODIFY_EXPR) or
constructible (otherwise) from FROM, which is a single type for
assignment or a list of types for construction. */
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 7ec7d42773c..cce3d0a679e 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -5637,6 +5637,8 @@ cp_parser_primary_expression (cp_parser *parser,
case RID_IS_UNION:
case RID_IS_ASSIGNABLE:
case RID_IS_CONSTRUCTIBLE:
+ case RID_IS_NOTHROW_ASSIGNABLE:
+ case RID_IS_NOTHROW_CONSTRUCTIBLE:
return cp_parser_trait_expr (parser, token->keyword);
// C++ concepts
@@ -10501,6 +10503,14 @@ cp_parser_trait_expr (cp_parser* parser, enum rid keyword)
kind = CPTK_IS_CONSTRUCTIBLE;
variadic = true;
break;
+case RID_IS_NOTHROW_ASSIGNABLE:
+ kind = CPTK_IS_NOTHROW_ASSIGNABLE;
+ binary = true;
+ break;
+case RID_IS_NOTHROW_CONSTRUCTIBLE:
+ kind = CPTK_IS_NOTHROW_CONSTRUCTIBLE;
+ variadic = true;
+ break;
default:
gcc_unreachable ();
}
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
Re: libstdc++: Fix visitor return type diagnostics [PR97499]
On Fri, 16 Oct 2020 at 13:02, Jonathan Wakely wrote: > > On 16/10/20 10:26 +0300, Ville Voutilainen via Libstdc++ wrote: > >Tested on Linux-PPC64. I haven't tested this with clang yet, > >Jonathan, can you help with that? The previous implementation > >indeed made an if-constexpr branch invalid for all instantiations > >of that branch, this one doesn't - now we have just a dependent static_assert > >which is well-formed for correct visitors and ill-formed for incorrect > >visitors. > > All I checked is that can now be included using -std=c++17, > but that works with Clang. > > OK for trunk, thanks. The subject line is wrong, this is 97449, not 97499. Fixing that and pushing.
Re: [PATCH] libstdc++: Diagnose visitors with different return types [PR95904]
On Sat, 17 Oct 2020 at 20:30, Stephan Bergmann wrote:
> Clang (with -std=c++17/20) now complains about
>
> > include/c++/11.0.0/variant:1032:10: error: no matching constructor for
> > initialization of 'std::__nonesuch'
> > return __nonesuch{};
> >^ ~~
> > include/c++/11.0.0/type_traits:2953:5: note: candidate constructor not
> > viable: requires 1 argument, but 0 were provided
> > __nonesuch(__nonesuch const&) = delete;
> > ^
>
> upon #include . (And I think legitimately so, as __nonsuch is
> not dependent?)
This is https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97449, I'll
commit a fix today.
libstdc++: Fix visitor return type diagnostics [PR97499]
Tested on Linux-PPC64. I haven't tested this with clang yet,
Jonathan, can you help with that? The previous implementation
indeed made an if-constexpr branch invalid for all instantiations
of that branch, this one doesn't - now we have just a dependent static_assert
which is well-formed for correct visitors and ill-formed for incorrect
visitors.
2020-10-16 Ville Voutilainen
PR libstdc++/97449
* include/std/variant
(__gen_vtable_impl<>::_S_apply_single_alt):
Diagnose visitor return type mismatches here..
(__gen_vtable_impl::_S_apply):
..not here.
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index a29c5bf513b..17f8bcd638b 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -960,9 +960,13 @@ namespace __variant
}
else
{
- __element = __gen_vtable_impl<
+ auto __tmp_element = __gen_vtable_impl<
remove_reference_t,
std::index_sequence<__indices..., __index>>::_S_apply();
+ static_assert(is_same_v<_Tp, decltype(__tmp_element)>,
+ "std::visit requires the visitor to have the same "
+ "return type for all alternatives of a variant");
+ __element = __tmp_element;
}
}
};
@@ -1026,10 +1030,8 @@ namespace __variant
std::declval<_Variants>()...))>;
if constexpr (__visit_ret_type_mismatch)
{
- static_assert(!__visit_ret_type_mismatch,
- "std::visit requires the visitor to have the same "
- "return type for all alternatives of a variant");
- return __nonesuch{};
+ struct __cannot_match {};
+ return __cannot_match{};
}
else
return _Array_type{&__visit_invoke};
Re: [PATCH] libstdc++: Diagnose visitors with different return types [PR95904]
On Sat, 10 Oct 2020 at 13:52, Jonathan Wakely wrote: > index_sequence uses size_t not unsigned long. This parameter pack > needs to be size_t... _Idxs, and the NTTP for __check_visitor_result > should be size_t _Idx. Fixed in https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=02cbd79e4728319e0887ad7783297853b527bb13 after approval-over-irc.
Re: [PATCH] libstdc++: Diagnose visitors with different return types [PR95904]
On Mon, 5 Oct 2020 at 01:15, Ville Voutilainen
wrote:
> The patch is borked, doesn't pass tests, fixing...
Unborked, ok for trunk if full testsuite passes?
2020-10-05 Ville Voutilainen
PR libstdc++/95904
* include/std/variant (__deduce_visit_result): Add a nested ::type.
(__gen_vtable_impl::_S_apply):
Check the visitor return type.
(__same_types): New.
(__check_visitor_result): Likewise.
(__check_visitor_results): Likewise.
(visit(_Visitor&&, _Variants&&...)): Use __check_visitor_results
in case we're visiting just one variant.
* testsuite/20_util/variant/visit_neg.cc: Adjust.
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index dd8847cf829..b32e564fd41 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -182,7 +182,7 @@ namespace __variant
// used for raw visitation with indices passed in
struct __variant_idx_cookie { using type = __variant_idx_cookie; };
// Used to enable deduction (and same-type checking) for std::visit:
- template struct __deduce_visit_result { };
+ template struct __deduce_visit_result { using type = _Tp; };
// Visit variants that might be valueless.
template
@@ -1017,7 +1017,26 @@ namespace __variant
static constexpr auto
_S_apply()
- { return _Array_type{&__visit_invoke}; }
+ {
+ if constexpr (_Array_type::__result_is_deduced::value)
+ {
+ constexpr bool __visit_ret_type_mismatch =
+ !is_same_v(),
+std::declval<_Variants>()...))>;
+ if constexpr (__visit_ret_type_mismatch)
+ {
+ static_assert(!__visit_ret_type_mismatch,
+ "std::visit requires the visitor to have the same "
+ "return type for all alternatives of a variant");
+ return __nonesuch{};
+ }
+ else
+ return _Array_type{&__visit_invoke};
+ }
+ else
+ return _Array_type{&__visit_invoke};
+ }
};
template
@@ -1692,6 +1711,26 @@ namespace __variant
std::forward<_Variants>(__variants)...);
}
+ template
+ constexpr inline bool __same_types = (is_same_v<_Tp, _Types> && ...);
+
+ template
+decltype(auto)
+__check_visitor_result(_Visitor&& __vis, _Variant&& __variant)
+{
+ return std::__invoke(std::forward<_Visitor>(__vis),
+ std::get<_Idx>(std::forward<_Variant>(__variant)));
+}
+
+ template
+constexpr bool __check_visitor_results(std::index_sequence<_Idxs...>)
+{
+ return __same_types(
+ std::declval<_Visitor>(),
+ std::declval<_Variant>()))...>;
+}
+
+
template
constexpr decltype(auto)
visit(_Visitor&& __visitor, _Variants&&... __variants)
@@ -1704,8 +1743,28 @@ namespace __variant
using _Tag = __detail::__variant::__deduce_visit_result<_Result_type>;
- return std::__do_visit<_Tag>(std::forward<_Visitor>(__visitor),
- std::forward<_Variants>(__variants)...);
+ if constexpr (sizeof...(_Variants) == 1)
+ {
+ constexpr bool __visit_rettypes_match =
+ __check_visitor_results<_Visitor, _Variants...>(
+ std::make_index_sequence<
+ std::variant_size...>::value>());
+ if constexpr (!__visit_rettypes_match)
+ {
+ static_assert(__visit_rettypes_match,
+ "std::visit requires the visitor to have the same "
+ "return type for all alternatives of a variant");
+ return;
+ }
+ else
+ return std::__do_visit<_Tag>(
+ std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__variants)...);
+ }
+ else
+ return std::__do_visit<_Tag>(
+ std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__variants)...);
}
#if __cplusplus > 201703L
diff --git a/libstdc++-v3/testsuite/20_util/variant/visit_neg.cc b/libstdc++-v3/testsuite/20_util/variant/visit_neg.cc
index 6279dec5aa2..748eb21c1ad 100644
--- a/libstdc++-v3/testsuite/20_util/variant/visit_neg.cc
+++ b/libstdc++-v3/testsuite/20_util/variant/visit_neg.cc
@@ -21,7 +21,7 @@
#include
#include
-// { dg-error "invalid conversion" "" { target *-*-* } 0 }
+// { dg-error "same return type for all alternatives" "" { target *-*-* } 0 }
// { dg-prune-output "in 'constexpr' expansion" }
void
Re: [PATCH] libstdc++: Diagnose visitors with different return types [PR95904]
On Sat, 3 Oct 2020 at 01:14, Jonathan Wakely wrote: > OK for trunk with those leading spaces switched to tab. The patch is borked, doesn't pass tests, fixing...
Re: [PATCH] libstdc++: Diagnose visitors with different return types [PR95904]
On Tue, 29 Sep 2020 at 14:20, Jonathan Wakely wrote:
> I think this is what we want:
>
>template
> constexpr inline __same_types = (is_same_v<_Tp, _Types> && ...);
>
> is_same_v is very cheap, it uses the built-in directly, so you don't
> need to instantiate any class templates at all.
>
> >+
> >+ template
>
> typename not class please.
>
> >+decltype(auto) __check_visitor_result(_Visitor&& __vis,
>
> New line after the decltype(auto) please, not in the middle of the
> parameter list.
Aye.
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index dd8847cf829..6f647d622c4 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -182,7 +182,7 @@ namespace __variant
// used for raw visitation with indices passed in
struct __variant_idx_cookie { using type = __variant_idx_cookie; };
// Used to enable deduction (and same-type checking) for std::visit:
- template struct __deduce_visit_result { };
+ template struct __deduce_visit_result { using type = _Tp; };
// Visit variants that might be valueless.
template
@@ -1017,7 +1017,22 @@ namespace __variant
static constexpr auto
_S_apply()
- { return _Array_type{&__visit_invoke}; }
+ {
+ constexpr bool __visit_ret_type_mismatch =
+ _Array_type::__result_is_deduced::value
+ && !is_same_v(),
+ std::declval<_Variants>()...))>;
+ if constexpr (__visit_ret_type_mismatch)
+ {
+ static_assert(!__visit_ret_type_mismatch,
+ "std::visit requires the visitor to have the same "
+ "return type for all alternatives of a variant");
+ return __nonesuch{};
+ }
+ else
+ return _Array_type{&__visit_invoke};
+ }
};
template
@@ -1692,6 +1707,26 @@ namespace __variant
std::forward<_Variants>(__variants)...);
}
+ template
+ constexpr inline bool __same_types = (is_same_v<_Tp, _Types> && ...);
+
+ template
+decltype(auto)
+__check_visitor_result(_Visitor&& __vis, _Variant&& __variant)
+{
+ return std::forward<_Visitor>(__vis)(
+std::get<_Idx>(std::forward<_Variant>(__variant)));
+}
+
+ template
+constexpr bool __check_visitor_results(std::index_sequence<_Idxs...>)
+{
+ return __same_types(
+ std::declval<_Visitor>(),
+ std::declval<_Variant>()))...>;
+}
+
+
template
constexpr decltype(auto)
visit(_Visitor&& __visitor, _Variants&&... __variants)
@@ -1704,8 +1739,28 @@ namespace __variant
using _Tag = __detail::__variant::__deduce_visit_result<_Result_type>;
- return std::__do_visit<_Tag>(std::forward<_Visitor>(__visitor),
- std::forward<_Variants>(__variants)...);
+ if constexpr (sizeof...(_Variants) == 1)
+{
+ constexpr bool __visit_rettypes_match =
+ __check_visitor_results<_Visitor, _Variants...>(
+ std::make_index_sequence<
+ std::variant_size...>::value>());
+ if constexpr (!__visit_rettypes_match)
+ {
+ static_assert(__visit_rettypes_match,
+ "std::visit requires the visitor to have the same "
+ "return type for all alternatives of a variant");
+ return;
+ }
+ else
+ return std::__do_visit<_Tag>(
+ std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__variants)...);
+ }
+ else
+ return std::__do_visit<_Tag>(
+ std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__variants)...);
}
#if __cplusplus > 201703L
[PATCH] c++: Diagnose visitors with different return types for std::visit [PR95904]
Not completely tested yet. This does fix the problem of converting
incompatible pointer-to-function types, and thus gets rid of the suggestion
that compiling the code with -fpermissive is a possibility. There
is a special-casing for visit() for visitation of a single variant, and there
we don't even instantiate the whole table mechanism. We should really
entertain the possibility of flattening the whole visitation table; then
this check could (at least in theory) be uniformly written as just
an iteration of all table elements, which is not so convenient to do
with the nested multitable. This seems like a worthy incremental
improvement to me.
2020-09-29 Ville Voutilainen
PR libstdc++/95904
* include/std/variant (__same_types): New.
(__check_visitor_result): Likewise.
(__check_visitor_results): Likewise.
(visit(_Visitor&&, _Variants&&...)): Use __check_visitor_results
in case we're visiting just one variant.
(__gen_vtable_impl::_S_apply):
Check the visitor return type.
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index dd8847cf829..56de78407c4 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -182,7 +182,7 @@ namespace __variant
// used for raw visitation with indices passed in
struct __variant_idx_cookie { using type = __variant_idx_cookie; };
// Used to enable deduction (and same-type checking) for std::visit:
- template struct __deduce_visit_result { };
+ template struct __deduce_visit_result { using type = _Tp; };
// Visit variants that might be valueless.
template
@@ -1017,7 +1017,22 @@ namespace __variant
static constexpr auto
_S_apply()
- { return _Array_type{&__visit_invoke}; }
+ {
+ constexpr bool __visit_ret_type_mismatch =
+ _Array_type::__result_is_deduced::value
+ && !is_same_v(),
+ std::declval<_Variants>()...))>;
+ if constexpr (__visit_ret_type_mismatch)
+ {
+ static_assert(!__visit_ret_type_mismatch,
+ "std::visit requires the visitor to have the same "
+ "return type for all alternatives of a variant");
+ return __nonesuch{};
+ }
+ else
+ return _Array_type{&__visit_invoke};
+ }
};
template
@@ -1692,6 +1707,27 @@ namespace __variant
std::forward<_Variants>(__variants)...);
}
+ template
+struct __same_types : public std::bool_constant<
+std::__and_...>::value> {};
+
+ template
+decltype(auto) __check_visitor_result(_Visitor&& __vis,
+ _Variant&& __variant)
+{
+ return std::forward<_Visitor>(__vis)(
+std::get<_Idx>(std::forward<_Variant>(__variant)));
+}
+
+ template
+constexpr bool __check_visitor_results(std::index_sequence<_Idxs...>)
+{
+ return __same_types(
+ std::declval<_Visitor>(),
+ std::declval<_Variant>()))...>::value;
+}
+
+
template
constexpr decltype(auto)
visit(_Visitor&& __visitor, _Variants&&... __variants)
@@ -1704,8 +1740,28 @@ namespace __variant
using _Tag = __detail::__variant::__deduce_visit_result<_Result_type>;
- return std::__do_visit<_Tag>(std::forward<_Visitor>(__visitor),
- std::forward<_Variants>(__variants)...);
+ if constexpr (sizeof...(_Variants) == 1)
+{
+ constexpr bool __visit_rettypes_match =
+ __check_visitor_results<_Visitor, _Variants...>(
+ std::make_index_sequence<
+ std::variant_size...>::value>());
+ if constexpr (!__visit_rettypes_match)
+ {
+ static_assert(__visit_rettypes_match,
+ "std::visit requires the visitor to have the same "
+ "return type for all alternatives of a variant");
+ return;
+ }
+ else
+ return std::__do_visit<_Tag>(
+ std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__variants)...);
+ }
+ else
+ return std::__do_visit<_Tag>(
+ std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__variants)...);
}
#if __cplusplus > 201703L
Re: [PATCH] Fix overflow handling in std::align
On Mon, 14 Sep 2020 at 15:49, Glen Fernandes wrote: > > On Mon, Sep 14, 2020 at 5:52 AM Ville Voutilainen wrote: > > On Mon, 14 Sep 2020 at 12:51, Ville Voutilainen > > wrote: > > > On Mon, 14 Sep 2020 at 09:18, Glen Fernandes > > wrote: > > > > Edit; Correct patch this time. > > > > > > > > Fix overflow handling in align > > > > > > Should the test verify that space is unmodified when nullptr is returned? > > > > ..and same for ptr. > > Sounds like a good idea. Updated patch attached. Looks good to me.
Re: [PATCH] Fix overflow handling in std::align
On Mon, 14 Sep 2020 at 12:51, Ville Voutilainen wrote: > > On Mon, 14 Sep 2020 at 09:18, Glen Fernandes via Libstdc++ > wrote: > > > > Edit; Correct patch this time. > > > > Fix overflow handling in align > > Should the test verify that space is unmodified when nullptr is returned? ..and same for ptr.
Re: [PATCH] Fix overflow handling in std::align
On Mon, 14 Sep 2020 at 09:18, Glen Fernandes via Libstdc++ wrote: > > Edit; Correct patch this time. > > Fix overflow handling in align Should the test verify that space is unmodified when nullptr is returned?
Re: [PATCH] c++: Fixing the wording of () aggregate-init [PR92812]
On Tue, 21 Jul 2020 at 22:39, Marek Polacek wrote: > > Okay. I think it's remotely reasonable that a static_cast(42) would > > work for an array, then. > > And by a natural extension, that using the concrete type would also > > work. That seems consistent, > > but doesn't seem like it rises to the level of "an important part of > > the design". Trafficking arrays > > around in generic code is already hard, because they don't behave like > > other things do, so the > > answer to "was this supposed to work?" is pretty much "it wasn't > > considered during the design phase". > > Fair enough. Sorry. :) None of the main driving-use-cases for allowing paren-init of aggregates involved arrays. They were to some extent expected to hitch a ride since they are aggregates, but otherwise they never received particular design-level lovin'. It seemed plausible to be able to paren-init a member array in a ctor-initializer. Paren-initializing automatic storage duration variables requires a typedef anyway. So the casting cases have not been fully fledged design-wise, and that's mostly just oversight. Or lack of energy, if you wish.
Re: [PATCH] c++: Fixing the wording of () aggregate-init [PR92812]
On Tue, 21 Jul 2020 at 21:56, Marek Polacek wrote:
>
> On Tue, Jul 21, 2020 at 12:53:03PM +0300, Ville Voutilainen wrote:
> > On Tue, 21 Jul 2020 at 02:28, Marek Polacek wrote:
> > >
> > > P1975R0 tweaks the static_cast wording: it says that "An expression e can
> > > be
> > > explicitly converted to a type T if [...] T is an aggregate type having a
> > > first
> > > element x and there is an implicit conversion sequence from e to the type
> > > of
> > > x." This already works for classes, e.g.:
> > >
> > > struct Aggr { int x; int y; };
> > > Aggr a = static_cast(1);
> > >
> > > albeit I noticed a -Wmissing-field-initializer warning which is unlikely
> > > to be
> > > helpful in this context, as there's nothing like static_cast(1, 2)
> > > to quash that warning.
> > >
> > > However, the proposal also mentions "If T is ``array of unknown bound of
> > > U'',
> > > this direct-initialization defines the type of the expression as U[1]"
> > > which
> > > suggest that this should work for arrays (they're aggregates too, after
> > > all).
> > > Ville, can you confirm that these
> > >
> > > int (&)[3] = static_cast(42);
> > > int (&)[1] = static_cast(42);
> > >
> > > are supposed to work now? There's no {} variant to check. Thanks.
> >
> > I don't know what it means to cast something to an array; doesn't that
> > create
> > an array prvalue? Is that a thing?
>
> Yes, I imagined this would be similar to
>
> using T = int[3];
> int (&)[3] = T{1, 2, 3}; // binds to array prvalue, lifetime extended
>
> but I'd like to avoid allowing code that isn't supposed to work.
Okay. I think it's remotely reasonable that a static_cast(42) would
work for an array, then.
And by a natural extension, that using the concrete type would also
work. That seems consistent,
but doesn't seem like it rises to the level of "an important part of
the design". Trafficking arrays
around in generic code is already hard, because they don't behave like
other things do, so the
answer to "was this supposed to work?" is pretty much "it wasn't
considered during the design phase".
> We also might want to consider if we want to extend the lifetime of r/r2 in my
> previous example (I think so; DRs 1299/1376). Worth bothering CWG?
I think it's a good idea to bother CWG with both the above and the
lifetime extension.
Re: [PATCH] c++: Fixing the wording of () aggregate-init [PR92812]
On Tue, 21 Jul 2020 at 02:28, Marek Polacek wrote:
>
> P1975R0 tweaks the static_cast wording: it says that "An expression e can be
> explicitly converted to a type T if [...] T is an aggregate type having a
> first
> element x and there is an implicit conversion sequence from e to the type of
> x." This already works for classes, e.g.:
>
> struct Aggr { int x; int y; };
> Aggr a = static_cast(1);
>
> albeit I noticed a -Wmissing-field-initializer warning which is unlikely to be
> helpful in this context, as there's nothing like static_cast(1, 2)
> to quash that warning.
>
> However, the proposal also mentions "If T is ``array of unknown bound of U'',
> this direct-initialization defines the type of the expression as U[1]" which
> suggest that this should work for arrays (they're aggregates too, after all).
> Ville, can you confirm that these
>
> int (&)[3] = static_cast(42);
> int (&)[1] = static_cast(42);
>
> are supposed to work now? There's no {} variant to check. Thanks.
I don't know what it means to cast something to an array; doesn't that create
an array prvalue? Is that a thing?
Re: [PATCH] libstdc++: std::variant with multiple identical types assignment fail to compile [PR91807]
On Wed, 1 Jul 2020 at 23:53, Jonathan Wakely wrote: > > On 01/07/20 23:32 +0300, Ville Voutilainen via Libstdc++ wrote: > >On Wed, 1 Jul 2020 at 21:09, Ville Voutilainen > > wrote: > >> And sure, s/move-construction/move-assignment/. > > > >And with dg-options. > > OK for master and gcc-10, thanks. > > Does it apply cleanly to gcc-9 too? Yes. I managed to misremember where the new variant implementation landed and misread the bug title/version, so backporting there, too.
Re: [PATCH] libstdc++: std::variant with multiple identical types assignment fail to compile [PR91807]
On Wed, 1 Jul 2020 at 21:09, Ville Voutilainen
wrote:
> And sure, s/move-construction/move-assignment/.
And with dg-options.
2020-07-01 Ville Voutilainen
PR libstdc++/91807
* include/std/variant
(_Copy_assign_base::operator=(const _Copy_assign_base&):
Do the move-assignment from a temporary so that the temporary
is constructed with an explicit index.
* testsuite/20_util/variant/91807.cc: New.
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index c9504914365..eb3d6779205 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -611,7 +611,8 @@ namespace __variant
this->_M_destructive_copy(__rhs_index, __rhs_mem);
else
__variant_cast<_Types...>(*this)
- = variant<_Types...>(__rhs_mem);
+ = variant<_Types...>(std::in_place_index<__rhs_index>,
+ __rhs_mem);
}
}
else
diff --git a/libstdc++-v3/testsuite/20_util/variant/91807.cc b/libstdc++-v3/testsuite/20_util/variant/91807.cc
new file mode 100644
index 000..04bb5d7c807
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/variant/91807.cc
@@ -0,0 +1,35 @@
+// { dg-options "-std=gnu++17" }
+// { dg-do compile { target c++17 } }
+
+// Copyright (C) 2020 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+#include
+
+struct me_data {
+ me_data() = default;
+
+ me_data(const me_data &) {};
+ me_data(me_data &&) noexcept {};
+ me_data& operator=(const me_data &) = default;
+};
+
+int main() {
+ std::variant v1, v2;
+
+ v2 = v1;
+}
Re: [PATCH] libstdc++: std::variant with multiple identical types assignment fail to compile [PR91807]
On Wed, 1 Jul 2020 at 20:46, Ville Voutilainen wrote: > > Looks like just a small thinko. We construct a temporary and move-construct > from it, but we should construct the temporary with the right index. > > OK for trunk and gcc-10 if full tests pass? > > 2020-07-01 Ville Voutilainen > > PR libstdc++/91807 > * include/std/variant > (_Copy_assign_base::operator=(const _Copy_assign_base&): > Do the move-construction from a temporary so that the temporary > is constructed with an explicit index. > * testsuite/20_util/variant/91807.cc: New. And sure, s/move-construction/move-assignment/.
[PATCH] libstdc++: std::variant with multiple identical types assignment fail to compile [PR91807]
Looks like just a small thinko. We construct a temporary and move-construct
from it, but we should construct the temporary with the right index.
OK for trunk and gcc-10 if full tests pass?
2020-07-01 Ville Voutilainen
PR libstdc++/91807
* include/std/variant
(_Copy_assign_base::operator=(const _Copy_assign_base&):
Do the move-construction from a temporary so that the temporary
is constructed with an explicit index.
* testsuite/20_util/variant/91807.cc: New.
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index c9504914365..eb3d6779205 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -611,7 +611,8 @@ namespace __variant
this->_M_destructive_copy(__rhs_index, __rhs_mem);
else
__variant_cast<_Types...>(*this)
- = variant<_Types...>(__rhs_mem);
+ = variant<_Types...>(std::in_place_index<__rhs_index>,
+ __rhs_mem);
}
}
else
diff --git a/libstdc++-v3/testsuite/20_util/variant/91807.cc b/libstdc++-v3/testsuite/20_util/variant/91807.cc
new file mode 100644
index 000..ddede7c9b32
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/variant/91807.cc
@@ -0,0 +1,34 @@
+// { dg-do compile { target c++17 } }
+
+// Copyright (C) 2020 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+#include
+
+struct me_data {
+ me_data() = default;
+
+ me_data(const me_data &) {};
+ me_data(me_data &&) noexcept {};
+ me_data& operator=(const me_data &) = default;
+};
+
+int main() {
+ std::variant v1, v2;
+
+ v2 = v1;
+}
Re: [PATCH] libstdc++: Add a __nonnnull__ attribute to std::string's _CharT* constructor
On Mon, 29 Jun 2020 at 00:16, Jonathan Wakely wrote: > >Hmm, let's use dg-additional-options here too, and axe the pointless > >-std=gnu++11. > > I agree the -std=gnu++11 isn't needed, but thre doesn't seem to be any > advantage to dg-additional-options here. The reason I suggested it for > th other tests was that it can take a target selector, so it can be > used to add options for some targets only (in the case of the variant > tests, only if the dialect is c++17 or later). > > Here you want the -Wnonnull added unconditionally, with no target > selector, so dg-options works fine. Right, I somehow managed to think that dg-additional-options amends options and dg-options overrides them. Here my intent is certainly the former.
Re: [PATCH] libstdc++: Add a __nonnnull__ attribute to std::string's _CharT* constructor
On Sun, 28 Jun 2020 at 13:56, Ville Voutilainen
wrote:
>
> 2020-06-28 Ville Voutilainen
>
> Add a __nonnnull__ attribute to std::string's _CharT* constructor
> * include/bits/basic_string.h (string(_CharT*, const _Alloc&)):
> Add a __nonnull__ attribute.
> * testsuite/21_strings/basic_string/cons/char/nonnull.cc: New.
> * testsuite/21_strings/basic_string/cons/wchar_t/nonnull.cc: Likewise.
Hmm, let's use dg-additional-options here too, and axe the pointless
-std=gnu++11.
diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h
index bc0c256b65e..d5e5eb06e1b 100644
--- a/libstdc++-v3/include/bits/basic_string.h
+++ b/libstdc++-v3/include/bits/basic_string.h
@@ -522,6 +522,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
// 3076. basic_string CTAD ambiguity
template>
#endif
+ __attribute__((__nonnull__))
basic_string(const _CharT* __s, const _Alloc& __a = _Alloc())
: _M_dataplus(_M_local_data(), __a)
{ _M_construct(__s, __s ? __s + traits_type::length(__s) : __s+npos); }
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/cons/char/nonnull.cc b/libstdc++-v3/testsuite/21_strings/basic_string/cons/char/nonnull.cc
new file mode 100644
index 000..1c09a1d2207
--- /dev/null
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/cons/char/nonnull.cc
@@ -0,0 +1,29 @@
+// Copyright (C) 2020 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-additional-options "-Wnonnull" }
+// { dg-do compile { target c++11 } }
+
+#include
+
+void
+test01()
+{
+ std::string s((const char*)nullptr); // { dg-warning "null arg" }
+ std::string t((char*)nullptr); // { dg-warning "null arg" }
+ std::string u(nullptr); // { dg-warning "null arg" }
+}
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/cons/wchar_t/nonnull.cc b/libstdc++-v3/testsuite/21_strings/basic_string/cons/wchar_t/nonnull.cc
new file mode 100644
index 000..456efed6c8c
--- /dev/null
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/cons/wchar_t/nonnull.cc
@@ -0,0 +1,29 @@
+// Copyright (C) 2020 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-additional-options "-Wnonnull" }
+// { dg-do compile { target c++11 } }
+
+#include
+
+void
+test01()
+{
+ std::wstring s((const wchar_t*)nullptr); // { dg-warning "null arg" }
+ std::wstring t((wchar_t*)nullptr); // { dg-warning "null arg" }
+ std::wstring u(nullptr); // { dg-warning "null arg" }
+}
[PATCH] libstdc++: Add a __nonnnull__ attribute to std::string's _CharT* constructor
2020-06-28 Ville Voutilainen
Add a __nonnnull__ attribute to std::string's _CharT* constructor
* include/bits/basic_string.h (string(_CharT*, const _Alloc&)):
Add a __nonnull__ attribute.
* testsuite/21_strings/basic_string/cons/char/nonnull.cc: New.
* testsuite/21_strings/basic_string/cons/wchar_t/nonnull.cc: Likewise.
diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h
index bc0c256b65e..d5e5eb06e1b 100644
--- a/libstdc++-v3/include/bits/basic_string.h
+++ b/libstdc++-v3/include/bits/basic_string.h
@@ -522,6 +522,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
// 3076. basic_string CTAD ambiguity
template>
#endif
+ __attribute__((__nonnull__))
basic_string(const _CharT* __s, const _Alloc& __a = _Alloc())
: _M_dataplus(_M_local_data(), __a)
{ _M_construct(__s, __s ? __s + traits_type::length(__s) : __s+npos); }
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/cons/char/nonnull.cc b/libstdc++-v3/testsuite/21_strings/basic_string/cons/char/nonnull.cc
new file mode 100644
index 000..b56bdc78b51
--- /dev/null
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/cons/char/nonnull.cc
@@ -0,0 +1,29 @@
+// Copyright (C) 2020 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++11 -Wnonnull" }
+// { dg-do compile { target c++11 } }
+
+#include
+
+void
+test01()
+{
+ std::string s((const char*)nullptr); // { dg-warning "null arg" }
+ std::string t((char*)nullptr); // { dg-warning "null arg" }
+ std::string u(nullptr); // { dg-warning "null arg" }
+}
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/cons/wchar_t/nonnull.cc b/libstdc++-v3/testsuite/21_strings/basic_string/cons/wchar_t/nonnull.cc
new file mode 100644
index 000..8196821f163
--- /dev/null
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/cons/wchar_t/nonnull.cc
@@ -0,0 +1,29 @@
+// Copyright (C) 2020 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++11 -Wnonnull" }
+// { dg-do compile { target c++11 } }
+
+#include
+
+void
+test01()
+{
+ std::wstring s((const wchar_t*)nullptr); // { dg-warning "null arg" }
+ std::wstring t((wchar_t*)nullptr); // { dg-warning "null arg" }
+ std::wstring u(nullptr); // { dg-warning "null arg" }
+}
Re: [PATCH] libstdc++: std::variant doesn't like types with a defaulted virtual destructor [PR95915]
On Sat, 27 Jun 2020 at 17:53, Ville Voutilainen
wrote:
>
> On Fri, 26 Jun 2020 at 21:20, Jonathan Wakely wrote:
> > For these three tests I think this would be slightly better:
> >
> > // { dg-additional-options "-Wno-deprecated" { target c++17 } }
> >
> > That way we only ignore the warning when actually needed.
>
> Sure thing. The test run revealed some additional things to tweak. OK for
> trunk
> and GCC 10?
With a twist, I mean. I don't plan to backport the deprecation, just
the bug fix for variant.
Re: [PATCH] libstdc++: std::variant doesn't like types with a defaulted virtual destructor [PR95915]
On Fri, 26 Jun 2020 at 21:20, Jonathan Wakely wrote:
> For these three tests I think this would be slightly better:
>
> // { dg-additional-options "-Wno-deprecated" { target c++17 } }
>
> That way we only ignore the warning when actually needed.
Sure thing. The test run revealed some additional things to tweak. OK for trunk
and GCC 10?
2020-06-27 Ville Voutilainen
PR libstdc++/95915
* include/std/type_traits (is_literal_type, is_literal_type_v):
Deprecate in C++17.
* include/std/variant (_Uninitialized):
Adjust the condition and the comment.
* testsuite/20_util/is_literal_type/deprecated-1z.cc: New.
* testsuite/20_util/is_literal_type/requirements/explicit_instantiation.cc:
Adjust.
* testsuite/20_util/is_literal_type/requirements/typedefs.cc: Likewise.
* testsuite/20_util/is_literal_type/value.cc: Likewise.
* testsuite/20_util/optional/constexpr/nullopt.cc:
Use __is_literal_type directly.
* testsuite/20_util/optional/nullopt.cc: Likewise.
* testsuite/20_util/variable_templates_for_traits.cc: Adjust.
* testsuite/20_util/variant/95915.cc: New.
* testsuite/20_util/variant/compile.cc: Add new test.
* testsuite/experimental/optional/constexpr/nullopt.cc:
Use __is_literal_type directly.
* testsuite/experimental/optional/nullopt.cc: Likewise.
* testsuite/experimental/type_traits/value.cc: Adjust.
* testsuite/util/testsuite_common_types.h:
Use __is_literal_type directly.
diff --git a/libstdc++-v3/include/std/type_traits b/libstdc++-v3/include/std/type_traits
index bc9a45b3746..9cd3a2df41a 100644
--- a/libstdc++-v3/include/std/type_traits
+++ b/libstdc++-v3/include/std/type_traits
@@ -703,7 +703,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
/// is_literal_type
template
-struct is_literal_type
+struct
+_GLIBCXX17_DEPRECATED
+is_literal_type
: public integral_constant
{
static_assert(std::__is_complete_or_unbounded(__type_identity<_Tp>{}),
@@ -3085,10 +3087,11 @@ template
template
_GLIBCXX20_DEPRECATED("use is_standard_layout_v && is_trivial_v instead")
inline constexpr bool is_pod_v = is_pod<_Tp>::value;
-#pragma GCC diagnostic pop
template
+ _GLIBCXX17_DEPRECATED
inline constexpr bool is_literal_type_v = is_literal_type<_Tp>::value;
-template
+#pragma GCC diagnostic pop
+ template
inline constexpr bool is_empty_v = is_empty<_Tp>::value;
template
inline constexpr bool is_polymorphic_v = is_polymorphic<_Tp>::value;
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index 6eeb3c80ec2..c9504914365 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -202,15 +202,9 @@ namespace __variant
std::forward<_Variants>(__variants)...);
}
- // _Uninitialized is guaranteed to be a literal type, even if T is not.
- // We have to do this, because [basic.types]p10.5.3 (n4606) is not implemented
- // yet. When it's implemented, _Uninitialized can be changed to the alias
- // to T, therefore equivalent to being removed entirely.
- //
- // Another reason we may not want to remove _Uninitialzied may be that, we
- // want _Uninitialized to be trivially destructible, no matter whether T
- // is; but we will see.
- template>
+ // _Uninitialized is guaranteed to be a trivially destructible type,
+ // even if T is not.
+ template>
struct _Uninitialized;
template
diff --git a/libstdc++-v3/testsuite/20_util/is_literal_type/deprecated-1z.cc b/libstdc++-v3/testsuite/20_util/is_literal_type/deprecated-1z.cc
new file mode 100644
index 000..a91ff56dcf6
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/is_literal_type/deprecated-1z.cc
@@ -0,0 +1,26 @@
+// Copyright (C) 2020 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++17" }
+// { dg-do compile { target c++17 } }
+
+#include
+
+static_assert(std::is_literal_type::value); // { dg-warning "is deprecated" }
+static_assert(std::is_literal_type_v); // { dg-warning "is deprecated" }
+
+// { dg-prune-output "declared here" }
diff --git a/libstdc++-v3/testsuite/20_util/is_literal_type/r
Re: [PATCH] libstdc++: std::variant doesn't like types with a defaulted virtual destructor [PR95915]
On Fri, 26 Jun 2020 at 19:12, Ville Voutilainen wrote: > > This patch also deprecates std::is_literal_type. I forgot to ask, OK for trunk and GCC 10 if full suite testing passes? The problematic compiler bug has been gone since GCC 10, so we can just as well backport this there, but not further.
[PATCH] libstdc++: std::variant doesn't like types with a defaulted virtual destructor [PR95915]
This patch also deprecates std::is_literal_type.
2020-06-26 Ville Voutilainen
PR libstdc++/95915
* include/std/type_traits (is_literal_type, is_literal_type_v):
Deprecate in C++17.
* include/std/variant (_Uninitialized):
Adjust the condition and the comment.
* testsuite/20_util/is_literal_type/deprecated-1z.cc: New.
* testsuite/20_util/is_literal_type/requirements/explicit_instantiation.cc:
Adjust.
* testsuite/20_util/is_literal_type/requirements/typedefs.cc: Likewise.
* testsuite/20_util/is_literal_type/value.cc: Likewise.
* testsuite/20_util/variant/95915.cc: New.
* testsuite/20_util/variant/compile.cc: Add new test.
diff --git a/libstdc++-v3/include/std/type_traits b/libstdc++-v3/include/std/type_traits
index bc9a45b3746..261f25d21e7 100644
--- a/libstdc++-v3/include/std/type_traits
+++ b/libstdc++-v3/include/std/type_traits
@@ -703,7 +703,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
/// is_literal_type
template
-struct is_literal_type
+struct
+_GLIBCXX17_DEPRECATED
+is_literal_type
: public integral_constant
{
static_assert(std::__is_complete_or_unbounded(__type_identity<_Tp>{}),
@@ -3087,6 +3089,7 @@ template
inline constexpr bool is_pod_v = is_pod<_Tp>::value;
#pragma GCC diagnostic pop
template
+ _GLIBCXX17_DEPRECATED
inline constexpr bool is_literal_type_v = is_literal_type<_Tp>::value;
template
inline constexpr bool is_empty_v = is_empty<_Tp>::value;
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index 6eeb3c80ec2..c9504914365 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -202,15 +202,9 @@ namespace __variant
std::forward<_Variants>(__variants)...);
}
- // _Uninitialized is guaranteed to be a literal type, even if T is not.
- // We have to do this, because [basic.types]p10.5.3 (n4606) is not implemented
- // yet. When it's implemented, _Uninitialized can be changed to the alias
- // to T, therefore equivalent to being removed entirely.
- //
- // Another reason we may not want to remove _Uninitialzied may be that, we
- // want _Uninitialized to be trivially destructible, no matter whether T
- // is; but we will see.
- template>
+ // _Uninitialized is guaranteed to be a trivially destructible type,
+ // even if T is not.
+ template>
struct _Uninitialized;
template
diff --git a/libstdc++-v3/testsuite/20_util/is_literal_type/deprecated-1z.cc b/libstdc++-v3/testsuite/20_util/is_literal_type/deprecated-1z.cc
new file mode 100644
index 000..a91ff56dcf6
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/is_literal_type/deprecated-1z.cc
@@ -0,0 +1,26 @@
+// Copyright (C) 2020 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++17" }
+// { dg-do compile { target c++17 } }
+
+#include
+
+static_assert(std::is_literal_type::value); // { dg-warning "is deprecated" }
+static_assert(std::is_literal_type_v); // { dg-warning "is deprecated" }
+
+// { dg-prune-output "declared here" }
diff --git a/libstdc++-v3/testsuite/20_util/is_literal_type/requirements/explicit_instantiation.cc b/libstdc++-v3/testsuite/20_util/is_literal_type/requirements/explicit_instantiation.cc
index d0a20f3cf4e..d9c57bb8ef4 100644
--- a/libstdc++-v3/testsuite/20_util/is_literal_type/requirements/explicit_instantiation.cc
+++ b/libstdc++-v3/testsuite/20_util/is_literal_type/requirements/explicit_instantiation.cc
@@ -1,3 +1,4 @@
+// { dg-options "-Wno-deprecated" }
// { dg-do compile { target c++11 } }
// 2010-02-21 Paolo Carlini
diff --git a/libstdc++-v3/testsuite/20_util/is_literal_type/requirements/typedefs.cc b/libstdc++-v3/testsuite/20_util/is_literal_type/requirements/typedefs.cc
index 9b7ae894725..24f508805f2 100644
--- a/libstdc++-v3/testsuite/20_util/is_literal_type/requirements/typedefs.cc
+++ b/libstdc++-v3/testsuite/20_util/is_literal_type/requirements/typedefs.cc
@@ -1,3 +1,4 @@
+// { dg-options "-Wno-deprecated" }
// { dg-do compile { target c++11 } }
// 2010-02-21 Paolo Carlini
diff --git a/libstdc++-v3/testsuite/20_util/is_literal_type/value.cc b/libstdc++-v3/testsuite/20_ut
Re: std::optional defaut constructor
On Thu, 4 Jun 2020 at 14:41, Richard Biener wrote: > Doesn't the placement new make the memory state of anything > not explicitely initialized indeterminate? That is, isn't the > testcase broken anyways since GCC can elide the memset > when seeing the placement new? Hmm, yes it does, and the test is broken.
Re: std::optional defaut constructor
On Thu, 4 Jun 2020 at 11:53, Marc Glisse wrote:
>
> On Thu, 4 Jun 2020, Ville Voutilainen wrote:
>
> > On Thu, 4 Jun 2020 at 11:00, Marc Glisse wrote:
> >> Maybe create a buffer, fill it with some non-zero values (-1?), then call
> >> placement new, and read some value in the middle of the buffer, possibly
> >> with some protection against optimizations? Ah, no, actual constructors
> >> are fine, it is only the inlined initialization that happens with the
> >> defaulted constructor that zeroes things.
> >
> > The zero-init is part of value-initialization of a class type with a
> > defaulted default constructor, so value-initialization with placement
> > new should indeed show us whether the target buffer is zeroed.
>
> Ah, yes, I had forgotten the empty () at the end of the operator new line
> when testing. Now the patch makes this runtime test go from abort to
> success at -O0 (with optimizations, the memset is removed as dead code). I
> am still not sure we want this kind of test though. And I added launder
> more to quiet a warning than with confidence that it does the right thing.
>
> #include
> struct A {
>int a[1024];
> };
> typedef std::optional O;
> int main(){
>unsigned char t[sizeof(O)];
>__builtin_memset(t, -1, sizeof(t));
>new(t)O();
>if(std::launder(t)[512] != (unsigned char)(-1)) __builtin_abort();
> }
Yeah, I think the patch is OK with or without the test. As a side
note, you don't need the launder
if the check uses the pointer value returned by placement-new.
Re: std::optional defaut constructor
On Thu, 4 Jun 2020 at 11:00, Marc Glisse wrote: > Maybe create a buffer, fill it with some non-zero values (-1?), then call > placement new, and read some value in the middle of the buffer, possibly > with some protection against optimizations? Ah, no, actual constructors > are fine, it is only the inlined initialization that happens with the > defaulted constructor that zeroes things. The zero-init is part of value-initialization of a class type with a defaulted default constructor, so value-initialization with placement new should indeed show us whether the target buffer is zeroed.
Re: std::optional defaut constructor
On Thu, 4 Jun 2020 at 10:22, Marc Glisse wrote: > > So the change is correct. Can we test the change somehow? > > It passes the testsuite, and libc++ has been doing it this way for years. > What I feared was some regression where it would yield worse code in some > cases, or lose some property (not guaranteed by the standard) like > triviality (to the point of affecting the ABI?), but I couldn't see > anything like that happening. > > (we still have PR86173 causing unnecessary memset in some cases) Right, I was just wondering whether we can reasonably verify in a test that the whole shebang is not zeroed. That may need a tree-dump scan in the test, and probably should go into PR86173 anyway, so I'm not saying such a thing needs to be a part of this fix. I'm kindly suggesting to Jonathan that this should be OK, and backports too.
Re: std::optional defaut constructor
On Thu, 4 Jun 2020 at 03:05, Ville Voutilainen wrote: > > > "noexcept" is a red herring, what matters is defaulted vs user-provided. > > > In one case, we end up zero-initializing the whole buffer, and not in the > > > other. > > > > Yes, I just came to that conclusion. This is value-init, so the > > language manages to zero-init the whole-object, > > but with the change, it just calls a user-provided constructor. > > That'll then merely boil down to value-initializing just the _Empty > > part > > of the _Storage in _Optional_payload_base. We are in > > http://eel.is/c++draft/dcl.init#8.1.2. The change takes us > > to http://eel.is/c++draft/dcl.init#8.1.1. > > Ha, and optional's default constructor isn't even specified to be defaulted. So the change is correct. Can we test the change somehow?
Re: std::optional defaut constructor
On Thu, 4 Jun 2020 at 02:20, Ville Voutilainen wrote: > > On Thu, 4 Jun 2020 at 02:13, Marc Glisse wrote: > > > > On Thu, 4 Jun 2020, Ville Voutilainen wrote: > > > > > On Thu, 4 Jun 2020 at 01:52, Marc Glisse wrote: > > >> > > >> Hello, > > >> > > >> is there any drawback to the attached patch? It changes the code > > >> generated for > > > > > > I don't get it. The noexceptness of the defaulted default constructor > > > should be a computation > > > of the noexceptness of the subobjects, and that should boil down to > > > whether optional's storage > > > is noexcept-default-constructible. And that thing has a noexcept > > > default constructor. Why > > > does this patch change anything? > > > > "noexcept" is a red herring, what matters is defaulted vs user-provided. > > In one case, we end up zero-initializing the whole buffer, and not in the > > other. > > Yes, I just came to that conclusion. This is value-init, so the > language manages to zero-init the whole-object, > but with the change, it just calls a user-provided constructor. > That'll then merely boil down to value-initializing just the _Empty > part > of the _Storage in _Optional_payload_base. We are in > http://eel.is/c++draft/dcl.init#8.1.2. The change takes us > to http://eel.is/c++draft/dcl.init#8.1.1. Ha, and optional's default constructor isn't even specified to be defaulted.
Re: std::optional defaut constructor
On Thu, 4 Jun 2020 at 02:13, Marc Glisse wrote: > > On Thu, 4 Jun 2020, Ville Voutilainen wrote: > > > On Thu, 4 Jun 2020 at 01:52, Marc Glisse wrote: > >> > >> Hello, > >> > >> is there any drawback to the attached patch? It changes the code generated > >> for > > > > I don't get it. The noexceptness of the defaulted default constructor > > should be a computation > > of the noexceptness of the subobjects, and that should boil down to > > whether optional's storage > > is noexcept-default-constructible. And that thing has a noexcept > > default constructor. Why > > does this patch change anything? > > "noexcept" is a red herring, what matters is defaulted vs user-provided. > In one case, we end up zero-initializing the whole buffer, and not in the > other. Yes, I just came to that conclusion. This is value-init, so the language manages to zero-init the whole-object, but with the change, it just calls a user-provided constructor. That'll then merely boil down to value-initializing just the _Empty part of the _Storage in _Optional_payload_base. We are in http://eel.is/c++draft/dcl.init#8.1.2. The change takes us to http://eel.is/c++draft/dcl.init#8.1.1.
Re: std::optional defaut constructor
On Thu, 4 Jun 2020 at 01:52, Marc Glisse wrote: > > Hello, > > is there any drawback to the attached patch? It changes the code generated for I don't get it. The noexceptness of the defaulted default constructor should be a computation of the noexceptness of the subobjects, and that should boil down to whether optional's storage is noexcept-default-constructible. And that thing has a noexcept default constructor. Why does this patch change anything?
Re: Fix Debug mode Undefined Behavior
On Mon, 11 May 2020 at 00:09, François Dumont via Libstdc++ wrote: > > I just committed this patch. This was a commit-without-review. When the patch was originally posted, the maintainer said "Let's revisit it in a few weeks.". That's not the same as "OK when stage1 reopens."
Re: [PATCH] libstdc++: Fix tuple and optional construction from {aggregate_member_value} (libstdc++/94890)
On Fri, 1 May 2020 at 21:15, Ville Voutilainen
wrote:
>
> Aggregate-paren-init breaks tuple and optional. This fixes the breakage.
> An LWG issue will be filed.
The previous approach was bogus. Here's a better one. Ok for master
and gcc-10 if
full testsuite run passes?
2020-05-02 Ville Voutilainen
PR libstdc++/94890
* include/std/optional (__no_cpp20_converting_aggregates): New.
(optional(_Up&&)): Use it.
* include/std/tuple (__no_cpp20_converting_aggregates): New.
(__valid_args): Use it.
* testsuite/20_util/optional/cons/94890.cc: New.
* testsuite/20_util/tuple/cons/94890.cc: Likewise.
diff --git a/libstdc++-v3/include/std/optional b/libstdc++-v3/include/std/optional
index 37c2ba7a025..e74d79ddfc6 100644
--- a/libstdc++-v3/include/std/optional
+++ b/libstdc++-v3/include/std/optional
@@ -685,6 +685,22 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template
using _Requires = enable_if_t<__and_v<_Cond...>, bool>;
+ // Constraint for rejecting converting aggregates.
+ template
+static constexpr bool __no_cpp20_converting_aggregates()
+{
+#if __cpp_aggregate_paren_init
+ return !__and_<
+ __and_<
+ is_aggregate<_Tp>,
+ __not_<
+ is_same<_Tp,
+ remove_reference_t<_Up>>>>>::value;
+#else
+ return true;
+#endif
+}
+
public:
using value_type = _Tp;
@@ -696,6 +712,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template, __not_tag<_Up>,
is_constructible<_Tp, _Up&&>,
+ __bool_constant<
+ __no_cpp20_converting_aggregates<_Up>()>,
is_convertible<_Up&&, _Tp>> = true>
constexpr
optional(_Up&& __t)
@@ -704,6 +722,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template, __not_tag<_Up>,
is_constructible<_Tp, _Up&&>,
+ __bool_constant<
+ __no_cpp20_converting_aggregates<_Up>()>,
__not_>> = false>
explicit constexpr
optional(_Up&& __t)
diff --git a/libstdc++-v3/include/std/tuple b/libstdc++-v3/include/std/tuple
index db4872d3a52..194b564341c 100644
--- a/libstdc++-v3/include/std/tuple
+++ b/libstdc++-v3/include/std/tuple
@@ -566,18 +566,40 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__and_...>::value;
}
+
+ // Constraint for rejecting converting aggregates.
+ template
+static constexpr bool __no_cpp20_converting_aggregates()
+{
+#if __cpp_aggregate_paren_init
+ return !__and_<
+ __and_<
+ is_aggregate>,
+ __not_<
+ is_same,
+ remove_reference_t<_Args>>>>...>::value;
+#else
+ return true;
+#endif
+}
+
// Constraint for tuple(_UTypes&&...) where sizeof...(_UTypes) == 1.
template
static constexpr bool __valid_args()
{
return sizeof...(_Elements) == 1
- && !is_same>::value;
+ && !is_same>::value
+ && __no_cpp20_converting_aggregates<_Up>();
}
// Constraint for tuple(_UTypes&&...) where sizeof...(_UTypes) > 1.
- template
- static constexpr bool __valid_args()
- { return (sizeof...(_Tail) + 2) == sizeof...(_Elements); }
+ template
+ static constexpr
+ typename enable_if<(sizeof...(_Args) > 1), bool>::type __valid_args()
+ {
+ return sizeof...(_Args) == sizeof...(_Elements)
+ && __no_cpp20_converting_aggregates<_Args...>();
+ }
/* Constraint for constructors with a tuple parameter ensures
* that the constructor is only viable when it would not interfere with
diff --git a/libstdc++-v3/testsuite/20_util/optional/cons/94890.cc b/libstdc++-v3/testsuite/20_util/optional/cons/94890.cc
new file mode 100644
index 000..fe038eef7fc
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/optional/cons/94890.cc
@@ -0,0 +1,26 @@
+// Copyright (C) 2019-2020 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-do compile { target c++17 } }
+
+#include
+
+struct c { int i; };
+
+std::optional x({0});
+
+static_assert(!std::is_convertible>::value, "");
diff --git a/libstdc++-v3/testsuite/20_util/tuple/cons
[PATCH] libstdc++: Fix tuple and optional construction from {aggregate_member_value} (libstdc++/94890)
Aggregate-paren-init breaks tuple and optional. This fixes the breakage.
An LWG issue will be filed.
Full suite test run pending. Ok for master and gcc-10 if the full tests pass?
2020-05-01 Ville Voutilainen
PR libstdc++/94890
* include/std/optional (optional(_Up&&)): Add is_aggregate
to the convertibility constraints.
* include/std/tuple (__is_implicitly_constructible)
(__is_explicitly_constructible): Likewise.
* testsuite/20_util/optional/cons/94890.cc: New.
* testsuite/20_util/tuple/cons/94890.cc: Likewise.
diff --git a/libstdc++-v3/include/std/optional b/libstdc++-v3/include/std/optional
index 37c2ba7a025..e0e307bc5f5 100644
--- a/libstdc++-v3/include/std/optional
+++ b/libstdc++-v3/include/std/optional
@@ -696,7 +696,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template, __not_tag<_Up>,
is_constructible<_Tp, _Up&&>,
+#if __cpp_aggregate_paren_init
+ __or_,
+ is_convertible<_Up&&, _Tp>>> = true>
+#else
is_convertible<_Up&&, _Tp>> = true>
+#endif
constexpr
optional(_Up&& __t)
: _Base(std::in_place, std::forward<_Up>(__t)) { }
@@ -704,7 +709,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template, __not_tag<_Up>,
is_constructible<_Tp, _Up&&>,
+#if __cpp_aggregate_paren_init
+ __not_<__or_,
+ is_convertible<_Up&&, _Tp>>>> = false>
+#else
__not_>> = false>
+#endif
explicit constexpr
optional(_Up&& __t)
: _Base(std::in_place, std::forward<_Up>(__t)) { }
diff --git a/libstdc++-v3/include/std/tuple b/libstdc++-v3/include/std/tuple
index db4872d3a52..7744d9c402d 100644
--- a/libstdc++-v3/include/std/tuple
+++ b/libstdc++-v3/include/std/tuple
@@ -467,7 +467,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
static constexpr bool __is_implicitly_constructible()
{
return __and_...,
+#if __cpp_aggregate_paren_init
+ __or_,
+ is_convertible<_UTypes, _Types>>...
+#else
is_convertible<_UTypes, _Types>...
+#endif
>::value;
}
@@ -478,8 +483,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
static constexpr bool __is_explicitly_constructible()
{
return __and_...,
- __not_<__and_...>>
- >::value;
+ __not_<__and_<
+#if __cpp_aggregate_paren_init
+ __or_,
+ is_convertible<_UTypes, _Types>>...>
+#else
+ is_convertible<_UTypes, _Types>...>
+#endif
+ >>::value;
}
static constexpr bool __is_implicitly_default_constructible()
diff --git a/libstdc++-v3/testsuite/20_util/optional/cons/94890.cc b/libstdc++-v3/testsuite/20_util/optional/cons/94890.cc
new file mode 100644
index 000..f66ddb7db13
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/optional/cons/94890.cc
@@ -0,0 +1,26 @@
+// Copyright (C) 2019-2020 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "--std=gnu++17" }
+// { dg-do compile { target c++17 } }
+
+#include
+
+struct c { int i; };
+
+std::optional x({0});
+
diff --git a/libstdc++-v3/testsuite/20_util/tuple/cons/94890.cc b/libstdc++-v3/testsuite/20_util/tuple/cons/94890.cc
new file mode 100644
index 000..76b99c9fd05
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/tuple/cons/94890.cc
@@ -0,0 +1,25 @@
+// Copyright (C) 2019-2020 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-do compile { target c++11 } }
+
+#include
+
+struct c { int i; };
+
+std::tuple x({0});
+
Re: [PATCH][libstd++][PR92156]
On Tue, 21 Apr 2020 at 11:29, kamlesh kumar wrote:
>
> Added the fix for emplace.
>
> diff --git a/libstdc++-v3/include/std/any b/libstdc++-v3/include/std/any
> index 6b7e68f0e63..f35d90e548d 100644
> --- a/libstdc++-v3/include/std/any
> +++ b/libstdc++-v3/include/std/any
> @@ -178,30 +178,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> /// Construct with a copy of @p __value as the contained object.
> template ,
While we're at it, we should rename _ValueType to _Tp and the decayed
type to _VTp,
so that it matches the standard's naming as close as possible, and
thus removes the ongoing
maintenance confusion about which is which.
> +int main() {
> +auto a = std::any(std::in_place_type, 5);
> +auto b = std::any(std::in_place_type, {1});
> +std::any p = std::pair(1, 1);
> +(void)p;
> +std::any t = std::tuple(1);
I think this sort of tests should VERIFY that the constructed any
contains what we expect.
Iow, do an any_cast and check that, for instance, a and b contain an any.
Re: [PATCH][libstd++][PR92156]
On Tue, 21 Apr 2020 at 09:11, Ville Voutilainen wrote: > > On Tue, 21 Apr 2020 at 04:10, kamlesh kumar wrote: > > > > Thank you for reviewing. > > without _Decay to decay_t in the constructor which takes inplace_type_t, > > cases like this fails > > auto a = std::any(std::in_place_type, 5); > > > > for these constructors, standard does not say anything about > > not-sameness checks with any. > > https://en.cppreference.com/w/cpp/utility/any/any. > > Well, sure. Thus: > - the in_place constructor should not _Decay > - the constructor from T should _Decay > - the assignment from a T should _Decay > - emplace should not _Decay > > These bugs are not regressions, so presumably they can wait for stage1. ..except that two of them are. :) Anyhoo, the non-any handling needs to be retained for the T-constructor and the T-assignment, and removing it from emplace is not a regression but should be eventually done.
Re: [PATCH][libstd++][PR92156]
On Tue, 21 Apr 2020 at 04:10, kamlesh kumar wrote: > > Thank you for reviewing. > without _Decay to decay_t in the constructor which takes inplace_type_t, > cases like this fails > auto a = std::any(std::in_place_type, 5); > > for these constructors, standard does not say anything about > not-sameness checks with any. > https://en.cppreference.com/w/cpp/utility/any/any. Well, sure. Thus: - the in_place constructor should not _Decay - the constructor from T should _Decay - the assignment from a T should _Decay - emplace should not _Decay These bugs are not regressions, so presumably they can wait for stage1.
Re: [PATCH][libstd++][PR92156]
On Mon, 20 Apr 2020 at 21:09, Ville Voutilainen wrote: > > On Sat, 18 Apr 2020 at 03:35, kamlesh kumar via Libstdc++ > wrote: > > > > On Fri, Apr 17, 2020, 10:59 PM kamlesh kumar > > wrote: > > > > > Fixes all this. > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92156 > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91630 > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90415 > > > > > > On Fri, Apr 17, 2020 at 10:45 PM kamlesh kumar > > > wrote: > > > > > > > > This patch corrects the requirement of 4,5 and 6th constructor > > > > As per https://en.cppreference.com/w/cpp/utility/any/any. > > The patch looks correct to me. We have some old cruft there, like the > overload your patch removes, it was > there to support copy-only types, but LWG issues axed that bit. This > constructor indeed should not check is_constructible, > because it'll end up instantiating this constructor itself, and > compute its constraints, and instantiate itself. > The in_place constructor doesn't have that problem, because it won't > instantiate itself. ..except the change from _Decay to decay_t looks wrong. _Decay also checks the non-sameness with any. That change shouldn't be made.
Re: [PATCH][libstd++][PR92156]
On Sat, 18 Apr 2020 at 03:35, kamlesh kumar via Libstdc++ wrote: > > On Fri, Apr 17, 2020, 10:59 PM kamlesh kumar > wrote: > > > Fixes all this. > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92156 > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91630 > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90415 > > > > On Fri, Apr 17, 2020 at 10:45 PM kamlesh kumar > > wrote: > > > > > > This patch corrects the requirement of 4,5 and 6th constructor > > > As per https://en.cppreference.com/w/cpp/utility/any/any. The patch looks correct to me. We have some old cruft there, like the overload your patch removes, it was there to support copy-only types, but LWG issues axed that bit. This constructor indeed should not check is_constructible, because it'll end up instantiating this constructor itself, and compute its constraints, and instantiate itself. The in_place constructor doesn't have that problem, because it won't instantiate itself.
Re: [PATCH] c++: Fix access checking for __is_assignable and __is_constructible (c++/94197)
On Tue, 17 Mar 2020 at 16:52, Ville Voutilainen
wrote:
>
> On Tue, 17 Mar 2020 at 16:42, Jason Merrill wrote:
> >
> > On 3/17/20 9:04 AM, Jonathan Wakely wrote:
> > > On 17/03/20 13:02 +, Jonathan Wakely wrote:
> > >> Shouldn't the test use { dg-do compile { target c++11 } } instead of:
> > >>
> > >> +// { dg-do compile }
> > >> +// { dg-options "-std=c++11" }
> >
> > Yes, good point.
>
> Ack, changing to { dg-do compile { target c++11 } } and committing
> with the (correct :P) expectation that
> the change is still ok with that change.
Jason, are backports ok too?
Re: [PATCH] c++: Fix access checking for __is_assignable and __is_constructible (c++/94197)
On Tue, 17 Mar 2020 at 16:42, Jason Merrill wrote:
>
> On 3/17/20 9:04 AM, Jonathan Wakely wrote:
> > On 17/03/20 13:02 +, Jonathan Wakely wrote:
> >> Shouldn't the test use { dg-do compile { target c++11 } } instead of:
> >>
> >> +// { dg-do compile }
> >> +// { dg-options "-std=c++11" }
>
> Yes, good point.
Ack, changing to { dg-do compile { target c++11 } } and committing
with the (correct :P) expectation that
the change is still ok with that change.
Re: [PATCH] c++: Fix access checking for __is_assignable and __is_constructible (c++/94197)
On Mon, 16 Mar 2020 at 23:25, Ville Voutilainen
wrote:
>
> Tested on Linux-PPC64.
>
> This ain't no regression. But it seems to hamper attempts to fix library
> regressions (see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94033).
It occurred to me that this can be done in one place.
2020-03-17 Ville Voutilainen
gcc/
PR c++/94197
* cp/method.c (assignable_expr): Use cp_unevaluated.
(is_xible_helper): Push a non-deferred access check for
the stub objects created by assignable_expr and constructible_expr.
testsuite/
PR c++/94197
* g++.dg/ext/pr94197.C: New.
diff --git a/gcc/cp/method.c b/gcc/cp/method.c
index 790d5704092..3427750f64e 100644
--- a/gcc/cp/method.c
+++ b/gcc/cp/method.c
@@ -1739,11 +1739,10 @@ check_nontriv (tree *tp, int *, void *)
static tree
assignable_expr (tree to, tree from)
{
- ++cp_unevaluated_operand;
+ cp_unevaluated cp_uneval_guard;
to = build_stub_object (to);
from = build_stub_object (from);
tree r = cp_build_modify_expr (input_location, to, NOP_EXPR, from, tf_none);
- --cp_unevaluated_operand;
return r;
}
@@ -1806,6 +1805,7 @@ constructible_expr (tree to, tree from)
static tree
is_xible_helper (enum tree_code code, tree to, tree from, bool trivial)
{
+ deferring_access_check_sentinel acs (dk_no_deferred);
if (VOID_TYPE_P (to) || ABSTRACT_CLASS_TYPE_P (to)
|| (from && FUNC_OR_METHOD_TYPE_P (from)
&& (TYPE_READONLY (from) || FUNCTION_REF_QUALIFIED (from
diff --git a/gcc/testsuite/g++.dg/ext/pr94197.C b/gcc/testsuite/g++.dg/ext/pr94197.C
new file mode 100644
index 000..768bfbac0a6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/pr94197.C
@@ -0,0 +1,75 @@
+// { dg-do compile }
+// { dg-options "-std=c++11" }
+
+template
+ T&& declval() noexcept;
+
+template
+struct bool_constant
+{
+ static constexpr bool value = B;
+ using type = bool_constant;
+};
+
+using true_type = bool_constant;
+using false_type = bool_constant;
+
+template
+ struct __is_nt_constructible_impl
+ : public false_type
+ { };
+
+template
+ struct __is_nt_constructible_impl
+ : public bool_constant(declval()))>
+ { };
+
+template
+ using __is_nothrow_constructible_impl
+= __is_nt_constructible_impl<__is_constructible(T, Arg), T, Arg>;
+
+template
+ struct __is_nothrow_copy_constructible_impl
+ : public __is_nothrow_constructible_impl
+ { };
+
+template
+ struct is_nothrow_copy_constructible
+ : public __is_nothrow_copy_constructible_impl::type
+ { };
+
+template
+ struct __is_nt_assignable_impl
+ : public false_type
+ { };
+
+template
+ struct __is_nt_assignable_impl
+ : public bool_constant() = declval())>
+ { };
+
+template
+ using __is_nothrow_assignable_impl
+= __is_nt_assignable_impl<__is_assignable(T, Arg), T, Arg>;
+
+template
+ struct __is_nothrow_copy_assignable_impl
+ : public __is_nothrow_assignable_impl
+ { };
+
+template
+ struct is_nothrow_copy_assignable
+ : public __is_nothrow_copy_assignable_impl::type
+ { };
+
+struct NType
+{
+ NType();
+private:
+ NType(const NType&);
+ NType& operator=(const NType&);
+};
+
+
+static_assert( !is_nothrow_copy_constructible::value, "" );
+static_assert( !is_nothrow_copy_assignable::value, "" );
[PATCH] c++: Fix access checking for __is_assignable and __is_constructible (c++/94197)
Tested on Linux-PPC64.
This ain't no regression. But it seems to hamper attempts to fix library
regressions (see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94033).
2020-03-16 Ville Voutilainen
gcc/
PR c++/94197
* cp/method.c (assignable_expr, constructible_expr): Push
a deferred access check for the stub object.
testsuite/
PR c++/94197
* g++.dg/ext/pr94197.C: New.
diff --git a/gcc/cp/method.c b/gcc/cp/method.c
index 790d5704092..b429ea48049 100644
--- a/gcc/cp/method.c
+++ b/gcc/cp/method.c
@@ -1739,11 +1739,11 @@ check_nontriv (tree *tp, int *, void *)
static tree
assignable_expr (tree to, tree from)
{
- ++cp_unevaluated_operand;
+ cp_unevaluated cp_uneval_guard;
+ deferring_access_check_sentinel acs (dk_no_deferred);
to = build_stub_object (to);
from = build_stub_object (from);
tree r = cp_build_modify_expr (input_location, to, NOP_EXPR, from, tf_none);
- --cp_unevaluated_operand;
return r;
}
@@ -1759,6 +1759,7 @@ constructible_expr (tree to, tree from)
{
tree expr;
cp_unevaluated cp_uneval_guard;
+ deferring_access_check_sentinel acs (dk_no_deferred);
if (CLASS_TYPE_P (to))
{
tree ctype = to;
diff --git a/gcc/testsuite/g++.dg/ext/pr94197.C b/gcc/testsuite/g++.dg/ext/pr94197.C
new file mode 100644
index 000..768bfbac0a6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/pr94197.C
@@ -0,0 +1,75 @@
+// { dg-do compile }
+// { dg-options "-std=c++11" }
+
+template
+ T&& declval() noexcept;
+
+template
+struct bool_constant
+{
+ static constexpr bool value = B;
+ using type = bool_constant;
+};
+
+using true_type = bool_constant;
+using false_type = bool_constant;
+
+template
+ struct __is_nt_constructible_impl
+ : public false_type
+ { };
+
+template
+ struct __is_nt_constructible_impl
+ : public bool_constant(declval()))>
+ { };
+
+template
+ using __is_nothrow_constructible_impl
+= __is_nt_constructible_impl<__is_constructible(T, Arg), T, Arg>;
+
+template
+ struct __is_nothrow_copy_constructible_impl
+ : public __is_nothrow_constructible_impl
+ { };
+
+template
+ struct is_nothrow_copy_constructible
+ : public __is_nothrow_copy_constructible_impl::type
+ { };
+
+template
+ struct __is_nt_assignable_impl
+ : public false_type
+ { };
+
+template
+ struct __is_nt_assignable_impl
+ : public bool_constant() = declval())>
+ { };
+
+template
+ using __is_nothrow_assignable_impl
+= __is_nt_assignable_impl<__is_assignable(T, Arg), T, Arg>;
+
+template
+ struct __is_nothrow_copy_assignable_impl
+ : public __is_nothrow_assignable_impl
+ { };
+
+template
+ struct is_nothrow_copy_assignable
+ : public __is_nothrow_copy_assignable_impl::type
+ { };
+
+struct NType
+{
+ NType();
+private:
+ NType(const NType&);
+ NType& operator=(const NType&);
+};
+
+
+static_assert( !is_nothrow_copy_constructible::value, "" );
+static_assert( !is_nothrow_copy_assignable::value, "" );
Re: [PATCH 1/1] libstdc++: Deal with ENOSYS == ENOTSUP
On Fri, 6 Mar 2020 at 11:52, Andreas Krebbel wrote: > > Hmm, what system does not have ENOSYS but has ENOTSUP? Meaning the > > !defined ENOSYS > > bit? > > > None that I know about. It is just to make sure the compare afterwards > operates on defined inputs. Ah, I see, indeed. This dance is done also for EOPNOTSUPP, looks good to me (but Jonathan still needs to do the approval).
Re: [PATCH 1/1] libstdc++: Deal with ENOSYS == ENOTSUP
On Fri, 6 Mar 2020 at 10:41, Andreas Krebbel wrote: > > zTPF uses the same numeric value for ENOSYS and ENOTSUP. > > Ok for mainline? > > libstdc++-v3/ChangeLog: > > 2020-03-06 Andreas Krebbel > > * src/c++11/system_error.cc: Omit the ENOTSUP case statement if it > would match ENOSYS. > --- > libstdc++-v3/src/c++11/system_error.cc | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/libstdc++-v3/src/c++11/system_error.cc > b/libstdc++-v3/src/c++11/system_error.cc > index 7844afe6d2a..1f06e67feea 100644 > --- a/libstdc++-v3/src/c++11/system_error.cc > +++ b/libstdc++-v3/src/c++11/system_error.cc > @@ -251,7 +251,8 @@ namespace > #ifdef ENOTSOCK >case ENOTSOCK: > #endif > -#ifdef ENOTSUP > +#if defined ENOTSUP && (!defined ENOSYS || ENOTSUP != ENOSYS) Hmm, what system does not have ENOSYS but has ENOTSUP? Meaning the !defined ENOSYS bit?
Re: [committed] libstdc++: Fix regression in std::move algorithm (PR 93872)
On Tue, 25 Feb 2020 at 15:36, Jonathan Wakely wrote:
> I think what I'd really like to do is get rid of __memmove entirely.
> We already have code that does the explicit assignment in a loop, for
> the cases where we can't use __builtin_memmove because the type is not
> trivially copyable.
>
> We should just use that existing code during constant evaluation, i.e.
> don't do the __builtin_memmove optimizations during constant
> evaluation. It seems much cleaner to just not use the optimization
> rather than wrap it to be usable in constant expressions.
>
> We already have to do that for {copy,move}_backward anyway, because
> __memmove doesn't correctly implement the std::memmove semantics for
> overlapping ranges. But we do it **wrong** and turn copy_backward into
> move_backward during constant evaluation.
>
> Here's a patch that gets rid of __memmove and fixes that bug
> (generated with 'git diff -b' so that the changes to the logic aren't
> obscured by the whitespace changes caused by re-indenting).
>
> Maybe I should just go ahead and do this now, since __memmove (and the
> problems it causes) are new for GCC 10 anyway. That would revert
> to something closer to the GCC 9 version.
Looks good to me.
[PATCH] libstd++: Library-side tests for parenthesized aggregate init (c++/92878, c++/92947)
This shebang adds library tests for all cases of parenthesized aggregate initialization that I could find. Tested locally on Linux-x64, going to test with full suite on Linux-PPC64, OK for trunk if tests pass? 2020-02-23 Ville Voutilainen Library-side tests for parenthesized aggregate init PR c++/92878 PR c++/92947 * testsuite/20_util/allocator_traits/members/92878_92947.cc: New. * testsuite/20_util/any/assign/92878_92947.cc: Likewise. * testsuite/20_util/any/cons/92878_92947.cc: Likewise. * testsuite/20_util/is_constructible/92878_92947.cc: Likewise. * testsuite/20_util/optional/assignment/92878_92947.cc: Likewise. * testsuite/20_util/optional/cons/92878_92947.cc: Likewise. * testsuite/20_util/pair/cons/92878_92947.cc: Likewise. * testsuite/20_util/shared_ptr/creation/92878_92947.cc: Likewise. * testsuite/20_util/specialized_algorithms/construct_at/92878_92947.cc: Likewise. * testsuite/20_util/unique_ptr/creation/92878_92947.cc: Likewise. * testsuite/20_util/uses_allocator/92878_92947.cc: Likewise. * testsuite/20_util/variant/92878_92947.cc: Likewise. * testsuite/23_containers/deque/modifiers/emplace/92878_92947.cc: Likewise. * testsuite/23_containers/forward_list/modifiers/92878_92947.cc: Likewise. * testsuite/23_containers/list/modifiers/emplace/92878_92947.cc: Likewise. * testsuite/23_containers/map/modifiers/emplace/92878_92947.cc: Likewise. * testsuite/23_containers/multimap/modifiers/emplace/92878_92947.cc: Likewise. * testsuite/23_containers/multiset/modifiers/emplace/92878_92947.cc: Likewise. * testsuite/23_containers/priority_queue/92878_92947.cc: Likewise. * testsuite/23_containers/queue/92878_92947.cc: Likewise. * testsuite/23_containers/set/modifiers/emplace/92878_92947.cc: Likewise. * testsuite/23_containers/stack/92878_92947.cc: Likewise. * testsuite/23_containers/unordered_map/modifiers/92878_92947.cc: Likewise. * testsuite/23_containers/unordered_multimap/modifiers/92878_92947.cc: Likewise. * testsuite/23_containers/unordered_multiset/modifiers/92878_92947.cc: Likewise. * testsuite/23_containers/unordered_set/modifiers/92878_92947.cc: Likewise. * testsuite/23_containers/vector/modifiers/emplace/92878_92947.cc: Likewise. aggr-init-lib-tests.diff.gz Description: application/gzip
Re: Implement the part of C++20 p1032 Misc constexpr bits.
On Wed, 20 Nov 2019 at 12:16, Christophe Lyon
wrote:
>
> On Wed, 20 Nov 2019 at 11:10, Ville Voutilainen
> wrote:
> >
> > On Wed, 20 Nov 2019 at 11:47, Christophe Lyon
> > wrote:
> > >
> > > On Thu, 14 Nov 2019 at 16:55, Jonathan Wakely wrote:
> > > >
> > > > On 09/11/19 02:07 +, Smith-Rowland, Edward M wrote:
> > > > >Here is the part of C++20 p1032 Misc constexpr bits.
> > > > >
> > > > >Tested on x86_64-linux. OK?
> > > >
> > > > OK for trunk, thanks.
> > > >
> > >
> > > Hi,
> > >
> > > The new test constexpr_allocator_arg_t.cc fails on arm and aarch64 and
> > > many other targets according to gcc-testresults.
> > > Is that expected?
> >
> > No, that's not expected. Can you give a link to the build log?
>
> On (cross) aarch64, I can see:
> FAIL: 20_util/tuple/cons/constexpr_allocator_arg_t.cc (test for excess errors)
> Excess errors:
> /libstdc++-v3/testsuite/20_util/tuple/cons/constexpr_allocator_arg_t.cc:48:
> error: non-constant condition for static assertion
> /libstdc++-v3/testsuite/20_util/tuple/cons/constexpr_allocator_arg_t.cc:48:
> in 'constexpr' expansion of 'test_tuple()'
> /libstdc++-v3/testsuite/20_util/tuple/cons/constexpr_allocator_arg_t.cc:31:
> in 'constexpr' expansion of 'ta.std::tuple double>::tuple >(std::allocator_arg, alloc)'
> /aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/gcc3/aarch64-none-linux-gnu/libstdc++-v3/include/tuple:705:
> error: 'constexpr std::_Tuple_impl<_Idx, _Head, _Tail
> ...>::_Tuple_impl(std::allocator_arg_t, const _Alloc&) [with _Alloc =
> std::allocator; long unsigned int _Idx = 0; _Head = int; _Tail =
> {double, double}]' called in a constant expression
> /aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/gcc3/aarch64-none-linux-gnu/libstdc++-v3/include/tuple:251:
> error: call to non-'constexpr' function 'std::__uses_alloc_t<_Tp,
> _Alloc, _Args ...> std::__use_alloc(const _Alloc&) [with _Tp = int;
> _Alloc = std::allocator; _Args = {}; std::__uses_alloc_t<_Tp,
> _Alloc, _Args ...> = std::__uses_alloc_t >]'
>
>
> I see it failing at r278333, was it fixed since then?
Yes, in r278373.
Re: Implement the part of C++20 p1032 Misc constexpr bits.
On Wed, 20 Nov 2019 at 11:47, Christophe Lyon wrote: > > On Thu, 14 Nov 2019 at 16:55, Jonathan Wakely wrote: > > > > On 09/11/19 02:07 +, Smith-Rowland, Edward M wrote: > > >Here is the part of C++20 p1032 Misc constexpr bits. > > > > > >Tested on x86_64-linux. OK? > > > > OK for trunk, thanks. > > > > Hi, > > The new test constexpr_allocator_arg_t.cc fails on arm and aarch64 and > many other targets according to gcc-testresults. > Is that expected? No, that's not expected. Can you give a link to the build log?
Re: [PATCH][Hashtable 5/6] Remove H1/H2 template parameters
On Mon, 18 Nov 2019 at 23:41, François Dumont wrote: > > Also, is > > this ABI-compatible > > for our unordered containers? > > > IMHO, yes it is. > > In hashtable_policy.h _H1 was the user hash which I renamed in _Hash, > the same template name that for unordered containers. > > _H2 was always _Mod_range_hashing and previous _Hash always > _Default_ranged_hash, both stateless types. Only _H1 and _H2 were stored > in _Hash_code_base for instance. _H1 is still as _Hash and _H2 was just > empty and moreover stored last so removing it has no impact. Right, and as I understood the code, it's all empty base objects anyway, so it boils down to whether a set of empty bases can change without causing an ABI break, and I don't think there's an ABI problem here because the actual _Hashtable is stored as a member of various containers, and its size doesn't change, and based on what you wrote, with which my code-reading agrees, the behaviour doesn't change either. So the patch seems okay to me.
Re: [PATCH][Hashtable 5/6] Remove H1/H2 template parameters
On Sun, 17 Nov 2019 at 23:15, François Dumont wrote: > > H1 used to be a reference to the user Hash, now _Hashtable and unordered > types agree on the same Hash type which is more intuitive. > > I also chose to not support anymore a stateful ranged hash functor. We > only use _Mod_range_hashing and _Mask_range_hashing. > > Thanks to this simplification _M_bucket_index can also be simplified. Do we know whether there are existing users that this breaks? Also, is this ABI-compatible for our unordered containers?
Re: [PATCH, libstdc++] Implement C++20 p1032 default_searcher constexprosity.
On Fri, 15 Nov 2019 at 22:16, Smith-Rowland, Edward M wrote: > > Pretty self-explanatory. LGTM. Jonathan still needs to ack it.
Re: [PATCH 1/3] C++20 constexpr lib part 1/3
On Sat, 6 Jul 2019 at 06:12, Ed Smith-Rowland via libstdc++ wrote: > By my reckoning, you have a constexpr source array, an output array that > is initialized as it must be for constexpr.?? You have to have a > deterministic result after the copy.?? In the local array version the > actual iterator is not leaking out - just the results of a calculation > that must return one result. > > The only thing that 'helps' is removing the constexpr from the iterator > return and of course that's the whole point of the thing. > > Blargh! But that's fine; the result of copy is not stored in a constexpr variable, but the function return is static_asserted so we have sufficiently tested that std::copy is indeed constexpr-compatible since it appears in a function that is evaluated at compile-time.
Re: [PATCH 1/3] C++20 constexpr lib part 1/3
On Thu, 27 Jun 2019 at 19:55, Ed Smith-Rowland via libstdc++
wrote:
> > I don't think this will work in a constant expression:
> >
> > ?? /// Assign @p __new_val to @p __obj and return its previous value.
> > ?? template
> > +?? _GLIBCXX20_CONSTEXPR
> > ?? inline _Tp
> > ?? exchange(_Tp& __obj, _Up&& __new_val)
> > ?? { return std::__exchange(__obj, std::forward<_Up>(__new_val)); }
> >
> > Because std::__exchange hasn't been marked constexpr. The test passes
> > because it doesn't call it in a context that requires constant
> > evaluation:
> >
> > ??const auto x = std::exchange(e, pi);
> Derp.
> >
> > I see the same problem in other tests too:
> >
> > +?? constexpr std::array car{{0, 1, 2, 3, 4, 5, 6, 6, 8, 9, 9,
> > 11}};
> > +
> > +?? const auto out0x = std::adjacent_find(car.begin(), car.end());
> > +
> > +?? const auto out1x = std::adjacent_find(car.begin(), car.end(),
> > +?? std::equal_to())
>
> I will go through all the tests and make sure that some nontrivial
> calculation is done that contributes to a final bool return.?? And clean
> up the mess.?? I did this in chunk 2 but I guess I left a lot of chunk 1.
As was the case with the iterator patch, it's not a question of doing
a nontrivial calculation, but
a question of initializing a constexpr variable with the result. (Yeah
sure a non-type template
argument would also work but eurgh). Then, and only then, have we
proven that the code
works in a constexpr context. Initializing a const doesn't do that.
constinit would, but that's
C++20.
Re: [PATCH] PR libstdc++/71579 assert that type traits are not misused with an incomplete type
On Thu, 20 Jun 2019 at 20:49, Antony Polukhin wrote:
>
> чт, 6 июн. 2019 г. в 15:19, Jonathan Wakely :
> > I'm removing some of these assertions again, because they are either
> > reundant or wrong.
>
> Thanks for cleaning up!
>
>
> In attachment there is an additional patch for type traits hardening.
>
> Things that still remain unasserted are type traits with variadic
> template arguments. I have to came up with a proper solution for
> providing a useful and lightweight diagnostics.
I see a
public __bool_constant<__is_trivially_assignable(_Tp, _Up)>
in this patch, followed by a trait-body that static_asserts. In such
cases, I think we want
to
a) be really careful about duplicating compiler diagnostics with library ones
b) look at the compiler diagnostics, and if they are lacking, improve them.
...because that's what Jonathan's cleanup was really about.
In the test modifications of __is_trivially_assignable, this looks bloody
suspicious:
+// { dg-prune-output "invalid use of incomplete type" }
+// { dg-prune-output "must be a complete" }
No. Don't merge. We are not replacing diagnostics A with diagnostics
B, we are ignoring existing
diagnostics and adding more. Which is exactly what Jonathan's cleanup avoided.
Re: Test for C++20 p0858 - ConstexprIterator requirements.
On Mon, 10 Jun 2019 at 02:53, Ed Smith-Rowland <3dw...@verizon.net> wrote: > Darn it, I had those constexpr lib patches in tree. > Attached are what I just committed to gcc-9 and passes there. Those > std::copy didn't really add anything anyway. They added a test that *i++ = *j++ works, and that i != j works.
Re: Test for C++20 p0858 - ConstexprIterator requirements.
On Mon, 10 Jun 2019 at 01:03, Rainer Orth wrote: > > Hi Ed, > > >>> I had supplied the option for gnu++2a by hand and they passed.?? They > >>> were not UNSUPPORTED. > >>> > >>> I just added the dg-options (at very top) and reran the testsuite > >>> without fancy tricks (except for gnu++2a). > >>> > >>> I also took out the #if __cplusplus.?? I was just playing around and > >>> discovered that these pass in C++17 if you comment out the C++20 > >>> constexpr algos. > >>> > >>> OK for trunk? > >> > >> OK for trunk. > > Committed 272084. > > 272085 actually ;-) Unfortunately, the new tests seem to FAIL (almost?) > everywhere: > > +FAIL: 21_strings/basic_string_view/requirements/constexpr_iter.cc (test for > excess errors) > > Excess errors: > /vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/21_strings/basic_string_view/requirements/constexpr_iter.cc:33: > error: call to non-'constexpr' function '_OI std::copy(_II, _II, _OI) [with > _II = const char*; _OI = int*]' > /vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/21_strings/basic_string_view/requirements/constexpr_iter.cc:41: > error: 'constexpr char test()' called in a constant expression > > +FAIL: 23_containers/array/requirements/constexpr_iter.cc (test for excess > errors) > > /vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/23_containers/array/requirements/constexpr_iter.cc:32: > error: call to non-'constexpr' function '_OI std::copy(_II, _II, _OI) [with > _II = const int*; _OI = int*]' > /vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/23_containers/array/requirements/constexpr_iter.cc:40: > error: 'constexpr int test()' called in a constant expression > > I'm seeing those on i386-pc-solaris2.11 and sparc-sun-solaris2.11, and > there are gcc-testresults reports on aarch64-unknown-linux-gnu, > i686-pc-linux-gnu, powerpc64le-unknown-linux-gnu, and > x86_64-pc-linux-gnu, among others. > > Please fix. Indeed. std::copy isn't constexpr yet. I don't see how Ed's test run can pass. We either need to put this on hold until enough of is constexprified, or we need to use loops in this test and test (constexpr) algorithms' use of (constexpr) iterators separately later.
Re: Test for C++20 p0858 - ConstexprIterator requirements.
On Sat, 1 Jun 2019 at 22:40, Ed Smith-Rowland <3dw...@verizon.net> wrote: > Ok, third time's a charm. > > I was brain dead about the constexpr patch.?? I'm now setting a constexpr > variable from test() in a caller. Looks good. Jonathan needs to approve it, though. > But static_assert is a constexpr context no? Yes, it is. It's just that now all of test() is verified to be okay as a constant expression, including std::copy.
Re: Test for C++20 p0858 - ConstexprIterator requirements.
On Sat, 1 Jun 2019 at 21:09, Ed Smith-Rowland <3dw...@verizon.net> wrote:
>
> On 5/31/19 6:29 PM, Ville Voutilainen wrote:
> > On Sat, 1 Jun 2019 at 01:24, Ed Smith-Rowland via libstdc++
> > wrote:
> >> Greetings,
> >>
> >> Iterators for and are usabe in a constexpr context
> >> since C++2017.
> >>
> >> This just adds a compile test to make sure and check a box for C++20
> >> p0858 - ConstexprIterator requirements.
> >
> > Those tests don't use the iterators in a constexpr context. To do
> > that, maybe do those std::copy operations
> > in a constexpr function and then initialize a constexpr variable with
> > the result of a call to that function?
>
> Thanks Ville,
>
> I had completely forgotten to make these test functions constexpr - FIXED.
..but that doesn't enforce a constexpr context. If you add another
function that calls these functions
and initializes a constexpr variable, then we have the enforcement I
seek. Such as
void test2()
{
constexpr char x = test();
}
PR c++/85254
Tested on Linux-PPC64, acked by Jason on irc. Applying to trunk
on Saturday.
2019-06-01 Ville Voutilainen
gcc/cp
PR c++/85254
* class.c (fixup_type_variants): Handle CLASSTYPE_FINAL.
testsuite/
PR c++/85254
* g++.dg/ext/is_final.C: Amend.
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index a2585a6..d6ac6ce 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -1907,6 +1907,7 @@ fixup_type_variants (tree t)
= TYPE_HAS_NONTRIVIAL_DESTRUCTOR (t);
TYPE_POLYMORPHIC_P (variants) = TYPE_POLYMORPHIC_P (t);
+ CLASSTYPE_FINAL (variants) = CLASSTYPE_FINAL (t);
TYPE_BINFO (variants) = TYPE_BINFO (t);
diff --git a/gcc/testsuite/g++.dg/ext/is_final.C b/gcc/testsuite/g++.dg/ext/is_final.C
index b3875ad..20e5d62 100644
--- a/gcc/testsuite/g++.dg/ext/is_final.C
+++ b/gcc/testsuite/g++.dg/ext/is_final.C
@@ -43,3 +43,17 @@ static_assert( __is_final (Ff), "Ff is final" );
static_assert( __is_final (Ff), "Ff is final" );
static_assert( __is_final (Ff), "Ff is final" );
+// PR 85254
+
+template struct final_trait_wrap{ typedef T type; };
+
+template struct my_is_final
+{
+ static const bool value = __is_final(typename final_trait_wrap::type);
+};
+
+struct final1 final {};
+template struct final2 final {};
+
+static_assert( my_is_final::value, "final1 is final" );
+static_assert( my_is_final>::value, "final2 is final" );
Re: Test for C++20 p0858 - ConstexprIterator requirements.
On Sat, 1 Jun 2019 at 01:24, Ed Smith-Rowland via libstdc++ wrote: > > Greetings, > > Iterators for and are usabe in a constexpr context > since C++2017. > > This just adds a compile test to make sure and check a box for C++20 > p0858 - ConstexprIterator requirements. Those tests don't use the iterators in a constexpr context. To do that, maybe do those std::copy operations in a constexpr function and then initialize a constexpr variable with the result of a call to that function?
Re: RFC: [PATCH] Remove using-declarations that add std names to __gnu_cxx
On Wed, 29 May 2019 at 23:00, Jonathan Wakely wrote: > Does anybody think we should keep __gnu_cxx::size_t, > __gnu_cxx::input_iterator_tag, __gnu_cxx::vector, __gnu_cxx::pair etc. > or should I go ahead and commit this? +1 go ahead.
Re: [PATCH] Changes to std::variant to reduce code size
On Thu, 16 May 2019 at 23:28, Jonathan Wakely wrote: > Here's what I've tested and am about to commit. Looks good to me.
Re: [PATCH] PR libstdc++/90388 fix std::hash> bugs
On Sat, 11 May 2019 at 00:55, Jonathan Wakely wrote: > > On 11/05/19 00:51 +0300, Ville Voutilainen wrote: > >On Sat, 11 May 2019 at 00:42, Jonathan Wakely wrote: > >> > >> A disabled specialization should not be callable, so move the function > >> call operator into a new base class which correctly implements the > >> disabled hash semantics. For the versioned namespace configuration do > >> not derive from __poison_hash in the enabled case, as the empty base > >> class serves no purpose but potentially increases the object size. For > >> the default configuration that base class must be kept, to preserve > >> layout. > > > >I continue to not be a fan of the versioned namespace ifdeffery in > >this, but I can live with it. > > The versioned namespace configuration should be as good as we can make > it, with no limitations due to ABI compatibility. Removing redundant > base classes allows more compact class layouts. With current trunk > this type has size 3, in the versioned namespace after my patch it has > size 2 (and if we patched hash> it would have size 1): I understand all of that, but I do question the value of optimizing the versioned namespace configuration at the cost of sprinkling such ifdefs into our code.
Re: [PATCH] PR libstdc++/90388 fix std::hash> bugs
On Sat, 11 May 2019 at 00:42, Jonathan Wakely wrote: > > A disabled specialization should not be callable, so move the function > call operator into a new base class which correctly implements the > disabled hash semantics. For the versioned namespace configuration do > not derive from __poison_hash in the enabled case, as the empty base > class serves no purpose but potentially increases the object size. For > the default configuration that base class must be kept, to preserve > layout. I continue to not be a fan of the versioned namespace ifdeffery in this, but I can live with it.
Re: Adding noexcept-specification on tuple constructors (LWG 2899)
On Wed, 24 Apr 2019 at 14:53, Jonathan Wakely wrote: > > On 24/04/19 11:21 +0100, Nina Dinka Ranns wrote: > >New diff attached. > > Thanks, this looks great. I think we can apply this as soon as stage 1 > begins (which should be Real Soon Now). Tested on Linux-PPC64, committed to trunk. Congrats, Nina, first patch in, let there be many more. :)
Re: Adding noexcept-specification on tuple constructors (LWG 2899)
On Mon, 29 Apr 2019 at 00:18, Ville Voutilainen wrote: > > On Wed, 24 Apr 2019 at 14:53, Jonathan Wakely wrote: > > > > On 24/04/19 11:21 +0100, Nina Dinka Ranns wrote: > > >New diff attached. > > > > Thanks, this looks great. I think we can apply this as soon as stage 1 > > begins (which should be Real Soon Now). > > Tested on Linux-PPC64, committed to trunk. Congrats, Nina, first patch > in, let there be many more. :) And as I was corrected, this was the second patch, but that doesn't change the suggestion that there should be many more. :P
Re: [PATCH] Cleanup algorithm implementations
On Fri, 19 Apr 2019 at 22:00, Thomas Rodgers wrote: > > * include/pstl/glue_algorithm_impl.h (stable_sort): Forward > execution policy. > (mismatch): Forward execution policy. > (equal): Qualify call to std::equal(). > (partial_sort): Forward execution policy. > (inplace_merge): Forward execution policy. +1, looks good to me.
Re: [PATCH 3/3] Fix condition for std::variant to be copy constructible
On Wed, 17 Apr 2019 at 19:12, Jonathan Wakely wrote: > > The standard says the std::variant copy constructor is defined as > deleted unless all alternative types are copy constructible, but we were > making it also depend on move constructible. Fix the condition and > enhance the tests to check the semantics with pathological copy-only > types (i.e. supporting copying but having deleted moves). > > The enhanced tests revealed a regression in copy assignment for > non-trivial alternative types, where the assignment would not be > performed because the condition in the _Copy_assign_base visitor is > false: is_same_v, remove_reference_t>. > > > Tested powerpc64le-linux. > > I plan to commit all three of these patches later today, unless > somebody sees a problem with them. Looks good to me.
