[PATCH] D41933: handle scoped_lockable objects being returned by value in C++17

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith created this revision.
rsmith added reviewers: aaron.ballman, delesley, dblaikie.
Herald added a subscriber: sanjoy.

In C++17, guaranteed copy elision means that there isn't necessarily a constructor call when a local variable is initialized by a function call that returns a `scoped_lockable` by value. That causes code such as:

  // see test for definition of ReadLockedPtr
  ReadLockedPtr<X> get();
  void f() {
    auto my_ptr = get();
    my_ptr->do_something_requiring_lock();
  }

... to fail for two reasons: the lock `my_ptr->mutex` is not held in the `do_something_requiring_lock()` call, and the lock `my_ptr` is not held at the point of the implicit destructor call at the end of the function.

There isn't really a good way to work around this: we don't have a list of "these are the locks you should expect to be held when an object is returned by value". But we can approximate that by pretending that the local `my_ptr` variable was actually initialized by running the move constructor of `ReadLockedPtr`, which is what this patch does.

[Note that this doesn't address the more problematic case where the returned type has no copy or move constructor. If that comes up, it can be hacked around with something like:

  struct SCOPED_LOCKABLE MyImmovableButReturnableScopedLock {
    MyImmovableButReturnableScopedLock(MyImmovableButReturnableScopedLock &&) = delete;
    MyImmovableButReturnableScopedLock(volatile MyImmovableButReturnableScopedLock&&) ATTRIBUTES_HERE; // not defined
  };

... on the basis that no sane program will ever actually call the other move constructor.]

Ideas for alternative approaches welcome. Ideally I'd like something that allows existing C++<=14 code to continue to work in C++17 mode unchanged, which I think this approach mostly provides.


Repository:
  rC Clang

https://reviews.llvm.org/D41933

Files:
  lib/Analysis/ThreadSafety.cpp
  test/SemaCXX/warn-thread-safety-analysis.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D41933.129385.patch
Type: text/x-patch
Size: 5140 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180111/85147a39/attachment-0001.bin>