r209774 - Thread Safety Analysis: update TIL traversal mechanism to allow arbitrary
Delesley Hutchins
delesley at google.com
Thu May 29 14:33:09 PDT 2014
Thanks for the review! Fixed in r209847.
> Why the access change? As best I can tell, it's not required.
It doesn't need to be. Done.
> Instead of all of these static_casts, would it make more sense to have
> a templated function on Literal? Eg)
done.
> auto I = std::find(Predecessors.begin(), Predecessors.end(), BB);
> return std::distance(Predecessors.begin(), I);
>
> Also, BB and the function itself can be const.
done.
> These should all be const functions.
Not really. The trivial definitions here are const, but they may be
non-const in other implementations.
> Why is Elems now public?
The inheritance hierarchy has changed, so the friend statement no longer works.
This is a trivial wrapper class; privacy is not really important here.
> Why is this member public?
Same reason as Elems earlier.
> This doesn't seem quite right. If Capacity is 0, what guarantees are
> there that InitialCapacity > N? Also, what guarantees that Capacity *
> 2 is large enough?
An excellent catch! Thanks! Done. Test coverage was poor: I always
call it with N==1. :-)
> This is potentially UB; BB can be nullptr from the preceding statement.
Another excellent catch. Thanks! Done.
> Same sort of UB here as above.
Why? BB1 and BB2 are checked for nullptr.
-DeLesley
--
DeLesley Hutchins | Software Engineer | delesley at google.com | 505-206-0315
More information about the cfe-commits
mailing list