Handle operators #447
Handle operators #447
Conversation
|
I think this doesn't work, technically speaking. The reason is that according to C++11, only classes without base classes count as POD, and handles really ought to stay POD. In particular, the last time I changed some existing classes to be non-POD, I immediately reaped a bunch of problems with default-initialization and zero-initialization not working as expected--IIRC it was related to default constructor not zero-initializing POD members or something like that but my memory is hazy. I am neither opposed nor particularly in favor of this, stylistically speaking, since the benefit is not very large and there is quite some churn. I'd have to think more (and probably discuss more) to see if this is a worthwhile change in that respect. |
|
Oh yeah and it breaks horribly on MSVC for the same reasons. But that's probably the least of the worries here, VS2013 is six years old and we're getting pretty close to the point where upgrading it makes good sense. Not only is VS2013 very buggy, but MS recently shut down the only resource (MS Connect) that had a collection of workarounds for these bugs, so even though I'll get some hate from some last Windows XP holdouts out there, it's really hard to justify still supporting it. |
|
I just tried to bump the MSVC requirement to 2015 and of course the testsuite crashes with |
|
ah, I figured I must have changed something fundamental for pod initialization to stop working here with GCC - worked around by adding the two constructors. (Pretty sure newer compilers also have xp support in a separate toolchain?) Can do this in a different way (either code duplication/macro usage or traits) without touching the POD classes themselves, if it's interesting. edit: oh it's used in a union... eek... yeah quickly getting out of "quickie cleanup hack" territory :) |
|
Oh, how'd traits work here? That sounds nicer than CRTP. |
|
err, like this... https://github.com/rpavlik/solvespace/tree/handle-operators-2 (oops, I patched it again) Don't know how to inject the "explicit operator bool" this way, but it at least gives you equality/inequality. |
|
I think this approach is not really viable, but #448 might be good. |
Handles seem like the kind of thing you should be able to compare, and the .v scattered throughout the codebase was an impediment to me learning it. Finally did a little modification I meant to do for a while: provide
==,!=,<andexplicit operator bool(aka "use inifstatements") operators for all the handle types. Easiest, least messy way was with a curiously-recurring-template-pattern base class so there is only one implementation of the operators, yet the handles are still type-safe (can't compare across handle types without using.vto get the uint32_t).If it seems useless to you, no hard feelings if you just reject this (or drop parts of it, like the operator bool perhaps, since that seems less obviously-useful than the comparison operators). FWIW, I also have a version of this based on #428 (actually I devved it there first then rebased it onto master), at https://github.com/rpavlik/solvespace/tree/handle-operators-after-simplify
Could revise to not use CRTP, but would then need a "tag type" for each handle type to keep them distinct, so this seemed simplest.