[PATCH] D129752: Thread safety analysis: Handle additional cast in scoped capability construction

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert marked an inline comment as done.
aaronpuchert added a comment.

In D129752#3682977 <https://reviews.llvm.org/D129752#3682977>, @aaron.ballman wrote:

> Do you think this warrants a release note (or did it close any open issues in the tracker)?

It's probably too technical/minor to be worth mentioning. It also doesn't close any issues that I'm aware of, I mostly did it in preparation for D129755 <https://reviews.llvm.org/D129755>, where I'm reusing the new function.



================
Comment at: clang/lib/Analysis/ThreadSafety.cpp:2091-2097
+  if (auto *CE = dyn_cast<CastExpr>(E))
+    if (CE->getCastKind() == CK_NoOp)
+      E = CE->getSubExpr()->IgnoreParens();
+  if (auto *CE = dyn_cast<CastExpr>(E))
+    if (CE->getCastKind() == CK_ConstructorConversion ||
+        CE->getCastKind() == CK_UserDefinedConversion)
+      E = CE->getSubExpr();
----------------
aaron.ballman wrote:
> I almost wonder if we should just turn this into a while loop rather than two casts in a specific order. However, I can't think of a situation where we'd need the loop, so I think this is fine. At the very least, it's incremental progress!
They should always be in that order, or at least until we get to the `CXXConstructExpr` or `CXXMemberCallExpr` that we want (there could be another standard conversion sequence for the argument inside of that, but that's not our concern here). The reason is that this constructor or conversion function call is the `CK_ConstructorConversion` or `CK_UserDefinedConversion`, respectively, while the remaining standard conversion sequence (to which the `NoOp` belongs) [happens after/above that](https://eel.is/c++draft/over.best.ics#over.ics.user).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129752