aaronpuchert marked 4 inline comments as done.
aaronpuchert added inline comments.


================
Comment at: include/clang/Analysis/Analyses/ThreadSafety.h:127
+                                         SourceLocation LocLocked,
                                          SourceLocation Loc) {}
 
----------------
aaron.ballman wrote:
> aaronpuchert wrote:
> > aaron.ballman wrote:
> > > Can you rename this one to `LocUnlocked` to mirror the new parameter?
> > This was supposed to mirror the following function (handleDoubleLock), 
> > where similarly `Loc` is the place of the issue itself and `LocLocked` 
> > indicates where the lock is coming from. If we want to change this, I would 
> > suggest to use present tense here: `LocUnlock`. The lock from `LocLocked` 
> > "happened in the past" (meaning earlier in the code), but the unlock 
> > "happens now" (meaning at the current location).
> Ah, good call on the tense of the identifier! Given that it's following a 
> pattern, I'd say you can commit as-is. If you're similarly bothered by the 
> name, then you could have a follow-up NFC commit that renames consistently 
> across APIs.
Changed in rC356430.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59455/new/

https://reviews.llvm.org/D59455



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to