[PATCH] D99165: [clang] Fix a crash on checkDestructorReference.

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 24 03:38:42 PDT 2021


hokein added a comment.

In D99165#2644243 <https://reviews.llvm.org/D99165#2644243>, @sammccall wrote:

> The fix doesn't look obviously correct: the side effect of marking the destructor reference seems important if we actually generate code. It's not obvious to me why the type can only be incomplete if there are errors.
>
> This was introduced between clang 8 and clang 9, I would guess by f8ccf052935adaf405e581fd31e8bc634cc5bbc7.
> @erik.pilkington 
> Looking at that patch, mostly this function was just a rename, but there's a new callsite for array-types that seems to be what we're hitting here.
> Maybe a slightly less-invasive version would be to guard that callsite with "and if the element type is complete"?

ok, after taking a closer look on the crash, you're right.  fixing checkDestructorReference or guarding at that particular callsite is not a root fix.

I managed to reduce a smaller case `ForwardClass array[var_size] = {0};`, and the crash occurs when clang is performing an initializer check on an incomplete-type var decl, this should not be happened --
because an incomplete type of decl should be marked invalid in clang (e.g. `ForwardClass array[var_size];`, `ForwardClass var;`), but for the crash case, the type of variable-length array is not treated as an incomplete type (this is the bug), 
thus the var decl is considered valid, and clang performs the initializer check on it.

So I think the right solution is to fix type of variable-length array -- if the element type of the variable-length array is incomplete, then the type of array is incomplete.



================
Comment at: clang/lib/AST/Type.cpp:2234
     // (C++ [dcl.array]p1).
-    // We don't handle variable arrays (they're not allowed in C++) or
-    // dependent-sized arrays (dependent types are never treated as incomplete).
----------------
This comment is pretty old, and was added in 2dfdb820ca550f75769f6850bc27f825f1dce4f7 in 2009. I don't think this is correct anymore, variable-length array is allowed in C++, and from http://eel.is/c++draft/basic.types.general#5, 

> an array of unknown bound or of incomplete element type, is an incompletely-defined object type.36 Incompletely-defined object types and cv void are incomplete types



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99165



More information about the cfe-commits mailing list