[PATCH] D59455: Thread safety analysis: Add note for unlock kind mismatch

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Mar 18 14:24:57 PDT 2019


aaron.ballman added inline comments.


================
Comment at: include/clang/Analysis/Analyses/ThreadSafety.h:127
+                                         SourceLocation LocLocked,
                                          SourceLocation Loc) {}
 
----------------
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.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59455





More information about the cfe-commits mailing list