rjmccall added a reviewer: doug.gregor. rjmccall added inline comments.
================ Comment at: lib/Sema/SemaInit.cpp:7074 + if (RD->hasDefinition() && RD->hasNonTrivialDestructor()) + S.MarkFunctionReferenced(E->getExprLoc(), S.LookupDestructor(RD)); + ---------------- If we're actually using the destructor, we can't just look it up and mark it referenced; we have to call CheckDestructorAccess and DiagnoseUseOfDecl. Walking the syntactic initializers like this seems wrong: - I don't know if we actually promise to rewrite these expressions to the actual recursive element initialization expression if the expression requires conversion. - This is going to miss implicitly-initialized elements, which can happen for a variety of reasons including (1) running out of explicit initializers and (2) designated initializers skipping a field. - We'll end up with redundant diagnostics from CheckDestructorAccess/DiagnoseUseOfDecl if the expression is an already-materialized temporary. - We'll end up with unwanted diagnostics from CheckDestructorAccess/DiagnoseUseOfDecl if the expression is a gl-value and actually part of a reference-initialization, where we don't actually use the destructor. I think the right solution is probably that list-initialization needs to recognize that the initialization of a record needs to be treated as a potential use of the destructor even if we aren't supposed to bind it as a temporary, because once an object is initialized in the local context, there are all sorts of reasons why we might need to call its destructor before the initialization completes/transitions. Basically every call to shouldBindAsTemporary in this file is suspect, because they're all places where we might need to check the destructor use even if we didn't make a temporary. We seem to get this right in SK_UserConversion where we call shouldDestroyEntity, and the comment there is spot-on — it doesn't make sense for this code to be specific to SK_UserConversion at all. My tentative suggestion would be to (1) make a helper function that does that exact "if should-bind then bind else if should-destroy then destroy" dance and then (2) look at all the should-bind checks and try to use your helper function instead. But I'd really like Richard and/or Doug to weigh in. ================ Comment at: test/CodeGenObjCXX/arc-list-init-destruct.mm:1 +// RUN: %clang_cc1 -triple x86_64-apple-macosx10.13.0 -std=c++1z -fobjc-arc -fobjc-exceptions -fcxx-exceptions -fexceptions -emit-llvm -o - %s | FileCheck %s + ---------------- Does the corresponding C++ test case (replacing `Class0 *f;` with `HasExplicitNonTrivialDestructor f;`) not reproduce the problem? Repository: rC Clang https://reviews.llvm.org/D45898 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
