contracts module #6789
contracts module #6789
Conversation
- Rename test files for better clarity: * simple_contracts_* → fallback_contracts_* * native_contracts_* → declaration_contracts_* - Add comprehensive declaration-based contract failure tests: * declaration_contracts_fail_pre.cpp * declaration_contracts_fail_post.cpp * declaration_contracts_fail_contract_assert.cpp - Remove redundant test files (contract_fail.cpp, simple_contracts_*) - Update CMakeLists.txt with proper test organization: * Declaration tests: C++26 syntax (__cpp_contracts mode) * Fallback tests: Simple syntax (fallback mode) * Disabled tests: No-op behavior testing - Set appropriate WILL_FAIL properties for all failure tests Test structure now clearly separates native C++26 contract testing from HPX_ASSERT fallback behavior testing, providing comprehensive coverage of all contract modes.
…er native or fallback mode
|
Can one of the admins verify this patch? |
|
@AlexPapadakis could you please take care of the clang-format, cmake-format, and inspect errors reported? |
|
@hkaiser yes absolutely, I will spend time on it in the Weekend |
|
|
||
| // Contract implementation: automatically selects native C++26 contracts | ||
| // or provides appropriate fallback behavior based on compiler capabilities | ||
| #ifdef HPX_HAVE_CONTRACTS |
There was a problem hiding this comment.
I think we could simplify by always using contracts, and choosing between native or not based on the HPX_HAVE_NATIVE_CONTRACTS flag
There was a problem hiding this comment.
I was also troubled by that.
The purpose of these two is clear:
HPX_HAVE_NATIVE_CONTRACTS--> Feature flag for compiler support of contracts.HPX_WITH_ASSERTS_AS_CONTRACT_ASSERTS--> Redirects HPX_ASSERT to the C++ contracts mechanism.
For the third one:
HPX_WITH_CONTRACTS--> Disables any contract-related HPX feature.
I think I agree that it would be simpler to omit this flag. For example, I cannot think of a case where we would want to completely ignore allHPX_CONTRACT_ASSERTs, in the same way that we don't need to disableHPX_ASSERTs (or deviate from the normal release/debug build assert semantics)
Another thing, the current name might be misleading, as it does not generally enable/disable C++ contracts, just its HPX integration.
There was a problem hiding this comment.
So you suggest to remove the cmake option HPX_WITH_CONTRACTS and then:
a) always use contracts when supported (or the normal asserts when they are not) and let the user change the contacts mode semantic to ignore.
or
b) follow the asserts module logic and disable them in non Debug builds
@Pansysk75 ?
|
|
||
| #ifdef HPX_HAVE_ASSERTS_AS_CONTRACT_ASSERTS | ||
| // Override HPX_ASSERT to use contract assertions | ||
| #define HPX_ASSERT(x) HPX_CONTRACT_ASSERT(x) |
There was a problem hiding this comment.
Will this change the behavior of only the files that include contracts.hpp?
If so, would we need to include this in every file we would like to use contracts?
Also, because of transitive includes I think it would be unclear if a file will be using contracts or not.
Unless this is automatically included in everything, in which case this is perfect.
There was a problem hiding this comment.
@Pansysk75 do you think we should then move the HPX_WITH_ASSERTS_AS_CONTRACT_ASSERTS to the asserts module? or should someone include the contracts header in each file
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
5 participants
Fixes #
Proposed Changes
-added contracts module
-with fallback to asserts when contracts are not supported
-and option to enchance asserts with contract asserts
Any background context you want to provide?
more on the GSoC report here:
https://alexpapadakis.github.io/hpx_contracts/
Checklist
Not all points below apply to all pull requests.