argotorg / argotorg/solidity

Enable plain `assert()` in non-release builds in CI

Open
#12,641 6 comments 0 reactions 0 assignees View on GitHub
build system :building_construction: low effort medium impact must have testing :hammer:
Dominant language
C++
Stars
25.7k
Forks
6.2k
Avg merge
1d 11h
Merged PRs (30d)
21

Description

Apparently we have the [`assert`](https://en.cppreference.com/w/cpp/error/assert) macro disabled in CI, even in non-release builds - at least on Windows (see https://github.com/ethereum/solidity/pull/12634#discussion_r800191742). While this macro is generally discouraged in our project in favor of the `solAssert()` family of macros, we still do have them in some places and I think that they should not be disabled. I wouldn't disable them even if we did not have any - some of the libraries we depend on might also be using `assert` for example.

We do not define `NDEBUG` so I'm not really sure why they're disabled. Is that maybe because we default to `RelWithDebInfo` mode rather than `Debug`? The task here is to investigate, enable the macro and verify that it does actually work in non-release builds on all platforms.

Make sure the STL assertions work too. A good example is one that is triggered by dereferencing an empty `optional`. Verify by reverting #15909 and checking whether that assertion fails.

Contributor guide

Open the contributing guide

Research direction

Start with the CI build configuration and compare non-release builds across platforms, especially the RelWithDebInfo mode and whether NDEBUG is defined. Verify that plain assert and STL assertions are active by checking the empty-optional case, then revert #15909 to confirm the expected failure. Done means assertions work in non-release CI builds on every platform without affecting release builds.

Written by the indexing model from the issue text.

Assessment

Tech stack
cmake, cpp
Domain
build-system, ci-cd, testing
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.