[PATCH] D47044: [analyzer] Ensure that we only visit a destructor for a reference if type information is available.

Henry Wong via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 8 07:31:24 PDT 2018


MTC added a comment.

I didn't test the code, but the code seems correct. Thanks!



================
Comment at: lib/StaticAnalyzer/Core/LoopWidening.cpp:88
+  MatchFinder Finder;
+  Finder.addMatcher(varDecl(hasType(referenceType())).bind("match"), new Callback(LCtx, MRMgr, ITraits));
+  Finder.matchAST(ASTCtx);
----------------
The code should fit within 80 columns of text.


================
Comment at: test/Analysis/loop-widening-invalid-type.cpp:1
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-max-loop 4 -analyzer-config widen-loops=true -verify %s
+
----------------
I think it's better to add more expressive tests. Like:

```
struct A {
  int x;
  A(int x) : x(x) {}
};

void invalid_type_region_access() {
  const A &a = A(10);
  for(int i = 0; i < 10; ++i) {}
  clang_analyzer_eval(a.x ==10); // expected-warning{{TRUE}}
}
```

I think should use more related names instead of `loop-widening-invalid-type.cpp`, like `loop-widening-reference-type`.


================
Comment at: test/Analysis/loop-widening-invalid-type.cpp:8
+
+void invalid_type_region_access() { // expected-no-diagnostics
+  const A &x = B();
----------------
I don't know what the purpose of the test is, is the comment `no-crash` better?


Repository:
  rC Clang

https://reviews.llvm.org/D47044





More information about the cfe-commits mailing list