Hacker Newsnew | past | comments | ask | show | jobs | submit | MontagFTB's commentslogin

Putting code with side effects into an assert is asking for trouble. Compile with NDEBUG set and the effects mysteriously disappear! Anything beyond an equality expression or straight boolean should be avoided.

I once spent several days debugging that same mistake. Stuff worked perfectly in tests but broke misteriously in production builds. Couldn't stop laughing for a few minutes when I finally figured it out.

Related our logging system has a debug which is not logged by default but can be turned on if a problem in an area is found (in addition to the normal error/info which is logged). I had the idea that if a test fails we should print all these debugs - easy enough to turn on but a number of tests failed because of side effects that didn't show up when off.

i'm trying to think of how/if we can run tests with all logging off to find the error and info logs with side effects.


This is just a symptom of a bad assert() implementation, which funny enough is the standard. If you properly (void) it out, side effects are maintained.

https://github.com/fiberfs/fiberfs/blob/7e79eaabbb180b0f1a79...


assert() is meant to be compiled away if NDEBUG is defined, otherwise it shouldn't be called assert(). Given that assert() may be compiled away, it makes sense not to give it anything that has side effects.

Abseil has the convention where instead of assert(), users call "CHECK" for checks that are guaranteed to happen at run time, or "DCHECK" for checks that will be compiled away when NDEBUG is defined.

https://github.com/abseil/abseil-cpp/blob/0093ac6cac892086a6...

https://github.com/abseil/abseil-cpp/blob/0093ac6cac892086a6...


If your assert compiles down to `if (condition) {}` in production then the compiler will optimize away the condition while keeping any side effects.

Yeah which may not be what you want. E.g. `assert(expensive_to_compute() == 0)`.

The correct way to solve this is with debug asserts (as in Rust, or how the parent described).


Genuine question, does Rust know if `expensive_to_compute()` has side effects? There are no params, so could it be compiled out if the return value is ignored? Ex: `expensive_to_compute()` What about: `(void) expensive_to_compute()`?

No, in general Rust doesn't (and can't) know whether an arbitrary function has side effects. The compiler does arguably have a leg up since Rust code is typically all built from source, but there's still things like FFI that act as visibility barriers for the compiler.

No, Rust is the same as C++ in terms of tracking side effects. It doesn't matter that there are no parameters. It could manipulate globals or call other functions that have side effects (e.g. printing).

What about rust const fn()? I think it guarantees there are no side effects

I think you're right. Equivalent to C++'s constexpr.

Compilers are very good these days. If it has no side effects it will likely be compiled out.

That's why you define your own assert macro and keep in on unconditionally. Your programs will be better for it.

An assertion can be arbitrarily expensive to evaluate. This may be worth the cost in a debug build but not in a release build. If all of assertions are cheap, they likely are not checking nearly as much as they could or should.

Possibly but I've never seen it in practice that some assert evaluation would be the first thing to optimize. Anyway should that happen then consider removing just that assert.

That being said being slow or fast is kinda moot point if the program is not correct. So my advisor to leave always all asserts in. Offensive programming.


Rust has assert and debug_assert, which are self-explanatory. But it also has an assert_unchecked, which is what other languages incl C++ call an "assume" (meaning "this condition not holding is undefined behaviour"), with the added bonus that debug builds assert that the condition is true.

Notably, like most things with "unchecked" in their name `core::hint::assert_unchecked` is unsafe, however it's also constant, that is, we can do this at compile time, it's just promising that this condition will turn out to be true and so you should use it only as an optimisation.

Necessarily, in any language, you should not optimise until you have measured a performance problem. Do not write this because "I think it's faster". Either you measured, and you know it's crucial to your desired performance, or you didn't measure and you are wasting everybody's time. If you just scatter such hints in your code because "I think it's faster" and you're wrong about it being true the program has UB, if you're wrong about it being faster the program may be slower or just harder to maintain.


Side effects are bad of course, but anything beyond a straight boolean or equality is bad?

`assert(vector.size() < 3)` is ridiculous to you?


I actually feel like asserts ended up in the worst situation here. They let you do one line quick checks which get compiled out which makes them very tempting for those but also incredibly frustrating for more complex real checks you’d want to run in debug builds but not in release.

Indeed.

   bool is_even(int* valPtr) {
      assert(valPtr != nullptr);
      return *valPtr % 2;
    }
Does not do what you think it does with nullptr. A major game engine [0] has a toggle to enable asserts in shipping builds, mostly for this reason

[0] https://dev.epicgames.com/documentation/en-us/unreal-engine/...


Let's not vague post on HN. What's the problem with the above?

The problem is the code unconditionally dereferences the pointer, which would be UB if it was a null pointer. This means it is legal to optimize out any code paths that rely on this, even if they occur earlier in program order.

But if the assertion fails, the program is aborted before the pointer would have been dereferenced, making it not UB. This explanation is bogus.

Only if the assert is active. It basically means that the code is invalid when NDEBUG is set.

When NDEBUG is set, there is no test, no assertion, at all. So yes, this code has UB if you set NDEBUG and then pass it a null pointer — but that's obvious. The code does exactly what it looks like it does; there's no tricks or time travel hiding here.

> it is legal to optimize out any code paths that rely on this, even if they occur earlier in program order.

I don't think this is true. The compiler cannot remove or reorder instructions that have a visible effect.

  if (p == 0)
    printf("Ready?\n");
  *p++;
The printf() can't be omitted.

> The compiler cannot remove or reorder instructions that have a visible effect.

You might be surprised! When it comes to UB compilers can and do reorder/eliminate instructions with side effects, resulting in "time travel" [0].

IIRC the upcoming version of the C standard bans this behavior, but the C++ standard still allows it (for now, at least).

[0]: https://devblogs.microsoft.com/oldnewthing/20140627-00/?p=63...


No, this is explicitly legal. Most compilers will shy away from it these days since it made a lot of people upset, but it's definitely allowed.

> The problem is the code unconditionally dereferences the pointer, which would be UB if it was a null pointer.

Only when NDEBUG is defined, right?


No, the code that does this is always active

Right so strictly speaking C++ could do anything here when passed a null pointer, because even though assert terminates the program, the C++ compiler cannot see that, and there is then undefined behaviour in that case

> because even though assert terminates the program, the C++ compiler cannot see that

I think it should be able to. I'm pretty sure assert is defined to call abort when triggered and abort is tagged with [[noreturn]], so the compiler knows control flow isn't coming back.


Shouldn't control flow diverge if the assert is triggered when NDEBUG is not defined? Pretty sure assert is defined to call abort when triggered and that is tagged [[noreturn]].

Sorry, yes, I misread you

I'm sorry, but what exactly is the problem with the code? I've been staring at it for quite a while now and still don't see what is counterintuitive about it.

There's nothing wrong with it. It does exactly what you think it does when passed null.

A lot of compilers will optimize out a NULL pointer check because dereferencing a NULL pointer is UB.

Because assert will not run the following code in the case of a NULL pointer, AFAIK this exact code is still defined behavior, but if for some reason some code dereferenced the NULL pointer before, it would be optimized out - there are some corner cases that aren't obvious on the surface.

This kind of thing was always theoretically allowed, but really started to become insidious within the past 5-10 years. It's probably one of the more surprising UB things that bites people in the field.

GCC has a flag "-fno-delete-null-pointer-checks" to specifically turn off this behavior.

https://qinsb.blogspot.com/2018/03/ub-will-delete-your-null-...

This is an actual Linux kernel exploit caused by this behavior where the compiler optimized out code that checked for a NULL pointer and returned an error.

https://lwn.net/Articles/342330/


Sure, but none of that is relevant to just the code snippet that was posted. The compiler can exploit UB in other code to do weird things, but that's just C being C. There's nothing unexpected in the snippet posted.

The issue is cause by C declaring that dereferencing a null pointer is UB. It's not really an issue with assertions.

You can get the same optimisation-removes-code for any UB.


> There's nothing unexpected in the snippet posted.

> The issue is cause by C declaring that dereferencing a null pointer is UB. It's not really an issue with assertions. > You can get the same optimisation-removes-code for any UB.

I disagree - It’s a 4 line toy example but in a 30-40 line function these things are not always clear. The actual problem is if you compile with NDEBUG=1, the nullptr check is removed and the optimiser can (and will, currently) do unexpected things.

The printf sample above is a good example of the side effects.


> The actual problem is if you compile with NDEBUG=1

That is entirely expected by any C programmer. Sure they named things wrong - it should have been something like `assert` (always enabled) and `debug_assert` (controlled by NDEBUG), as Rust did. And I have actually done that in my C++ code before.

But I don't think the mere fact that assertions can be disabled was the issue that was being alluded to.


I wrote the comment, assertions being disabled was exactly what was being alluded to.

> that is entirely expected by any C programmer

That’s great. Every C programmer also knows to avoid all the footguns and nasties - yet we still have issues like this come up all the time. I’ve worked as a C++ programmer for 12 years and I’d say it’s probably 50/50 in practice how many people would spot that in a code review.


It's definitely a footgun, but the compiler isn't doing weird stuff because the assertions can be disabled. It's doing weird stuff because there's UB all over the place and it expects programmers to magically not make any mistakes. Completely orthogonal to this particular (fairly minor IMO) footgun.

> I’ve worked as a C++ programmer for 12 years and I’d say it’s probably 50/50 in practice how many people would spot that in a code review.

Spot what? There's absolutely nothing wrong with the code you posted.


That assert could completely fail to fire if inlined into another function that did a dereference first.

Depends on where you're coming from, but some people would expect it to enforce that the pointer is non-null, then proceed. Which would actually give you a guaranteed crash in case it is null. But that's not what it does in C++, and I could see it not being entirely obvious.

Assert doesn't work like that in any language.

It does in Rust: assert is always enabled, whereas the debug-only version is called debug_assert.

But yes, “assert” in most languages is debug-only.


He said

> some people would expect it to enforce that the pointer is non-null, then proceed

No language magically makes the pointer non-null and then continues. I don't even know what that would mean.


This is a very "Dr Dr it hurts when I do this" "Don't do that" one it must be said.

I don't mean to be that guy, but for "functional" programmers a print statement has "side effects".

But your meaning is clear. In an assert expression, don't call functions that might change the program/database state. Be as "const" as possible.


Not just for functional programmers. Prints and other I/O operations absolutely are side effects. That's not running counter to the point being made. Print in an assert and NDEBUG takes away that behavior.

You're right of course. I was thinking specifically of printing log/debug statements in the assert(..), but that usually only happens if the assert(..) fails and exits, and in that case the "no side effects" rule no longer matters.

Before preaching it to others, the writing of a homily or sermon first needs to affect the heart of the one delivering it. Such heart-work is exceedingly difficult if not impossible with AI.


Any word on how much more memory safe the implementation is? If passing a previous test suite is the criteria for success, what has changed, really? Are there previous memory safety tests that went from failing to passing?

I am very interested to know if this time and energy spent actually improved memory safety.

Other engineers facing the same challenges want to know!


If the previous impl had known memory safety issues I'd imagine they'd fix them as a matter of priority. It's hard to test for memory safety issues you don't know about.

On the rust side, the question is how much `unsafe` they used (I would hope none at all, although they don't specify).


You can look: https://github.com/LadybirdBrowser/ladybird/pull/8104/files?...

It seems like it is used mostly for FFI.


It is entirely possible a Rust port could have caught previously unknown memory safety issues. Furthermore, a Rust port that looks and feels like C++ may be peppered with unsafe calls to the point where the ROI on the port is greatly reduced.

I am not trying to dunk on the effort; quite the contrary. I am eager to hear more about the goals it originally set out to achieve.


None at all, the generated AST and bytecode are stated to be identical


> We do [cubic curve fitting] all the time in image processing, and it works very well. It would probably work well for audio as well, although it's not used -- not in the same form, anyway -- in these applications.

Is there a reason the solution that "works very well" for images isn't/can't be applied to audio?


The short answer is that our eyes and ears use very different processing mechanisms. Our eyes sense using rods and cones where the distribution of them reflects a spatial distribution of the image. Our ears instead work by performing an analogue forier transform and hearing the frequencies. If you take an image and add lots of very high frequency noise, the result will be almost indistinguishable, but if you do the same for audio it will sound like a complete mess.


AFAIK it introduces harmonic distortion


I'd love to know more about this, do you perhaps have any refs? Thanks


Not an expert in this field, just a scrub, so I can't really give you much.

There is this website that has painstakingly compares many resampling algorithms from all sorts of software:

https://src.infinitewave.ca

Try it's mirror if you can't access it: https://megapro17.github.io/src/index.html

The only one that says it is a cubic interpolation is the "Renoise 2.8.0 (cubic)" one, the spectrogram isn't very promising with all sorts of noise, intermodulation and aliasing issues. And, by switching to the 1khz tone spectrum view you can see some harmonics creeping up.

When I used to mess with trackers I would sometimes chose different interpolations and bicubic definitely still colored the sound, with sometimes enjoyable results. Obviously you don't want that as a general resampler...


Just to note that this site hasn't been updated for a while.

Much better, more modern and with automated upload analysis site would be [1] although it is designed for finding the highest fidelity resampler rather than AB comparisons.

[1] https://src.hydrogenaudio.org


We saw these is Ravenna


What, no Doom running on Voyager 2?


If the Standard has anything to say about compatibility between different language versions, I doubt many developers know those details. This is breeding ground for ODR violations, as you’re likely using compilers with different output (as they are built in different eras of the language’s lifetime) especially at higher optimization settings.

This flies in the face of modern principles like building all your C++, from source, at the same time, with the same settings.

Languages like Rust include these settings in symbol names as a hash to prevent these kinds of issues by design. Unless your whole team is a moderate-level language lawyer, you must enforce this by some other means or risk some really gnarly issues.


> Languages like Rust include these settings in symbol names as a hash to prevent these kinds of issues by design.

Historically, C++ compilers' name mangling scheme for symbols did precisely the same thing. The 2000-2008 period for gcc was particularly painful since the compiler developers really used it very frequently, to "prevent these kinds of issues by design". The only reason most C++ developers don't think about this much any more is that most C++ compilers haven't needed to change their demangling algorithm for a decade or more.


C++’s name mangling scheme handles some things like namespaces and overloading, but it does not account for other settings that can affect the ABI layer of the routine, like compile time switches or optimization level.


The name mangling scheme was changed to reflect things other than namespaces and overloading, it was modified to reflect fundamental compiler version incompatibilities (i.e. the ABI)

Optimization level should never cause link time or run time issues; if it does I'd consider that a compiler/linker bug, not an issue with the language.


Looping through inflate/deflate on rotated pixels still takes more time than updating a bit in the Exif (and the chunk’s associated CRC)


It's still negligible from the consumer standpoint.

Like, if you had millions of images you needed to rotate on a server in a batch job, then OK.

But if you're just rotating one photo, or even a hundred, that you've just taken, it's plenty fast enough.


Dithering the errors across the image would make the final result a lot more palette-able.


There are plenty of posts out there on using Knuth’s dancing links as a fast sudoku solver. Has it fallen out of fashion?


Dancing links is a very cute data-structure for a backtracking search, but there are a lot more aspects of writing a good Sudoku solver than just having a good data-structure for backtracking. Propagation (making deductions), heuristics, learning, parallelism, restarts, no-goods, ...

While 9x9 Sudoku problems are trivial to solve for more or less any program, 25x25 Sudoku instances are quite tricky and a simple and fast but naive search for a solution can easily take hours.


For generating puzzles it's really useful since it lets you determine if a randomly generated puzzle has only one possible path to solving it (exact cover problem). And it's fast so adding it to a pipeline doesn't incur much if any overhead.


Is there any property in particular of dancing links that you think helps in determining this, or is it just that a backtracking search can be used to test all cases?

For pen-and-paper puzzles like Sudoku there is usually the goal that a solution should be findable by a series of deductive steps. For 9x9 Sudoku, most deductive steps used correspond to the effects well-known propagation techniques offer[1]. With a suitable propagation level, if the puzzle is solved search-free, then one knows that both there is only one solution and that there is a deductive path to solve it.

[1]: See "Sudoku as a Constraint Problem", Helmut Simonis, https://ai.dmi.unibas.ch/_files/teaching/fs21/ai/material/ai... for some data on 9x9 Sudoku difficulty and the propagation techniques that are needed for search-free solving.


Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: