[PATCH] D32449: Modifying PthreadLockChecker.cpp to reduce false positives.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Thanks, this is great! Two more things:

- You have touched other code, unrelated to your patch, with clang-format; we're usually trying to avoid that, because it creates merge conflicts out of nowhere, and because some of that code actually seems formatted by hand intentionally. It's better to revert these changes; you can use the `git clang-format` thing to format only actual changes.

- Updating .gitignore sounds like the right thing to do (llvm's .gitignore already has this), but i guess we'd better make a separate commit for that.



================
Comment at: lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp:143-146
+    if(lstate->isSchrodingerUntouched())
+      state = state->remove<LockMap>(lockR);
+    else if(lstate->isSchrodingerUnlocked())
+      state = state->set<LockMap>(lockR, LockState::getUnlocked());
----------------
malhar1995 wrote:
> NoQ wrote:
> > I think we can be certain that the lock is in one of these states, and assert that.
> We can be certain that the lock state will be either of the two only if I add the following statement before returning from this function.
> ```
> state = state->remove<DestroyRetVal>(lockR);
> ```
> If I don't add the above statement, a return value symbol for the region specified by lockR will still be in DestroyRetVal and it may have an actual lock state (locked, unlocked or destroyed).
Yep, that's a great thing to do. I didn't notice this.

Generally, it's great to keep the program state free from stuff that would no longer be necessary.


Repository:
  rL LLVM

https://reviews.llvm.org/D32449