[PATCH] D158561: [-Wunsafe-buffer-usage] Add AST info to the unclaimed DRE debug notes for analysis

Artem Dergachev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Sep 20 14:58:05 PDT 2023


NoQ added a comment.

I like where this is going, and I appreciate the test! Tests for tiny debug utilities aren't critical for the compiler, but they're a great way to demonstrate and document how to invoke the script and how it's supposed to work.



================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:55
+static std::optional<std::string> printStmtClass(const Stmt *Stmt) {
+  switch (Stmt->getStmtClass()) {
+#define STMT(CLASS, PARENT)                                                    \
----------------
This looks like a manual reimplementation of a `StmtVisitor`. Maybe just use it directly?
```lang=c++
class StmtDebugPrinter : public ConstStmtVisitor<StmtDebugPrinter, std::optional<std::string>> {
public:
  std::optional<std::string> VisitStmt(const Stmt *S) {
    return S->getStmtClassName();
  }
  std::optional<std::string> VisitBinaryOperator(const BinaryOperator *BO) {
    // Customize...
  }
};
```


================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:37-38
+
+  if (BO->getOpcode() == BinaryOperator::Opcode::BO_Comma)
+    BOOpStr = "COMMA"; // Do not print `,` as it is used as the separator
+  return "BinaryOperator(" + BOOpStr.str() + ")";
----------------
Maybe just use something else as a separator, like ` ==> `? (Python `.split()` method will eat that happily.)


================
Comment at: clang/test/SemaCXX/warn-unsafe-buffer-usage-debug-unclaimed.cpp:10
+// RUN:                 | sed 's/^The unclaimed DRE trace://' \
+// RUN:                 | python3 %S/../../utils/analyze_safe_buffer_debug_notes.py \
+// RUN:                 | FileCheck %s
----------------
You probably need to do something similar to `test/Analysis/exploded-graph-rewriter/lit.local.cfg`, to properly discover the right python executable which may not be straightforward on some buildbots (imagine Windows!).


================
Comment at: clang/utils/analyze_safe_buffer_debug_notes.py:29-33
+    while True:
+        try:
+            line = input()
+        except EOFError:
+            break
----------------
A bit more idiomatic I think (might behave differently around linebreaks).


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

https://reviews.llvm.org/D158561



More information about the cfe-commits mailing list