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

Uniformly return by value #29

Open
2 tasks
johannesugb opened this issue Oct 30, 2020 · 0 comments
Open
2 tasks

Uniformly return by value #29

johannesugb opened this issue Oct 30, 2020 · 0 comments
Labels
bug Something isn't working C++ C++-centric task

Comments

@johannesugb
Copy link
Member

johannesugb commented Oct 30, 2020

As pointed out in some C++ talk that I'll have to find once again returning by reference or even const-reference can be very dangerous. Because if the returnee dies, the reference is dangling. Therefore, one should always return by value.

There are some exceptions to this rule where it is okay to return by reference --- namely when it can be ensured with certainty that the returnee outlives anything that could be done with a returned reference. Specifically, that applies to:

  • root::physical_device
  • root::device
  • root::dynamic_dispatch
  • root::memory_allocator

In many cases (e.g. returning a handle) return by value has probably exactly the same performance cost as returning the reference. I.e. in many cases, you don't get anything positive from returning a reference. In some cases, return by reference can be cheaper: e.g. when returning a vector of something. However, correctness is always more important than performance.

Actually, there is also a potential "security" risk: By const_cast-ing away the const of a const T&, one could modify the internal state through the returned reference.

Definition of Done:

  • All functions/methods follow the "return by value" principle.
  • Concrete locations TBD.
@johannesugb johannesugb added bug Something isn't working C++ C++-centric task labels Oct 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working C++ C++-centric task
Development

No branches or pull requests

1 participant