This is an archive of the discontinued LLVM Phabricator instance.

[C++17] Fix class template argument deduction for default constructors without an initializer
ClosedPublic

Authored by Rakete1111 on Sep 23 2017, 12:22 PM.

Details

Diff Detail

Event Timeline

Rakete1111 created this revision.Sep 23 2017, 12:22 PM

Rebased and friendly ping :)

Slight change + rebased + friendly ping :)

The standard hasn't allowed deducing any placeholder type without an initializer (10.1.7.4.1 [dcl.type.auto.deduct]/2) yet.

It's unclear to me what

extern A x;

wants to archive. Usually when people writing extern then expect an initializer to appear somewhere else, but with this declaration, defining

A x = ...;

later may fail by resolving to a different type, which feels... interesting.

@lichray Isn't [dcl.type.auto.deduct] only for auto and decltype(auto)? Class template argument deduction is in [dcl.type.class.deduct], which doesn't seem to disallow declarations without an initializer.

About that extern business, yes that's indeed counter intuitive and weird. Couldn't find anything in the standard prohibiting this though, but I'm not so good at that either.

@lichray Isn't [dcl.type.auto.deduct] only for auto and decltype(auto)?

Sorry, reasoned on a confusingly similar part... Here is
updated information:

10.1.7.5 [dcl.type.class.deduct]
If a placeholder for a deduced class type appears as a decl-specifier in
the decl-specifier-seq of an initializing declaration (11.6) of a variable,
[...].

11.6 [dcl.init]/22
A declaration that specifies the initialization of a variable, whether from
an explicit initializer or by default initialization, is called the initializing
declaration of that variable. [ Note: In most cases this is the defining
declaration (6.1) of the variable, but the initializing declaration of a non-
inline static data member (12.2.3.2) might be the declaration within the
class definition and not the definition at namespace scope. —end note ]

So deducing from default initialization is indeed allowed, but extern
could be interpreted as outlawed because similar to the case of non-
inline static data member, where the declaration doesn't give the
initialization of that variable. @rsmith, comments?

rsmith edited edge metadata.Nov 13 2017, 11:57 AM

So deducing from default initialization is indeed allowed, but extern
could be interpreted as outlawed because similar to the case of non-
inline static data member, where the declaration doesn't give the
initialization of that variable. @rsmith, comments?

That's almost right, but not all extern declarations are disallowed. (An extern declaration is still a defining declaration if it has an initializer.)

lib/Sema/SemaDecl.cpp
10208

Please put this ?: expression into the declaration of DeduceInits instead of here. (We should build an empty list instead of a list of one null pointer when there is no init.)

Made DeduceInits empty instead of a workaround somewhere else. Is it still ok, @rsmith ?

That's almost right, but not all extern declarations are disallowed. (An extern declaration is still a defining declaration if it has an initializer.)

Good to know, thanks.

So gcc got this wrong?

https://wandbox.org/permlink/RVApvaca1ebUfInn

Yes, the declaration extern A a; in that example is ill-formed.

Rakete1111 updated this revision to Diff 128662.Jan 4 2018, 3:37 PM
Rakete1111 marked an inline comment as done.

Rebased + friendly 2018 ping

rsmith requested changes to this revision.Feb 28 2018, 4:41 PM

Per [dcl.type.class.deduct]p1, only the initializing declaration of a variable can use a placeholder type. The existing diagnostic was correct in many of the modified cases.

test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.type.class.deduct/p1.cpp
15

This should be ill-formed: this is not the initializing declaration of B::a.

17–19

Likewise, this should remain ill-formed.

test/Parser/cxx1z-class-template-argument-deduction.cpp
78

Likewise.

This revision now requires changes to proceed.Feb 28 2018, 4:41 PM

Addressed review comments :)

Thanks!

Rakete1111 marked 3 inline comments as done.Mar 13 2018, 7:36 AM

LGTM

test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.type.class.deduct/p1.cpp
17–19

Please add one more test to the end of the file saying

static A y;

as a remainder to the readers.

@lichray Ok done :) Thanks for reviewing

Rakete1111 marked an inline comment as done.Mar 18 2018, 1:11 PM
lichray accepted this revision.Mar 18 2018, 10:21 PM
rsmith accepted this revision.Mar 21 2018, 5:01 PM
rsmith added inline comments.
test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.type.class.deduct/p1.cpp
17

The diagnostic we produce in this case is not very good. The problem is not that the declaration requires an initializer (how can a forward declaration like this require an initializer?), instead the problem is that type deduction is only possible for the initializing declaration of a variable.

This revision is now accepted and ready to land.Mar 21 2018, 5:01 PM
lichray edited the summary of this revision. (Show Details)Mar 23 2018, 7:44 PM
This revision was automatically updated to reflect the committed changes.