Breaking change for libc++ incoming (Minor, C++17 only)

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Breaking change for libc++ incoming (Minor, C++17 only)

Sumner, Brian via cfe-dev
In C++17, we introduced a new variadic class called scoped_lock.
It took a set of mutexes, and locked them all (in the constructor) and unlocked them all (in the destructor).  It also has an option to "adopt" the locks (i.e, you can pass it already locked mutexes and it will not lock them again).

The constructors look like this (as originally defined):

class scoped_lock {
...
    explicit scoped_lock(MutexTypes&... m);
    scoped_lock(MutexTypes&... m, adopt_lock_t);
...
};

but this caused some problems with template deduction (specifically in deduction guides)
So, at the last minute in the C++17 cycle, the committee decided to change the second constructor to take the `adopt_lock` flag AT THE START.

class scoped_lock {
...
    explicit scoped_lock(MutexTypes&... m);
    scoped_lock(adopt_lock_t, MutexTypes&... m);
...
};

scoped_lock is a new feature in C++17, and so the installed base of people using it should be very small. There are no uses of it in the LLVM code base outside of the libc++ test suite.

I'm going to make this change to bring libc++ in conformance with the (hopefully final) standard, and people who are using the old call will have to update their code. Sorry about that.

-- Marshall

P.S. Only people who are using the second constructor (the one with adopt_lock) will be affected by this change.


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Breaking change for libc++ incoming (Minor, C++17 only)

Sumner, Brian via cfe-dev

Will libc++ make any policy changes regarding implementations of unreleased standards as a result of this?  Perhaps all future-standard work should go in an experimental namespace, or be hidden behind an ifdef of some sort?

 

Or is the occasional break like this considered acceptable in the long term, so long as the breaking feature hasn’t received large adoption?

 

From: cfe-dev [mailto:[hidden email]] On Behalf Of Marshall Clow via cfe-dev
Sent: Thursday, July 27, 2017 12:05 PM
To: [hidden email]
Subject: [cfe-dev] Breaking change for libc++ incoming (Minor, C++17 only)

 

In C++17, we introduced a new variadic class called scoped_lock.

It took a set of mutexes, and locked them all (in the constructor) and unlocked them all (in the destructor).  It also has an option to "adopt" the locks (i.e, you can pass it already locked mutexes and it will not lock them again).

 

The constructors look like this (as originally defined):

 

class scoped_lock {

...

    explicit scoped_lock(MutexTypes&... m);

    scoped_lock(MutexTypes&... m, adopt_lock_t);

...

};

 

but this caused some problems with template deduction (specifically in deduction guides)

So, at the last minute in the C++17 cycle, the committee decided to change the second constructor to take the `adopt_lock` flag AT THE START.

 

class scoped_lock {

...

    explicit scoped_lock(MutexTypes&... m);

    scoped_lock(adopt_lock_t, MutexTypes&... m);

...

};

 

scoped_lock is a new feature in C++17, and so the installed base of people using it should be very small. There are no uses of it in the LLVM code base outside of the libc++ test suite.

 

I'm going to make this change to bring libc++ in conformance with the (hopefully final) standard, and people who are using the old call will have to update their code. Sorry about that.

 

-- Marshall

 

P.S. Only people who are using the second constructor (the one with adopt_lock) will be affected by this change.

 


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Breaking change for libc++ incoming (Minor, C++17 only)

Sumner, Brian via cfe-dev
In reply to this post by Sumner, Brian via cfe-dev
On Thu, Jul 27, 2017 at 10:04 AM, Marshall Clow via cfe-dev
<[hidden email]> wrote:

> In C++17, we introduced a new variadic class called scoped_lock.
> It took a set of mutexes, and locked them all (in the constructor) and
> unlocked them all (in the destructor).  It also has an option to "adopt" the
> locks (i.e, you can pass it already locked mutexes and it will not lock them
> again).
>
> The constructors look like this (as originally defined):
>
> class scoped_lock {
> ...
>     explicit scoped_lock(MutexTypes&... m);
>     scoped_lock(MutexTypes&... m, adopt_lock_t);
> ...
> };
>
> but this caused some problems with template deduction (specifically in
> deduction guides)
> So, at the last minute in the C++17 cycle, the committee decided to change
> the second constructor to take the `adopt_lock` flag AT THE START.
>
> class scoped_lock {
> ...
>     explicit scoped_lock(MutexTypes&... m);
>     scoped_lock(adopt_lock_t, MutexTypes&... m);
> ...
> };
>
> scoped_lock is a new feature in C++17, and so the installed base of people
> using it should be very small. There are no uses of it in the LLVM code base
> outside of the libc++ test suite.
>
> I'm going to make this change to bring libc++ in conformance with the
> (hopefully final) standard, and people who are using the old call will have
> to update their code. Sorry about that.

Was r298681 the first commit that added scoped_lock? If so it hasn't
shipped in any LLVM releases yet, and perhaps we should merge the
upcoming fix to 5.0.0?

 - Hans
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Breaking change for libc++ incoming (Minor, C++17 only)

Sumner, Brian via cfe-dev
In reply to this post by Sumner, Brian via cfe-dev


On Thu, Jul 27, 2017 at 10:12 AM, Ben Craig <[hidden email]> wrote:

Will libc++ make any policy changes regarding implementations of unreleased standards as a result of this? 


The only policy change I can think of for this is a reminder that things in unreleased standards are subject to change.  Maybe put that on http://libcxx.llvm.org/cxx1z_status.html (and now http://libcxx.llvm.org/cxx2a_status.html)
 

Perhaps all future-standard work should go in an experimental namespace, or be hidden behind an ifdef of some sort?


