[cfe-dev] Discussion: should we enforce access control in C++ attributes?

[Delesley Hutchins]

> Could this function not be defined? Possibly defined as deleted (=
> delete)? (not sure whether it makes sense to allow calls to deleted
> functions in the unevaluated context of a thread safety attribute (I
> think/assume we don't allow calls to deleted functions in other
> unevaluated contexts (which is where a deleted function would differ
> from an undefined function))).

Yes, it can be left undefined, and that's what I used to do in some of
our code.  However, a reviewer pointed out to me that if it's
undefined, anyone wanting the mutex can get it by supplying a
definition (perhaps even thinking that the lack of definition was
unintentional) so it was safer to define the method and return 0.

Marking it as deleted is an interesting idea that I hadn't thought of.
 However, instead of turning off the access control checks inside
attributes, I'd have to turn off the deleted checks instead.  Once
again, that would turn off the checks for every part of the expression
in an attribute, not just the parts that refer to the mutex.  I'm not
sure that's a good idea.

> Have you considered doing something like C++11's noexcept operator (
> http://en.cppreference.com/w/cpp/language/noexcept )? In which you
> could provide an expression & whatever locks are required by that
> expression are required by this "Locks Required" clause (or other
> properties you're interested in capturing in the locking annotations)
>
> So in this case:
>
>   virtual void addTask(int taskID)
> EXCLUSIVE_LOCKS_REQUIRED(required_locks(TaskQueue::addTask(taskID)));
>
> or something to that effect. This encapsulates the contract better &
> without code duplication - if the contract of the base addTask
> changes, so does the derived implementation (or the wrapper function,
> or the whatever else needs this). Relying on the implementation
> details of the functions you're calling & duplicating their contracts
> explicitly seems sub-optimal compared to propagating the important
> expressions up.

Another interesting idea.  :-)  Yes, I could do this.  In the case of
virtual methods, the best thing is really to propogate LOCKS_REQUIRED
automatically, since it's actually a type safety violation to
redeclare it anyway.  I included the virtual method primarily for
comparison with the private type example later.  However, your
technique would also work for external functions like addTwoTasks, so
it's definitely a viable option.

I guess my main concern here is that
EXCLUSIVE_LOCKS_REQUIRED(required_locks(TaskQueue::addTask(taskID)))
is a lot less clear than EXCLUSIVE_LOCKS_REQUIRED(mu).  Moreover, for
more complicated methods, it's not feasible to encapsulate the
contract in a completely general manner -- you'd have to list every
method that might require a mutex.  I think this idea would be really
useful in certain cases, such as templates (where noexcept is also
most useful), but I'm pretty sure I'm going to get some major pushback
from users if I recommend this as the officially "blessed" way to do
thread annotations.  :-)

Thanks for the suggestions,

  -DeLesley