r205728 - Thread Safety Analysis: update to internal SExpr handling.

[Delesley Hutchins]

Patch looks good overall, except for the constness of
handleDestructorCall, and SExprRef below.  You are free to submit
(with those fixes); I won't update ThreadSafetyTIL until the patch
goes in.

> That means this entire class is mostly for documentation. In the
> attached patch, I simply commented it out and explained it a bit
> further.

The advantage of keeping it a class is that you can inherit from it
and only "override" one or two methods, rather than having to
implement the whole interface.  The disadvantage is that if you get
the type signature wrong, the overriding doesn't happen and there's no
warning.   I could go either way on this.  You can keep it commented
if you prefer.

> My attach patch managed it (but perhaps that's just because I was
> testing with MSVC).

This works because I goofed.  :-)  handleDestructorCall will
eventually need to take non-const arguments, because I'd like this
traversal to eventually replace the main runAnalysis() routine in
ThreadSafety.cpp.  (The casts are copied from there, where they are
needed.)

> That's kind of what I figured, but what's strange to me is that SExpr
> (and subclasses) seem to always be created in the Arena -- so the
> destructors aren't being called anyway (I didn't see any explicit
> destructor calls happening). That's why my proposed patch tries to
> make this more clear -- you must create them in an arena, and you
> cannot delete them.

This is an excellent fix.  Thanks!

> Shouldn't that be defined as a swap (since right now, it's just
> copying the pointer)?
>
> SExprRef(SExprRef &&R) { std::swap(Ptr, R.Ptr); }

No.  My bad.  This should be changed to:

  SExprRef(SExprRef &&R) : Ptr(R.Ptr) { R.Ptr = nullptr; }

Also, please put back the nullptr constructor.  Null SExprRefs are
allowed, and the null constructor cuts out some overhead.
Perhaps the copy constructor should be explicitly deleted?

Thanks again for the review; lots of good comments.

  -DeLesley

-- 
DeLesley Hutchins | Software Engineer | delesley at google.com | 505-206-0315