NoQ added a comment.

Thx a lot for the fix!



================
Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:482-483
+// them with parameters.  This function incapsulates such cases.
+static SVal process(SVal Value, const ParmVarDecl *Parameter,
+                    SValBuilder &SVB) {
+  QualType ParamType = Parameter->getType();
----------------
Maybe at least `processParameter`?


================
Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:496-497
+  if (isTransparentUnion(ParamType) &&
+      // Let's check that we indeed trying to bind different types.
+      !isTransparentUnion(Value, SVB.getContext())) {
+    BasicValueFactory &BVF = SVB.getBasicValueFactory();
----------------
I think this check could be implemented more reliably on the AST. I.e., ask 
what's the type of the argument expression instead.

It's typically better to query the AST types instead of `SVal` types because 
AST types are richer (cf. lvalue/rvalue confusion from D104550#inline-993348 - 
which is a very common source of bugs) and also significantly less likely to be 
buggy / improperly modeled. So i'm very much in favor of knowing in advance 
which type do we expect according to the AST and then later asserting that we 
got something compatible from the `SVal`.


================
Comment at: clang/test/Analysis/transparent_union_bug.c:5
+void clang_analyzer_warnIfReached();
+void clang_analyzer_printState();
+
----------------
Unused :P


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104716

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

Reply via email to