Skip to content

Conversation

@magv
Copy link
Contributor

@magv magv commented Jun 15, 2025

This helps the compiler identify which code paths are taken rarely, and move them out of the way from hot paths. I'd expect a marginal (below 1%) performance improvement, if there was a benchmark suite. The NORETURN macro currently expands into __attribute__((noreturn)), which GCC and clang understand, or nothing, if the configure script figured out that the compiler doesn't know the syntax. In principle, C11 and C++11 both support a noreturn attribute (with different syntax), but I wasn't sure what is the minimum language version requirement.

There's the issue of the Crash() function that crashes or not depending on the flags. I did not mark it as noreturn. I think that one deserves a separate consideration (i.e. why is it used in sort.c?).

@coveralls
Copy link

coveralls commented Jun 15, 2025

Coverage Status

coverage: 58.027% (+0.1%) from 57.893%
when pulling 75b5da9 on magv:master
into eb24708 on form-dev:master.

@jodavies
Copy link
Collaborator

I don't measure any performance difference here indeed, but that doesn't mean we shouldn't commit "performance best practice".

I would say that Crash should not be called directly, and only via Terminate. There are only the two calls in sort.c to replace: these are both fatal errors.

@jodavies
Copy link
Collaborator

jodavies commented Aug 7, 2025

@magv can you fix the conflicts since the merge of #674 ?

magv added 2 commits January 23, 2026 15:45
This helps the compiler identify which code paths are taken
rarely, and move them out of the way from hot paths, if not
already.
@jodavies jodavies merged commit d776835 into form-dev:master Jan 23, 2026
84 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants