Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Flexible array type with explicitly defined destructor. #36788

Open Quuxplusone opened 6 years ago

Quuxplusone commented 6 years ago
Bugzilla Link PR37815
Status NEW
Importance P enhancement
Reported by zahira.ammarguellat@intel.com
Reported on 2018-06-15 08:13:42 -0700
Last modified on 2019-10-09 23:32:54 -0700
Version unspecified
Hardware PC Windows NT
CC dgregor@apple.com, llvm-bugs@lists.llvm.org, o_kniemeyer@maxon.de, richard-llvm@metafoo.co.uk, zahira.ammarguellat@intel.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also
test.cpp:

struct A {
  ~A() {  }
};
struct C {
 int c1;
 A a[];
};

bash-4.2$ clang -c test.cpp
test.cpp:6:5: error: flexible array member 'a' of type 'A []' with non-trivial
      destruction
  A a[];
    ^
1 error generated.
bash-4.2$

GCC allows this.
Quuxplusone commented 6 years ago
(In reply to zahira.ammarguellat from comment #0)
> GCC allows this.

That doesn't make it a good idea.

That said, I think it'd be reasonable to allow this if C has a user-provided
destructor. While we can't possibly know how to synthesize a correct destructor
for C, if the user writes their own destructor we can leave it to them to
destroy the array elements. (C++ has a similar rule for, eg, unions with non-
trivially-destructible elements.)
Quuxplusone commented 5 years ago
I'd vote for allowing flexible array members with non-trivial destructors. GCC,
ICC and MSVC support this non-standard extension, and in LLVM's code there's
already a FIXME (clang/lib/Sema/SemaDecl.cpp):

        // FIXME: GCC allows this. We should probably either implicitly delete
        // the destructor of the containing class, or just allow this.
        QualType BaseElem = Context.getBaseElementType(FD->getType());
        if (!BaseElem->isDependentType() && BaseElem.isDestructedType()) {
          Diag(FD->getLocation(), diag::err_flexible_array_has_nontrivial_dtor)
            << FD->getDeclName() << FD->getType();
          FD->setInvalidDecl();
          EnclosingDecl->setInvalidDecl();
          continue;
        }

IMHO deleting the implicit destructor is the best option.