Skip to content

[BUG] Generalize _string-literal_ of contract like C++26's static_assert #751

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
JohelEGP opened this issue Oct 14, 2023 · 3 comments
Closed
Labels
bug Something isn't working

Comments

@JohelEGP
Copy link
Contributor

Title: Generalize string-literal of contract like C++26's static_assert.

Description:

Custom text output after a contract failure isn't part of the Contracts' MVP.
If Cpp2's contracts feature is going to have this anyways,
we could craft better diagnostics messages
if its second argument behaved like that of static_assert in C++26.

Minimal reproducer (https://cpp2.godbolt.org/z/GYqdx3WTE):

// After P2361R6 and P2741R3:
static_assert(std::is_same_v<int, decltype(0)>, std::string("0") + " was not `int`.");
main: () = {
  [[assert: std::is_same_v<int, decltype(0)>, std::string("0") + " was not `int`."]]
}
Commands:
cppfront main.cpp2
clang++18 -std=c++26 -stdlib=libc++ -lc++abi -pedantic-errors -Wall -Wextra -Wconversion -Werror=unused-result -I . main.cpp

Expected result: Program returned: 0.

Actual result and error: main.cpp2(4,47): error: expected contract message string (at 'std').

See also:

1697303665
1697303677
1697303684

@JohelEGP
Copy link
Contributor Author

I mentioned at #831 (comment) that I had written something like
require(v.empty() || v.front() == 42, "`(v.front())$` is not 42.");.
require is a contract-like interface (which is actually hooked into evaluating contracts since commit 580b83a).
My expectation was for the condition v.empty() to guard the accesses to v.front().
Unfortunately, since require is just a function, both arguments are unconditionally evaluated.
Thus, the use of v.front() in the string literal is always executed, resulting in UB (but see note 1).

Now, since commit 2893e64, contracts are conditionally evaluated.
Still, since this issue is not fixed, I would need to manually use an if statement and t.violation.
With this issue fixed, I could instead write
assert<t>(v.empty() || v.front() == 42, "`(v.front())$` is not 42.");.
That is correct by construction.

Although what we really want is lazy arguments/parameters.

[ Note 1:
Since in Cpp2 indexing is checked by default, I really should have written v[0].
I have had to use v.end(), and it's unfortunate that there's no equivalent, easily spelled safe alternative.
Although std::chrono does have something like v[last] (https://eel.is/c++draft/time.cal.wdlast#overview-example-1):

constexpr auto wdl = Sunday[last];      // wdl is the last Sunday of an as yet unspecified month
static_assert(wdl.weekday() == Sunday);

Poor code takes built-in [] being commutative as an excuse to make last[v] work: https://cpp2.godbolt.org/z/9sKbn9W3e.
v[last] can't work generically since operator[] has to be implemented by the left type.
-- end note ]

@gregmarr
Copy link
Contributor

Still, since this issue is not fixed, I would need to manually use an if statement and t.violation.

Unless/until we have the ability to have an argument that is not evaluated, you could just protect the v.front() in the string as well.

require(v.empty() || v.front() == 42, "`(!v.empty() ? v.front() : 0)$` is not 42.");.

@hsutter
Copy link
Owner

hsutter commented Dec 20, 2023

Fixed, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants