[CIR][CIRGen] Implement delegating constructors #821
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir -fexceptions -fcxx-exceptions %s -o %t.cir | ||
| // RUN: FileCheck --input-file=%t.cir %s | ||
|
|
||
| struct Delegating { | ||
| Delegating(); | ||
| Delegating(int); | ||
| }; | ||
|
|
||
| // Check that the constructor being delegated to is called with the correct | ||
| // arguments. | ||
| Delegating::Delegating() : Delegating(0) {} | ||
|
|
||
| // CHECK-LABEL: cir.func @_ZN10DelegatingC2Ev(%arg0: !cir.ptr<!ty_22Delegating22> {{.*}}) {{.*}} { | ||
| // CHECK-NEXT: %0 = cir.alloca !cir.ptr<!ty_22Delegating22>, !cir.ptr<!cir.ptr<!ty_22Delegating22>>, ["this", init] {alignment = 8 : i64} | ||
| // CHECK-NEXT: cir.store %arg0, %0 : !cir.ptr<!ty_22Delegating22>, !cir.ptr<!cir.ptr<!ty_22Delegating22>> | ||
| // CHECK-NEXT: %1 = cir.load %0 : !cir.ptr<!cir.ptr<!ty_22Delegating22>>, !cir.ptr<!ty_22Delegating22> | ||
| // CHECK-NEXT: %2 = cir.const #cir.int<0> : !s32i | ||
| // CHECK-NEXT: cir.call @_ZN10DelegatingC2Ei(%1, %2) : (!cir.ptr<!ty_22Delegating22>, !s32i) -> () | ||
| // CHECK-NEXT: cir.return | ||
| // CHECK-NEXT: } | ||
|
|
||
| struct DelegatingWithZeroing { | ||
| int i; | ||
| DelegatingWithZeroing() = default; | ||
| DelegatingWithZeroing(int); | ||
| }; | ||
|
|
||
| // Check that the delegating constructor performs zero-initialization here. | ||
| // FIXME: we should either emit the trivial default constructor or remove the | ||
| // call to it in a lowering pass. | ||
| DelegatingWithZeroing::DelegatingWithZeroing(int) : DelegatingWithZeroing() {} | ||
|
|
||
| // CHECK-LABEL: cir.func @_ZN21DelegatingWithZeroingC2Ei(%arg0: !cir.ptr<!ty_22DelegatingWithZeroing22> {{.*}}, %arg1: !s32i {{.*}}) {{.*}} { | ||
| // CHECK-NEXT: %0 = cir.alloca !cir.ptr<!ty_22DelegatingWithZeroing22>, !cir.ptr<!cir.ptr<!ty_22DelegatingWithZeroing22>>, ["this", init] {alignment = 8 : i64} | ||
| // CHECK-NEXT: %1 = cir.alloca !s32i, !cir.ptr<!s32i>, ["", init] {alignment = 4 : i64} | ||
| // CHECK-NEXT: cir.store %arg0, %0 : !cir.ptr<!ty_22DelegatingWithZeroing22>, !cir.ptr<!cir.ptr<!ty_22DelegatingWithZeroing22>> | ||
| // CHECK-NEXT: cir.store %arg1, %1 : !s32i, !cir.ptr<!s32i> | ||
| // CHECK-NEXT: %2 = cir.load %0 : !cir.ptr<!cir.ptr<!ty_22DelegatingWithZeroing22>>, !cir.ptr<!ty_22DelegatingWithZeroing22> | ||
| // CHECK-NEXT: %3 = cir.const #cir.zero : !ty_22DelegatingWithZeroing22 | ||
| // CHECK-NEXT: cir.store %3, %2 : !ty_22DelegatingWithZeroing22, !cir.ptr<!ty_22DelegatingWithZeroing22> | ||
| // CHECK-NEXT: cir.call @_ZN21DelegatingWithZeroingC2Ev(%2) : (!cir.ptr<!ty_22DelegatingWithZeroing22>) -> () extra(#fn_attr1) | ||
| // CHECK-NEXT: cir.return | ||
| // CHECK-NEXT: } | ||
|
|
||
| void canThrow(); | ||
| struct HasNonTrivialDestructor { | ||
| HasNonTrivialDestructor(); | ||
| HasNonTrivialDestructor(int); | ||
| ~HasNonTrivialDestructor(); | ||
| }; | ||
|
|
||
| // Check that we call the destructor whenever a cleanup is needed. | ||
| // FIXME: enable and check this when exceptions are fully supported. | ||
| #if 0 | ||
|
There was a problem hiding this comment. I wasn't sure of the best way to handle a case like this, where it'll currently NYI but we want to run the test once other support is in place. I could just not include these tests at all, but then we have to remember to add them later. By including them with an The alternative would be creating a separate test that's currently marked XFAIL, which should start passing once the NYIs are implemented, at which point an XFAIL test passing would error out and give us a loud reminder to come back to this. The problem is that we then put the burden of fixing the test up on the person implementing the NYI, which isn't ideal either (e.g. you shouldn't have to also flesh out delegating constructor support at the same time as implementing virtual bases). Suggestions are welcome here. There was a problem hiding this comment. I wasn't sure of the best way to handle a case like this, where it'll currently NYI but we want to run the test once other support is in place. I could just not include these tests at all, but then we have to remember to add them later. By including them with an #if 0 and FIXME comments, there's at least some indication of missing coverage, though we still have to remember to enable them later.
I don't think there's a silver bullet and there might a best judgment call depending on the case, some testcases also have commented code (e.g. the massive aarch64 builtins), which are very much like |
||
| HasNonTrivialDestructor::HasNonTrivialDestructor(int) | ||
| : HasNonTrivialDestructor() { | ||
| canThrow(); | ||
| } | ||
| #endif | ||
|
|
||
| // From clang/test/CodeGenCXX/cxx0x-delegating-ctors.cpp, check that virtual | ||
| // inheritance and delegating constructors interact correctly. | ||
| // FIXME: enable and check this when virtual inheritance is fully supported. | ||
| #if 0 | ||
| namespace PR14588 { | ||
| void other(); | ||
|
|
||
| class Base { | ||
| public: | ||
| Base() { squawk(); } | ||
| virtual ~Base() {} | ||
|
|
||
| virtual void squawk() { other(); } | ||
| }; | ||
|
|
||
| class Foo : public virtual Base { | ||
| public: | ||
| Foo(); | ||
| Foo(const void *inVoid); | ||
| virtual ~Foo() {} | ||
|
|
||
| virtual void squawk() { other(); } | ||
| }; | ||
|
|
||
| Foo::Foo() : Foo(nullptr) { other(); } | ||
| Foo::Foo(const void *inVoid) { squawk(); } | ||
| } // namespace PR14588 | ||
| #endif | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To elaborate on this: CodeGen omits calls to default trivial constructors:
clangir/clang/lib/CodeGen/CGExprCXX.cpp
Lines 618 to 620 in 8a8ba9e
CIRGen purposely skips that logic:
clangir/clang/lib/CIR/CodeGen/CIRGenExprCXX.cpp
Lines 370 to 375 in 8a8ba9e
There doesn't actually seem to be a pass that lowers the ctor call into trivial initialization though (or at least I couldn't get such a pass to run), so the emitted LLVM IR still contains the constructor call, but the constructor is never defined, so we'll get a link error.
I tried triggering the same behavior in another way without any luck. E.g. for https://godbolt.org/z/n43n6h185, we're not emitting the default constructor call; but that's because we end up in here instead:
clangir/clang/lib/CIR/CodeGen/CIRGenDecl.cpp
Line 308 in 8a8ba9e
The comments in that function about retaining ctor calls also seem potentially inaccurate though, or I'm misunderstanding their intent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's right, we need to add support in LoweringPrepare for those. Similar / related to things tracked with
should*in missing features.The initial plan here was to add a
cir.ctor_calloperation, which could have attributes to designate triviality, and keep some other properties around (via AST or directly), such operation would be folded away in LoweringPrepare. It could also be emitted directly out of CIRGen or added via the idiom recognizer (I prefer the former).None of this has been implemented yet. An alternative solution is to omit the ctor call directly (like OG codegen does) and add an assert on missing feature. Such that we can track and come back to this whenever we care about an analysis that uses this information - but in the meantime we get LLVM codegen parity.
Right now you'd have to look at a regular cir.call and try to see if it was a ctor call + determine if it's trivial, which isn't ideal since we have CIR for the very reason (raising ).
The part on retaining ctor calls is legit, for purposes of static analsys we might want the allocas to be tagged properly based on ctors touching them, like the lifetime checker does. We need to keep the source semantics close first out of CIRGen.