[PATCH] D157385: [clang][CFG] Cleanup functions

[Timm Bäder via Phabricator via cfe-commits]

tbaeder marked an inline comment as done.
tbaeder added inline comments.


================
Comment at: clang/test/Analysis/scopes-cfg-output.cpp:1473-1474
+// CHECK-NEXT:    3: F f __attribute__((cleanup(cleanup_F)));
+// CHECK-NEXT:    4: [B1.3].~F() (Implicit destructor)
+// CHECK-NEXT:    5: CleanupFunction (cleanup_F)
+// CHECK-NEXT:    6: CFGScopeEnd(f)
----------------
aaronpuchert wrote:
> Interesting test! But it seems CodeGen has them swapped: compiling this snippet with `clang -c -S -emit-llvm` I get
> ```lang=LLVM
> define dso_local void @_Z4testv() #0 personality ptr @__gxx_personality_v0 {
>   %1 = alloca %class.F, align 1
>   %2 = alloca ptr, align 8
>   %3 = alloca i32, align 4
>   invoke void @_Z9cleanup_FP1F(ptr noundef %1)
>           to label %4 unwind label %5
> 
> 4:                                                ; preds = %0
>   call void @_ZN1FD2Ev(ptr noundef nonnull align 1 dereferenceable(1) %1) #3
>   ret void
> 
> ; ...
> }
> ```
> So first cleanup, then destructor. This is with 17.0.0-rc2.
Interesting, I thought I checked this and used the correct order. Will re-check, thanks.


================
Comment at: clang/test/Analysis/scopes-cfg-output.cpp:1480
+public:
+  ~F() {}
+};
----------------
aaronpuchert wrote:
> As with the cleanup function, a definition shouldn't be necessary.
Is there a way to test whether the contents of the cleanup function are being checked as well? From these tests, I only know we consider them called, but not whether we (properly) analyze their bodies in the context as well. Or is that separate from this patch?


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

https://reviews.llvm.org/D157385