Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Usage of special member functions #300

Closed
miscco opened this issue May 6, 2019 · 6 comments
Closed

Usage of special member functions #300

miscco opened this issue May 6, 2019 · 6 comments

Comments

@miscco
Copy link

miscco commented May 6, 2019

Generally it is considered best practise to not declare special member functions needlessly but rather default them, e.g. the trivial default constructor

struct foo {
   foo() {}
};

would be better declared

struct foo {
   foo() = default;
};

The same applies for the other special member functions, Greg Falcon even mentions it in his talk at CppCon2018 [https://www.youtube.com/watch?v=6ZOygaUjzjQ]. Nevertheless these special member functions are present everywhere in abseil, e.g.

constexpr node_handle_base() {}

Is there a reason for that or is that just on the bottom of the list?

@asoffer
Copy link

asoffer commented May 14, 2019

With = default, the constructor will not be constexpr.

@miscco
Copy link
Author

miscco commented May 14, 2019

According to cppreference this is not the case since C++11. In fact, the current C++ standard draft reads in Chapter 15.1 [class.ctor] §7:

A default constructor that is defaulted and not defined as deleted is implicitly defined when it is odr-used to create an object of its class type or when it is explicitly defaulted after its first declaration.
The implicitly-defined default constructor performs the set of initializations of the class that would be performed by a user-written default constructor for that class with no ctor-initializer and an empty compound-statement.
If that user-written default constructor would be ill-formed,the program is ill-formed.
If that user-written default constructor would satisfy the requirements of a constexpr constructor the implicitly-defined default constructor is constexpr.
Before the defaulted default constructor for a class is implicitly defined, all the non-user-provided default constructors for its base classes and its non-static data members shall have been implicitly defined.

Similarly a implicitely defined default constructor also inherits the exception specification that follow from the class-members

@zhangxy988
Copy link
Contributor

Note that the proposed change does change traits behavior, e.g. https://gcc.godbolt.org/z/ek-8XZ.
I am not sure why the constructors were written in the form constexpr T() {} instead of T() = default. For that we need to check with the original author.
Do you have a specific concern on this other than "it looks better"?

@miscco
Copy link
Author

miscco commented Jun 7, 2019

Yes the implicitely defaulted constructor indeed may change the type traits of the class. However please note that they are strictly more correct.

In you example there is no reason why Bar should not be trivially constructible. Even worse the fourth assert is an actual bug. Bar<NoDefaultCtor> should not be marked as default constructible

@brenoguim
Copy link

Having correct traits behavior is kind of important. Several containers optimize based on traits. Not sure its relevant in this case, but in general it's kind of important.

@miscco
Copy link
Author

miscco commented Aug 5, 2019

THere was a internal discussion and the corresponding PRs were closed. Insofar this issue should be closed too

@miscco miscco closed this as completed Aug 5, 2019
copybara-service bot pushed a commit that referenced this issue Sep 11, 2024
  - d1397431006ea1362a5914d4a90b265d0c7c6f2c Update zoneinfo files to 2024b (#300) by Bradley White <14679271+devbww@users.noreply.github.com>
  - 8bdbd840e97ff32e17f25db85c82589819ad3352 Fix mingw compiler error due to missing function declarat... by Biswapriyo Nath <nathbappai@gmail.com>
  - 6624659e01e73e41527d6b27036e9f79a556560f Add GitHub Actions CI (#299) by Derek Mauro <761129+derekmauro@users.noreply.github.com>

PiperOrigin-RevId: 673451051
Change-Id: Id39f2186bbdcb802d4fc4c5e21207c6f3709c56f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants