Hacker News new | ask | show | jobs
by raptorfactor 405 days ago
This is cursed:

https://github.com/es3n1n/defendnot/blob/master/defendnot-lo...

If you're curious what's actually going on there:

https://github.com/es3n1n/defendnot/blob/master/cxx-shared/s...

4 comments

can someone well versed in explaining CPP magic explain what is going on and why it is cursed?
We're starting with this code:

   defer->void { CoUninitialize(); };
Using the macros in the second linked file, this expands to:

   auto _defer_instance_1234 = Defer{} % [&]()->void { CoUninitialize(); };
* The 1234 is whatever the line number is, which makes the variable name unique.

* auto means infer the type of this local variable from the expression after the =.

* Defer{} means default construct a Defer instance. Defer is an empty type, but it allows the % following it to call a specific function because...

* Defer has an overloaded operator%. It's a template function, which takes a callable object (type is the template parameter Callable) and returns a DeferHolder<Callable> instance.

* [&]()->void { /*code here*/ }; is C++ syntax for a lambda function that captures any variables it uses by address (that's the [&] bit), takes no parameters (that's the () bit) and returns nothing (that's the ->void bit). The code goes in braces.

* DeferHolder calls the function it holds when it is destroyed.

It's subjective but some (including me!) would say it's cursed because it's using a macro to make something that almost looks like C++ syntax but isn't quite. I'm pretty confident with C++ but I had no idea what was going on at first (except, "surely this is using macros somehow ... right?"). [Edit: After some thought, I think the most confusing aspect is that defer->void looks like a method call through an object pointer rather than a trailing return type.]

I'd say it would be better to just be honest about its macroness, and also just do the extra typing of the [&] each time so the syntax of the lambda is all together. (You could then also simplify the implementation.) You end up with something like this:

   DEFER([&]()->void { CoUninitialize(); });
Or if you go all in with no args lambda, you could shorten it to:

   DEFER({ CoUninitialize(); });
A way to do the same thing that is less gross: https://github.com/abseil/abseil-cpp/blob/master/absl/cleanu...
That's interesting! So i assume that this macro allows code to get registered to be run after the 'current' scope exits.

But from my understanding (or lack thereof), the `auto _defer_instance_1234 =` is never referenced post construction. Why doesn't the compiler immediately detect that this object is unused and thus optimize away the object as soon as possible? Is it always guaranteed that the destructor gets called only after the current scope exits?

> Why doesn't the compiler immediately detect that this object is unused and thus optimize away the object as soon as possible? Is it always guaranteed that the destructor gets called only after the current scope exits?

Yes, exactly. The destructor is allowed to have some visible side effect such as closing a file handle or unlocking a mutex that could violate the assumption of the code in that block. (Even just freeing some memory could be an issue for code in the block.) It is guaranteed that the destructor is closed at the end of the block, and that all the destructors called in that way happen in reverse order to the order of their corresponding constructors.

Yes, this is guaranteed. The compiler cannot simply elide statements with effects.
I don't think we actually need `->void` -- shouldn't the compiler be able to infer the return type (or rather, absence thereof)? My experience is that the compiler only struggles when the return value needs to be implicitly converted to some other type.

Would it have looked any less cursed if it just read `defer { CoUninitialize(); };`?

Agreed that the simplest "fix" would be to just rename the macro to be all-caps.

> I don't think we actually need `->void`

Yes, agreed.

> Would it have looked any less cursed if it just read `defer { CoUninitialize(); };`?

It's subjective but personally I still hate it.

> Agreed that the simplest "fix" would be to just rename the macro to be all-caps.

Actually I think the bigger part of my suggestion is switching from an object-like macro to a function-like macro [1], which makes it all a bit less magical.

[1] https://stackoverflow.com/questions/36126687/function-like-m...

And, I personally hate macros that pretend to be functions but provide no visual indicator that they're not actually functions. For instance, `#define min(x, y) (x < y ? x : y)` evaluates its args multiple times. It's a little less bad when it only takes a single argument, but I am still irritated by things like htonl.

I think the "best" approach here would be to make it a function-like macro, and also change the name to all caps.

(Also, I tend to agree that `defer { ... };` is still cursed -- it requires the trailing semicolon, which further breaks the illusion of a keyword that takes a block scope.)

> * Defer has an overloaded operator%. It's a template function, which takes a callable object (type is the template parameter Callable) and returns a DeferHolder<Callable> instance.

Is there any reason to use operator% instead of a normal method call? Except possibly looking cool, which doesn't seem useful given that the call is hidden away in a macro anyway.

If you used a normal method call then there would need to be a corresponding close bracket at the end of the overall line of code, after the end of the lambda function. But the macro ("defer") only occurs at the start of the line, so it has no way to supply that close bracket. So the caller of the macro would have to supply it themselves. As I mentioned near the end of my comment, it seems like the defer macro is specifically engineered to avoid the caller needing a close bracket.

If you don't mind that, I said that you can "simplify the implementation" - what I meant was, as you say, you don't need the overloaded Defer::operator% (or indeed the Defer class at all). Instead you could do:

   template <typename Callable>
   DeferHolder<Callable> _get_defer_holder(Callable&& cb) {
       return DeferHolder<Callable>{std::forward<Callable>(cb)};
   }
   #define DEFER(my_lambda) auto COMMON_CAT(_defer_instance_, __LINE__) = _get_defer_holder(my_lambda)
Disclaimer: I haven't tried it and I don't normally write macros so this could have glaring issues.
Eh, there are better implementations that are less syntactically obtuse (no ->void) but other than that it’s fine. Fairly obvious what it’s supposed to do and I’ve needed similar things in the past. There’s a cppcon talk that use ->* operator for precedence reasons and the macro lets you use it like ‘defer { … };’
This is a class which implements a 'defer' mechanism, similar to Go and Javascript constructs, which do the same thing - delay execution of the given block until the current block scope is exited. Its pretty clever, actually, and quite useful.

I personally don't find it that cursed, but for many old C++ heads this may be an overwhelming smell - adding a class to implement what should be a language feature may tweak some folks' ideology a bit too far.

C++ sort-of guarantees that your objects' destructors will be called when they go out of scope.

So you can abuse this mechanic to 'register' things to be executed at the end of the current scope, almost no matter how you exit the current scope.

yeah sorry i didnt feel like implementing my own RAII stuff for all the COM thingies due to time constraints. it will be changed in the next update though
Honestly if this isn't part of a public API this isn't very cursed in terms of C++, especially if you have a lot of one-off cleanup operations.

I think the only bit I don't like personally is the syntax. I normally implement defer as a macro to keep things clean. If done correctly it can look like a keyword: `defer []{ something(); };`.

I think the syntax is exactly why they're saying it's cursed. IMO your suggestion is no better - yes it makes defer look like a keyword, but it's not! As I said in a sibling comment, I think it's clearer if you're honest that you're using a macro: DEFER([](){something();});

Or you could even make a non-macro version (but then you need to think of variable names for each defer):

   auto defer_uninitialise = do_defer([](){CoUninitialize();});
Sure, I've used __LINE__ for this before too, and yeah I agree that my keyword construction was too clever (seemed cool at the time, since the macro had a dangling = at the end to make it work).
Why did you write it with two structs though? You could do

    #define defer(body) DeferHolder COMMON_CAT(_defer_instance, __LINE__) {([&]()->void body)};
and call it as

    defer({
        function body here;
    });
Which looks much nicer. The preprocessor treats balanced curlies as one single token regardless of how many lines it spans, precisely to enable this usage.
What's cursed about this? I use this pattern all over in my code although the signature at the callsite looks a bit different (personal preference).

D (for example) has the concept of statements that trigger at end of scope built into the language.

Code is a way you treat your coworkers - Michael Feather, https://x.com/mfeathers/status/1031176879577780224

TL;DR, not AI

The code defers a function call until the point in time that an object goes out of scope. The implementation uses C macros to create a more succinct syntax that omits parts of the necessary C lambda/unnamed function definition and to create a unique variable name for managing the deferred function call. However, the resulting syntax eschews the common convention of using UPPER CASE to denote C macros, and instead appears similar at first glance to a function call from an object pointer.

This can cause confusion if one is not familiar with this pattern and expects macros to be communicated differently. Some commenters say this is common enough, or useful enough to them, to be considered almost idiomatic in some contexts.

For technical explanation, https://news.ycombinator.com/item?id=43959403#43960905 provides a useful breakdown of how the macro works.