This is an archive of the discontinued LLVM Phabricator instance.

[C++17] Don't crash while diagnosing different access specifier of a deduction guide.
AbandonedPublic

Authored by Rakete1111 on Jan 31 2019, 1:20 PM.

Details

Summary

This fixes PR40552 by not showing the diagnostic that complains about different access specifiers, since the template does not have one.

Diff Detail

Event Timeline

Rakete1111 created this revision.Jan 31 2019, 1:20 PM
aaron.ballman added a subscriber: aaron.ballman.
aaron.ballman added inline comments.
test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
360–361

These errors are pretty unfortunate -- they don't really help the user to understand what's gone wrong here. They're an improvement over the crash, but I think we should try to make the errors more useful if we can.

Why is Typo() being treated as a deduction guide? Perhaps we could look to see if there is a -> before making that determination?

Herald added a project: Restricted Project. · View Herald TranscriptFeb 1 2019, 5:49 AM
Rakete1111 marked an inline comment as done.Feb 2 2019, 4:55 AM
Rakete1111 added inline comments.
test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
360–361

Perhaps we could look to see if there is a -> before making that determination?

Yes that would be possible. The diagnostic would change for the following code:

template <typename>
struct Foo {};

Foo();// -> Foo<int>; 
// currently: deduction guide missing ->
// after: C++ requires type specifier for every declaration

Is that acceptable? Or I guess I could restrict this to partial deduction guides in classes.

aaron.ballman added inline comments.Feb 4 2019, 5:56 AM
test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
360–361

I think the original diagnostic is better in that case. If you restrict to partial deduction guides, do we get all the good diagnostics?

rsmith added inline comments.Feb 8 2019, 4:15 PM
test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
360–361

I think the most likely intent in that case was to declare a constructor of Foo, and either it was accidentally written after the end of the class, or the in-class declaration got copy-pasted and the programmer forgot to fix it up. And moreso for a case such as

template <typename>
struct Foo { Foo(); };

template<typename T>
Foo() {
    // ...
}

... where we currently give three errors about deduction guides and no hint that a qualified name is required to define a constructor.

I think it's comparatively unlikely that someone would forget the -> in a deduction guide, given how central it is to the purpose of the declaration. So always disambiguating as a failed constructor rather than a failed deduction guide seems reasonable to me.

Rakete1111 abandoned this revision.Feb 18 2019, 6:04 PM

This revision has been superseded :)