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

Maintain experimental branches #6204

Draft
wants to merge 30 commits into
base: master
Choose a base branch
from

Conversation

Paul-Licameli
Copy link
Collaborator

@Paul-Licameli Paul-Licameli commented Apr 2, 2024

Resolves: (direct link to the issue)

Depends on

Expect more commits, but review may commence now.

Eliminate Experimental.cmake, and instead introduce namespace Experimental, which may be scattered.

The intention is that all experimentals are governed by constexpr booleans in a namespace
Experimental which need not be defined all in one widely included header, but a textual scan
of the source code can easily find all of them.

EXPERIMENTAL_EQ_SSE_THREADED will be removed (using pffft is the more promising thing for
better single-threaded EQ performance) but the rest will be repaired to keep them buildable.
This even includes the ScoreAlign experiment of @rbdannenberg which @saintmatthieu may also
find interesting.

I pass no judgment on the rest of them -- whether they were good or bad, they have simply been
forgotten and neglected. Maybe there is something of value in some of them to be rediscovered.

History: Experimental.cmake was introduced to error-proof the copy-paste of code containing
EXPERIMENTAL_* switches. Doing that but neglecting to #include "Experimental.h" could
cause unintended changes of behavior. Instead I introduced Experimental.cmake, to inject a -D
compiler option for every .cpp file for enabled experiments.

This was fool-proof, but introduced the inconvenience, that changing Experimental.cmake would
cause everything to recompile in one's own development build.

This alternative should allow switching experimentals on, and waiting less time to build.

And better, it will prevent the un-committed experimentals from falling into an un-buildable state
of disrepair, if instead, the C++17 if contexpr construct is used. The branch that is false (and
known false at compile time) is still required to pass compilation checks, so other changes to function
signatures will force updates of those disabled branches too to keep a successful build.

  • I signed CLA
  • The title of the pull request describes an issue it addresses
  • If changes are extensive, then there is a sequence of easily reviewable commits
  • Each commit's message describes its purpose and effects
  • There are no behavior changes unnecessary for the stated purpose of the PR

Recommended:

  • Each commit compiles and runs on my machine without known undesirable changes of behavior

@Paul-Licameli Paul-Licameli added the code quality Type safety, elimination of warnings, modernizing C++ idioms, etc. label Apr 2, 2024
@Paul-Licameli Paul-Licameli marked this pull request as draft April 2, 2024 22:13
@Paul-Licameli Paul-Licameli force-pushed the Maintain-experimental-branches branch 6 times, most recently from 17a4a34 to 7ceed82 Compare April 5, 2024 15:29
@Paul-Licameli
Copy link
Collaborator Author

Rebased and resolved conflicts with wave-clip-refactoring

@Paul-Licameli Paul-Licameli force-pushed the Maintain-experimental-branches branch from 7ceed82 to 3343eb6 Compare April 5, 2024 16:30
@Paul-Licameli Paul-Licameli force-pushed the Maintain-experimental-branches branch 3 times, most recently from d6f231b to 243780d Compare April 15, 2024 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code quality Type safety, elimination of warnings, modernizing C++ idioms, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant