[PATCH] D64656: Ensure placeholder instruction for cleanup is created

Øystein Dale via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jul 12 14:46:18 PDT 2019


oydale marked 2 inline comments as done.
oydale added inline comments.


================
Comment at: clang/test/CodeGenCXX/pr40771-ctad-with-lambda-copy-capture.cpp:1
+// RUN: %clang_cc1 -emit-obj --std=c++17 -fcxx-exceptions -fexceptions -O1 %s
+// CHECK no crash
----------------
lebedev.ri wrote:
> lebedev.ri wrote:
> > Why do you need `-O1`?
> > 
> Also, you only check $? Don't you want to check the actual IR?
> 
> 

I wanted the test case to cover both a debug build and a release build of clang, hence the use of -O1.

For a release build without asserts, the issue is only visible when building with -O1 or higher. For a debug build the optimization level doesn't matter because it hits an assert (the one mentioned in the commit message) earlier in the compile process.

Using clang++ as provided by my distro and on godbolt the following works fine:
```
clang++ --std=c++17 -c crash.cpp
```

The following crashes:
```
clang++ --std=c++17 -c -O1 crash.cpp
```

I do not have sufficient knowledge of what the IR is expected to look like in this case to write a test for it. Further, all I have done is to ensure that a placeholder instruction is created when it's needed. This placeholder instruction is erased at the end of the function where it was created, so the generated IR should already be covered by other tests.


================
Comment at: clang/test/CodeGenCXX/pr40771-ctad-with-lambda-copy-capture.cpp:2
+// RUN: %clang_cc1 -emit-obj --std=c++17 -fcxx-exceptions -fexceptions -O1 %s
+// CHECK no crash
+
----------------
lebedev.ri wrote:
> Either add missing `:` or please write better comment :)
I'm not sure what else that comment would contain. I want the test to verify that clang does not crash given the provided input. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64656





More information about the cfe-commits mailing list