Hacker News new | ask | show | jobs
by dataflow 530 days ago
> None of those are easier than not needing to do it at all though; if your functions exits are only where you specify, you can cleanup only once on those paths.

Huh? I don't get it. This:

  stack.push_back(k);
  absl::Cleanup _ = [&] { assert(stack.back() == k); stack.pop_back(); }
  if (foo()) {
    printf("foo()\n");
    return 1;
  }
  if (bar()) {
    printf("bar()\n");
    return 2;
  }
  baz();
  return 3;
is both easier, more readable, and more robust than:

  stack.push_back(k);
  if (foo()) {
    printf("foo()\n");
    assert(stack.back() == k);
    stack.pop_back();
    return 1;
  }
  if (bar()) {
    printf("bar()\n");
    assert(stack.back() == k);
    stack.pop_back();
    return 2;
  }
  baz();
  assert(stack.back() == k);
  stack.pop_back();
  return 3;
as well as:

  stack.push_back(k);
  auto pop_stack = [&] { assert(stack.back() == k); stack.pop_back(); }
  if (foo()) {
    printf("foo()\n");
    pop_stack();
    return 1;
  }
  if (bar()) {
    printf("bar()\n");
    pop_stack();
    return 2;
  }
  baz();
  pop_stack();
  return 3;
and unlike the others, it avoids repeating the same code three times.

(Ironically, I missed the manual cleanups before the final returns in the last two examples right as I posted this comment. Edited to fix now, but that itself should say something about which approach is actually more bug-prone...)

1 comments

I can't parse this super well on mobile, but what invariant is this maintaining? I was imagining a function that manipulated a collection, and e.g. needed to decrement a length field to ensure the observable elements remained valid, then increment it, then do something else.

The gnarliest scenario I recall was a ring-buffer implementation that relied on a field always being within the valid length, and a single code path not performing a mod operation, which was only observably a problem after a specific sequence of reserving, popping, and pushing.

EDIT: oh, I think I see; is your code validating the invariant, or maintaining it?

> I can't parse this super well on mobile, but what invariant is this maintaining.

The stack length (and contents, too). It pushes, but ensures a pop occurs upon returning. So the stack looks the same before and after.

> I was imagining a function that manipulated a collection, and e.g. needed to decrement a length field to ensure the observable elements remained valid, then increment it, then do something else.

That is exactly what the code is doing.

> EDIT: oh, I think I see; is your code validating the invariant, or maintaining it?

Both. First it manipulates the stack (pushing onto it), then it does some stuff. Then before returning, it validates that the last element is still the one pushed, then pops that element, returning the stack to its original length & state.

> The gnarliest scenario I recall was a ring-buffer implementation that [...]

That sounds like the kind of thing scope guards would be good at.

Then I think the counter-example is where function calls that can't fail are interspersed. Those are the cases where with exceptions (outside checked exceptions) you have to assume they could fail, and in a language without exceptions you can rely on them not to fail, and skip adding any code to maintain the invariant between them.

E.g. in the case you provided, if pop & push couldn't fail, that would just be two calls in sequence.

I still don't follow, I'm sorry.

> E.g. in the case you provided, if pop & push couldn't fail, that would just be two calls in sequence.

I have no idea what you mean here. Everything in the comment would be exactly the same even if stack.push_back() was guaranteed to succeed (maybe due to a prior stack.reserve()). And those calls aren't occurring in sequence, one is occurring upon entrance and the other upon exit. Perhaps you're confused what absl::Cleanup does? Or I'm not sure what you mean.

I think you're going to have to give a code example if/when you have the chance, to illustrate what you mean.

But also, even if you find "a counterexample" where something else is better than exceptions just means you finally found found a case where there's a different tool for a (different) job. Just like how me finding a counterexample where exceptions are better doesn't mean exceptions are always better. You simply can't extrapolate from that to exceptions being bad in general, is kind of my whole point.

Apologies, I believe I meant if the foo/bar/baz calls couldn't fail. If there's no exceptions, you don't need the cleanup block, but in the presence of exceptions you have to assume they (and all calls) can fail.

The problem re. there being a counter-example to exceptions (as implemented in C++) is that they're not opt-in or out where it makes sense. At least as I understand it, there's no way for foo/bar/baz to guarantee to you that they can't throw an exception, so you can rely on it (e.g. in a way that if this changes, you get a compiler error such that something you were relying on has changed). noexcept just results in the process being terminated on exception right?

> I meant if the foo/bar/baz calls couldn't fail. If there's no exceptions, you don't need the cleanup block

First, I think you're making an incorrect assumption -- the assumption that "if (foo())" means "if foo() failed". That's not what it means at all. They could just as well be infallible functions doing things like:

  if (tasks.empty()) {
    printf("Nothing to do\n");
    return 1;
  }
or

  if (items.size() == 1) {
    return items[0];
  }
Second, even ignoring that, you'd still need the cleanup block! The fact that it is next to the setup statement (i.e. locality of information) is extremely important from a code health and maintainability standpoint. Having setup & cleanup be a dozen lines away from each other makes it far too easy to miss or forget one of them when the other is modified. Putting them next to each other prevents them from going out of sync and diverging over time.

Finally, your foo() and bar() calls can actually begin to fail in the future when more functionality is added to them. Heck, they might call a user callback that performs arbitrary tasks. Then your code will break too. Whereas with the cleanup block, it'll continue to be correct.

What you're doing is simplifying code by making very strong and brittle -- not to mention unguaranteed in almost all cases -- assumptions on how it looks during initial authorship, and assuming the code will remain static throughout the lifetime of your own code. In that context, putting them together seems "unnecessary", yeah. But point-in-time programming is not software engineering. The situation is radically different when you factor in what can go wrong during updates and maintenance.