These were already behind an #ifdef - you had to use -std=c++1z to get them.
 

 Or is the occasional break like this considered acceptable in the long term, so long as the breaking feature hasn’t received large adoption?


In general, I'd like to avoid all breaking changes.

But I'm also uninterested in supporting things that were in a draft standard and then removed/changed before the standard became final.

The other alternative is to delay implementing things in libc++ until the standard is ratified.


-- Marshall
  

 

From: cfe-dev [mailto:[hidden email]] On Behalf Of Marshall Clow via cfe-dev
Sent: Thursday, July 27, 2017 12:05 PM
To: [hidden email]
Subject: [cfe-dev] Breaking change for libc++ incoming (Minor, C++17 only)

 

In C++17, we introduced a new variadic class called scoped_lock.

It took a set of mutexes, and locked them all (in the constructor) and unlocked them all (in the destructor).  It also has an option to "adopt" the locks (i.e, you can pass it already locked mutexes and it will not lock them again).

 

The constructors look like this (as originally defined):

 

class scoped_lock {

...

    explicit scoped_lock(MutexTypes&... m);

    scoped_lock(MutexTypes&... m, adopt_lock_t);

...

};

 

but this caused some problems with template deduction (specifically in deduction guides)

So, at the last minute in the C++17 cycle, the committee decided to change the second constructor to take the `adopt_lock` flag AT THE START.

 

class scoped_lock {

...

    explicit scoped_lock(MutexTypes&... m);

    scoped_lock(adopt_lock_t, MutexTypes&... m);

...

};

 

scoped_lock is a new feature in C++17, and so the installed base of people using it should be very small. There are no uses of it in the LLVM code base outside of the libc++ test suite.

 

I'm going to make this change to bring libc++ in conformance with the (hopefully final) standard, and people who are using the old call will have to update their code. Sorry about that.

 

-- Marshall

 

P.S. Only people who are using the second constructor (the one with adopt_lock) will be affected by this change.

 



_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Breaking change for libc++ incoming (Minor, C++17 only)

Sumner, Brian via cfe-dev
In reply to this post by Sumner, Brian via cfe-dev


On Thu, Jul 27, 2017 at 10:32 AM, Hans Wennborg <[hidden email]> wrote:
On Thu, Jul 27, 2017 at 10:04 AM, Marshall Clow via cfe-dev
<[hidden email]> wrote:
> In C++17, we introduced a new variadic class called scoped_lock.
> It took a set of mutexes, and locked them all (in the constructor) and
> unlocked them all (in the destructor).  It also has an option to "adopt" the
> locks (i.e, you can pass it already locked mutexes and it will not lock them
> again).
>
> The constructors look like this (as originally defined):
>
> class scoped_lock {
> ...
>     explicit scoped_lock(MutexTypes&... m);
>     scoped_lock(MutexTypes&... m, adopt_lock_t);
> ...
> };
>
> but this caused some problems with template deduction (specifically in
> deduction guides)
> So, at the last minute in the C++17 cycle, the committee decided to change
> the second constructor to take the `adopt_lock` flag AT THE START.
>
> class scoped_lock {
> ...
>     explicit scoped_lock(MutexTypes&... m);
>     scoped_lock(adopt_lock_t, MutexTypes&... m);
> ...
> };
>
> scoped_lock is a new feature in C++17, and so the installed base of people
> using it should be very small. There are no uses of it in the LLVM code base
> outside of the libc++ test suite.
>
> I'm going to make this change to bring libc++ in conformance with the
> (hopefully final) standard, and people who are using the old call will have
> to update their code. Sorry about that.

Was r298681 the first commit that added scoped_lock? If so it hasn't
shipped in any LLVM releases yet, and perhaps we should merge the
upcoming fix to 5.0.0?


Absolutely .

-- Marshall



_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Breaking change for libc++ incoming (Minor, C++17 only)

Sumner, Brian via cfe-dev
In reply to this post by Sumner, Brian via cfe-dev
On Thu, Jul 27, 2017 at 10:32 AM, Hans Wennborg <[hidden email]> wrote:
On Thu, Jul 27, 2017 at 10:04 AM, Marshall Clow via cfe-dev
<[hidden email]> wrote:
> In C++17, we introduced a new variadic class called scoped_lock.
> It took a set of mutexes, and locked them all (in the constructor) and
> unlocked them all (in the destructor).  It also has an option to "adopt" the
> locks (i.e, you can pass it already locked mutexes and it will not lock them
> again).
>
> The constructors look like this (as originally defined):
>
> class scoped_lock {
> ...
>     explicit scoped_lock(MutexTypes&... m);
>     scoped_lock(MutexTypes&... m, adopt_lock_t);
> ...
> };
>
> but this caused some problems with template deduction (specifically in
> deduction guides)
> So, at the last minute in the C++17 cycle, the committee decided to change
> the second constructor to take the `adopt_lock` flag AT THE START.
>
> class scoped_lock {
> ...
>     explicit scoped_lock(MutexTypes&... m);
>     scoped_lock(adopt_lock_t, MutexTypes&... m);
> ...
> };
>
> scoped_lock is a new feature in C++17, and so the installed base of people
> using it should be very small. There are no uses of it in the LLVM code base
> outside of the libc++ test suite.
>
> I'm going to make this change to bring libc++ in conformance with the
> (hopefully final) standard, and people who are using the old call will have
> to update their code. Sorry about that.

Was r298681 the first commit that added scoped_lock? If so it hasn't
shipped in any LLVM releases yet, and perhaps we should merge the
upcoming fix to 5.0.0?


Committed as revision 309296 
I'll watch the bots and let you know when it's ready to be merged into 5.0

-- Marshall


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Loading...