Page MenuHomePhabricator

[C++20] Correctly handle constexpr/consteval explicitly defaulted special member instantiation behavior
AbandonedPublic

Authored by aaron.ballman on Jun 17 2022, 11:48 AM.

Details

Reviewers
rsmith
erichkeane
jyknight
Group Reviewers
Restricted Project
Summary

When instantiating a class template, if the primary class template special member function declaration was declared constexpr or consteval, the instantiation is also considered to be a consteval function even if the instantiation would not be valid to use in a constant expression (http://eel.is/c++draft/dcl.constexpr#7). However, when instantiating such a class, we were incorrectly saying the special member was not a constexpr function when a base class or member variable would cause the instantiation to not be usable in a constant expression. This addresses the issue by falling back to the primary declaration of the explicitly defaulted special member function.

This caused some unexpected test failures with friend declaration checking, but the standard is quite unclear on what's expected there. We also have existing bugs in that particular area (https://godbolt.org/z/cPKGvx746). I raised some questions on the Core reflectors about what is expected and put a FIXME into the test to make it clear that's unresolved.

Diff Detail

Event Timeline

aaron.ballman created this revision.Jun 17 2022, 11:48 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 17 2022, 11:48 AM
aaron.ballman requested review of this revision.Jun 17 2022, 11:48 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 17 2022, 11:48 AM
rsmith added inline comments.Jun 17 2022, 12:53 PM
clang/lib/AST/DeclCXX.cpp
1315–1316

Incidentally: this was fixed in DR1361, a long time ago. We can remove this comment.

1319

Shouldn't this be looking in the class template itself? It doesn't seem correct to use things like this->hasConstexprDefaultConstructor() from here, because we've not finished gathering the data that contributes to that function's return value yet.

That said... I don't think that changing DefaultedDefaultConstructorIsConstexpr can work here. Keep in mind that a class can have more than one default constructor, and in C++20 onwards can even have more than one defaulted default constructor. This flag needs to apply to all of them, and only some subset of them might have been declared as constexpr in the template from which they are instantiated. So this flag should be answering the question, "is a defaulted default constructor *implicitly* constexpr?", and any callers that are assuming that it means "will this particular defaulted default constructor be constexpr?" should be changed to take into account whether the default constructor is explicitly declared as constexpr.

clang/lib/Sema/SemaDeclCXX.cpp
7555–7556

It would be clearer and more in line with the wording to ask if MD is instantiated here, rather than to ask if the class is being instantiated (though the two are presumably equivalent because you can't instantiate a constructor template to form a special member function).

7576

I think we'll now treat instantiated constructors and non-instantiated ones differently here:

#ifdef TEMPLATE
template<typename T>
#endif
struct S {
  constexpr S() = default;
  TypeWithThrowingConstructor x;
};

In the templated case, the instantiation's constructor will be constexpr, but in the non-template case it won't be. That doesn't seem consistent. It would be cleaner and would probably require fewer special cases if we treated both cases the same: if you specify constexpr, you get a constexpr function, always, but we'll generate an error message if the function is non-templated and wouldn't have been implicitly constexpr.

clang/test/SemaCXX/cxx2a-consteval.cpp
774

This diagnostic demonstrates that we're not doing the proper checking here: we do attempt to call a constexpr constructor that doesn't satisfy the constexpr requirements, even though we're not supposed to. IIRC we try to hand-wave our way to conformance here by saying that all of the things that cause a constexpr constructor to fail to satisfy the constexpr requirements would also cause evaluation to fail, but it'd be worth double-checking that (in particular: what happens if the class has a virtual base class?).

aaron.ballman abandoned this revision.Aug 17 2022, 5:34 AM

This was partially covered by https://reviews.llvm.org/D131479 and the rest should be handled in a different patch, so abandoning.