Add SubsystemType concept to better enforce presence of get_subsystem_name()
#7567
+12
−10
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We previously had this comment, but it's easy to miss:
And then you'd get an error at the point of use, if your new type didn't meet this requirement:
This kind of error is familiar to seasoned C++ developers, but ugly - it is thrown during template substitution, dumping the include-stack of a bunch of internal headers but not including the line in our new code that actually triggers this. Fundamentally, the fact that
install_subsystemrequiresget_subsystem_name()is only discovered when you hit this error, or by noticing the comment, and working out the required return type requires further spelunking or error message iteration.By writing this requirement as an explicit constraint we get an earlier, clearer (and more verbose, for better and worse) error:
Including the correct line in the callstack is a huge win, and better yet we don't need to look at how
install_subsystem()is actually implemented - its interface/signature precisely describes the requirement, and is all we need to look at to produce write a fix.Those are the pros, but let's just list some cons for discussion:
conceptin our code. Creeping complexity of adopting new language features.{ T::get_subsystem_name() } -> std::convertible_to<std::string_view>as "your type must have a static function namedget_subsystem_name()returning something convertible to a string"? With squinting, and practice.