Thanks for pointing out the static data member issue; I'll definitely need to fix that. In some cases, clang expands the AST to better reflect the semantics of the expression (e.g. inserting an implicit cast, or converting myvar to this->myvar for an ordinary data member), while in other cases it doesn't (e.g. not converting this->lock to S::lock below). I still haven't got my head around all the details. :-) However, it is clear to me that I'll need to do a substantial overhaul of the mutexID stuff to do a better job of canonicalizing lock expressions; that will resolve issues like this one.
-DeLesley On Tue, Jan 17, 2012 at 11:02 AM, Richard Smith <[email protected]> wrote: > Hi, > > On Fri, January 13, 2012 22:39, Delesley Hutchins wrote: >> This patch modifies the thread safety analysis, so that 'this' can be >> used as a valid lock expression. > > Using '0' to represent 'this' is a little subtle; I'd like to see a comment on > either MutexID or MutexID::DeclSeq calling this out. With that, LGTM. > > One observation: this change breaks code which uses 'this->m' to name a mutex > which is a static data member (such a reference no longer matches against > naming the mutex as 'Class::m'). However, code which used 'p->m' was already > broken in this way, and the fix for that more general case should take care of > the 'this' case too, so I don't think that is an issue with this patch per se. > Testcases: > > struct __attribute__((lockable)) S { > static S *lock; > static int n __attribute__((guarded_by(*lock))); > int f() __attribute__((exclusive_locks_required(*this->lock))) { > return n; // was accepted, now rejected > } > int g(S *p) __attribute__((exclusive_locks_required(*p->lock))) { > return n; // rejected, should probably be accepted > } > }; > > These may seem like strange ways to write such code, but the 'this->' form, at > least, might plausibly be used in templates. > > - Richard > -- DeLesley Hutchins | Software Engineer | [email protected] | 505-206-0315 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
