[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