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

Fix std::functional problems. #677

Merged
merged 4 commits into from
Apr 24, 2023
Merged

Fix std::functional problems. #677

merged 4 commits into from
Apr 24, 2023

Conversation

jtv
Copy link
Owner

@jtv jtv commented Apr 22, 2023

Thanks @KayEss for pointing out that I shouldn't be using a std::function as a deleter for a std::unique_ptr. Using ordinary function pointers instead.

Also, finally figured out how to work around a bunch of gcc compile errors when I combined...

  • The address sanitizer or "asan" (-fsanitize=address).
  • Warnings as errors (-Werror).
  • Uninitialized analyzer (-Wanalyzer-use-of-uninitialized-value).

Unfortunately, the combination of audit mode (--enable-audit in the configure script) and maintainer mode (--enable-maintainer-mode in the configure script) got us into exactly that combination.

It would seem that there's a problem with gcc. It doesn't happen in clang. I worked around it by replacing the one regex in the code with a manual parsing loop like I used to have.

Thanks @KayEss for pointing out that I shouldn't be using a
`std::function` as a deleter for a `std::shared_ptr` or
std::unique_ptr`.  Using ordinary function pointers instead.

Also, finally figured out how to work around a bunch of gcc compile
errors when I combined...
* The address sanitizer or "asan" (`-fsanitize=address`).
* Warnings as errors (`-Werror`).
* Uninitialized analyzer (`-Wanalyzer-use-of-uninitialized-value`).

Unfortunately, the combination of audit mode (`--enable-audit` in the
`configure` script) and maintainer mode (`--enable-maintainer-mode`
in the `configure` script) got us into exactly that combination.

It would seem that there's a problem with gcc.  It doesn't happen in
clang.  I worked around it by replacing the one regex in the code with
a manual parsing loop like I used to have.
@KayEss
Copy link
Contributor

KayEss commented Apr 22, 2023

I shouldn't be using a std::function as a deleter for a std::shared_ptr or std::unique_ptr

A closure is fine for a shared_ptr -- it's a much heavier weight abstraction and the closure just goes alongside the control block (although with std::function I think it can cost an extra allocation if it's large enough). With unique_ptr they're not allowed because it's meant to be a very light weight abstraction near that of a single pointer, so the custom deleter has to go in the type not in the instance.

@jtv
Copy link
Owner Author

jtv commented Apr 22, 2023

I shouldn't be using a std::function as a deleter for a std::shared_ptr or std::unique_ptr

A closure is fine for a shared_ptr -- it's a much heavier weight abstraction and the closure just goes alongside the control block (although with std::function I think it can cost an extra allocation if it's large enough). With unique_ptr they're not allowed because it's meant to be a very light weight abstraction near that of a single pointer, so the custom deleter has to go in the type not in the instance.

Thanks for explaining. This is far from foolproof, which is disappointing. Now that we're here I suppose I'll keep the plain old function pointer, if there's any chance that it's the slightest bit more efficient. Or less verbose code, really.

@jtv jtv merged commit 5b71ced into master Apr 24, 2023
3 checks passed
@jtv jtv deleted the functional-fixes branch April 24, 2023 21:54
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

Successfully merging this pull request may close these issues.

None yet

2 participants