There is a reason to be cautious there, the standard library also tends to static_assert on completeness instead of trying to be SFINAE-friendly.
That said, here's one possible place where it might make sense with SFINAE. Note that I haven't fully thought this through, I'm just doing this off the top of my head right now, so it's possible I'll miss something:
#include <iterator>
#include <type_traits>
namespace stdext { template<class> class checked_array_iterator; } // Forward-declare MSVC-specific type
namespace bar {
namespace {
template<class T>
std::enable_if_t<(std::is_same_v<stdext::checked_array_iterator<typename std::iterator_traits<typename T::foo_iterator>::value_type *>, typename T::foo_iterator> && ((void)sizeof(typename T::foo_iterator), true))> debug_verify(const T &obj, ...) {
// extra sanity checks against obj.get_foo_iterator()
}
template<class T>
std::enable_if_t<!(std::is_same_v<stdext::checked_array_iterator<typename std::iterator_traits<typename T::foo_iterator>::value_type *>, typename T::foo_iterator> && ((void)sizeof(typename T::foo_iterator), true))> debug_verify(const T &) {
// nothing to do
}
}
class S {
auto foo() {
bar::debug_verify(this->field1);
bar::debug_verify(this->field2);
}
// ...
};
}
(Edit: Removed a final comment about turning it into a is_checked_array_iterator trait - messed up that part.)
> which would avoid putting (void)sizeof(It) directly inside an SFINAE context. I'm just saying you could do it this way without violating ODR or other stuff I think (unless I'm missing something).
Putting it in an alias template doesn't sidestep putting it in an SFINAE context. The detection idiom works because it can still be an SFINAE context.
I thought that's why you wrote "static_assert instead of trying to be SFINAE-friendly" then, but okay.
> I'm just curious about a use case you have where you use enable_if on completeness.
That's all you want, without SFINAE? How about something like this:
template<class T>
class S {
std::unique_ptr<unsigned char[]> buf;
public:
template<std::enable_if_t<((void)sizeof(T), true), bool> = false>
explicit S(unsigned char *p, size_t n) : buf(p) {
assert(sizeof(T) <= n);
}
// ...
};
EDIT:
Come to think of it, actually I think static_assert here is less safe than enable_if and increases the risk of an ODR violation. Check out the difference in behavior here when you uncomment the enable_if_t:
#include <type_traits>
class X;
template<class T>
class S {
public:
// template<std::enable_if_t<((void)sizeof(T), true), bool> = false>
explicit S(unsigned char *) {
static_assert(sizeof(T) > 0, "oops");
}
};
static S<X> s(nullptr);
int main() { }
class X { };About your edit: I didn't know about that issue. Apparently it is ODR violation and ill-formed, no diagnostic required according to https://eel.is/c++draft/temp.point#7 :(. I'm not happy about it.
Having said that, yours is not free from ODR violation issues.
I saw it as "SFIAE" (no N) given there's no other overloads and thus you would get an error, but sure, I guess syntactically it's in an SFIANE context.
> ODR violation in this variable template itself https://eel.is/c++draft/meta#rqmts-5, or otherwise further down the line. [...] Having said that, yours is not free from ODR violation issues.
I'm not passing anything incomplete to something in <type_trait> anywhere in this code though.
In fact I don't even see where it says that such UB (even if you were to deliberately pass it to is_constructible) would be an ODR violation specifically.
Note the claim wasn't "it is impossible to write undefined behavior by using this snippet". The claim was just there's no ODR violation in that code. If your goal was to introduce undefined behavior in general there are about a million ways you can already do that even without this. This is just about the most benign kind of UB you can have in reality; I'm pretty darn sure no compiler is going to deliberately generate a buggy program with this snippet merely because the type was technically incomplete there. Worst case is you end up calling the wrong constructor, but you're the one writing the constructors, so you wouldn't be introducing potentially-conflicting overloads in the first place. And even in that case you would really have to deliberately go out of your way to write unrealistic code to get back a buggy program (i.e. such that the alternative constructor actually ends up doing the wrong thing). I know I've never seen this blow up in actual code.
> About your edit: I didn't know about that issue. Apparently it is ODR violation and ill-formed, no diagnostic required according to https://eel.is/c++draft/temp.point#7 :(. I'm not happy about it.
I dare say that situation is a lot more likely to cause problems in reality than the UB you're worried about, given forward declarations + subsequent accidental name collisions are things I've written in the past. If anything I would want to mitigate against this, not that.
template<class T>
class S {
public:
// template<std::enable_if_t<((void)sizeof(T), true), bool> = false>
explicit S(unsigned char *) {
static_assert(sizeof(T) > 0, "oops");
}
};
In any case, I think we're on the same page at this point; we both understand the issues and risks/benefits with both approaches. Hope the examples I provided were helpful.
Anyway, this discussion is dragging on forever, so I'm just gonna stop here and wrap this up with the following:
As I see it, the only realistic way you could ever generate a buggy program with this is if you're getting nerd-sniped and desperately trying to come up with a counterexample in response to someone's challenge on HN. For practical purposes I see it as a non-issue.