This is an archive of the discontinued LLVM Phabricator instance.

[AST] Dont invalidate VarDecl even the default initializaiton is failed.
ClosedPublic

Authored by hokein on Apr 3 2020, 7:18 AM.

Details

Summary

This patch would cause clang emit more diagnostics, but it is much better than https://reviews.llvm.org/D76831

cpp
struct A {
  A(int);
  ~A() = delete;
};
void k() {
  A a;
}

before the patch:

/tmp/t3.cpp:24:5: error: no matching constructor for initialization of 'A'
  A a;
    ^
/tmp/t3.cpp:20:3: note: candidate constructor not viable: requires 1 argument, but 0 were provided
  A(int);
  ^
/tmp/t3.cpp:19:8: note: candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 0 were provided
struct A {

After the patch:

/tmp/t3.cpp:24:5: error: no matching constructor for initialization of 'A'
  A a;
    ^
/tmp/t3.cpp:20:3: note: candidate constructor not viable: requires 1 argument, but 0 were provided
  A(int);
  ^
/tmp/t3.cpp:19:8: note: candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 0 were provided
struct A {
       ^
/tmp/t3.cpp:24:5: error: attempt to use a deleted function
  A a;
    ^
/tmp/t3.cpp:21:3: note: '~A' has been explicitly marked deleted here
  ~A() = delete;

Diff Detail

Event Timeline

hokein created this revision.Apr 3 2020, 7:18 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 3 2020, 7:18 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
hokein edited the summary of this revision. (Show Details)Apr 3 2020, 7:19 AM
hokein edited the summary of this revision. (Show Details)Apr 3 2020, 7:23 AM
hokein added a reviewer: sammccall.
hokein retitled this revision from [AST] Dont invalide VarDecl even the default initializaiton is failed. to [AST] Dont invalidate VarDecl even the default initializaiton is failed..
sammccall accepted this revision.Apr 9 2020, 12:52 PM

the attempt to use a deleted function diagnostic is a bit spammy, because the default-constructor and destructor are often deleted for similar reasons. But I guess this is probably OK, unless you can see an easy way to suppress it.

clang/lib/Sema/SemaDecl.cpp
12559

Tempting as it is, I'm not sure we should leave a big comment around describing code that isn't here.
Maybe just a minimal If default-init fails, leave var uninitialized but valid, for recovery.

clang/test/AST/ast-dump-invalid-initialized.cpp
7

may be worth testing the other existing behavior we're being consistent with here too: A a1 = garbage(); etc

clang/test/CodeCompletion/invalid-initialized-class.cpp
8

Again, may want Foo bar = garbage(); bar.^

clang/test/SemaCXX/cxx0x-deleted-default-ctor.cpp
11

nit: extra //

This revision is now accepted and ready to land.Apr 9 2020, 12:52 PM
hokein updated this revision to Diff 257231.Apr 14 2020, 1:50 AM
hokein marked 5 inline comments as done.

rebase and address review comments

the attempt to use a deleted function diagnostic is a bit spammy, because the default-constructor and destructor are often deleted for similar reasons. But I guess this is probably OK, unless you can see an easy way to suppress it.

didn't see a trivial way to suppress this diagnostic, as we don't have enough information to distinguish the VarDecl (valid bit is true, no init-expr) has an ill-formed default initialization. I think we could build a recovery-expr and use it as the init-expr for the VarDecl, and suppress this diagnostic.

This revision was automatically updated to reflect the committed changes.