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.

Sign up for GitHub

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

Jump to bottom

Tests don't compile (6.2.01, clang 13) #1933

Open
adriaandegroot opened this issue Nov 30, 2021 · 3 comments · May be fixed by #1945
Open

Tests don't compile (6.2.01, clang 13) #1933

adriaandegroot opened this issue Nov 30, 2021 · 3 comments · May be fixed by #1945

Comments

@adriaandegroot
Copy link
Contributor

The build fails with clang 13:

--- Praat_tests.o ---
Praat_tests.cpp:745:9: error: call to deleted constructor of 'Vec'
        return x;
               ^
Praat_tests.cpp:738:2: note: 'Vec' has been explicitly marked deleted here
        Vec (const Vec& other) = delete;   // cannot assign const Vec to Vec (4)

This makes sense, because, well .. because the copy-constructor for Vec is deleted. What I don't understand is why the code is the way it is. I can patch this not-useful bit of test code out in packaging, but the class is a bit strange.
@adriaandegroot
Copy link
Contributor Author

Vec is a resource-owning class, (the at pointer), so a copy should copy the (array? buffer?) that Vec points at. By deleting the copy-constructor, you're blocking all the places where implicit copies are made. You can add a move constructor,

Vec(Vec&& other) : at(nullptr), size(other.size) { std::swap(at, other.at); }

So that at least return x or return Vec() works.

@adriaandegroot
Copy link
Contributor Author

  • Vec doesn't have a destructor, so it will leak the buffer if it has one.
  • The commented-out test-code seems to suggest that x[1]=3.0 might work; in the current test, with at set to nullptr, it is UB though.
  • From the comments it seems like what you really want to be disabling is the copy-assignment operator .. or maybe putting a wrapper around std::vector<double> to implement copy-on-write.
  • A different fix, without adding a move-constructor, is to explicitly copy (via the non-const reference constructor). Then you can write return Vec(x) in the copy() function.

freebsd-git pushed a commit to freebsd/freebsd-ports that referenced this issue Nov 30, 2021
audio/praat: fix build on 14-
80ed4eb 
Build failures reported upstream at
	praat/praat#1933
The build failure is in test-code, in a static function
that does nothing useful -- looks like some C++-experimentation
that is still in the source tree. Massage it away so that
builds on 14- can happen. Bump PORTREVISION since on pre-14-
it might now pick a different constructor, and so potentially
the package changes.
@adriaandegroot adriaandegroot linked a pull request Dec 6, 2021 that will close this issue
Open
@PaulBoersma
Copy link
Member

well, this is a test, so its point is to generate warning messages and error messages. Of course those that generate actual error messages should be commented out. Can you tell me what exactly your compiler settings are?

By the way, Vec does not own the at pointer; it's a view. I was trying to have const propagate through the pointer, to no avail yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix tests by explicitly copying adriaandegroot/praat
2 participants
@PaulBoersma @adriaandegroot