Re: [PATCH] implement pre-c++20 contracts

[Jeff Chapman via Gcc-patches]

> OK, I'll start with -alt then, thanks.

Andrew is exactly correct, contracts-jac-alt is still the current branch
we're focusing our upstreaming efforts on.

It's trailing upstream master by a fair bit at this point. I'll get a merge
pushed shortly.

Please let me know if there's anything I can do to help review along!


On Thu, Dec 3, 2020 at 12:41 PM Jason Merrill  wrote:

> On 12/3/20 12:07 PM, Andrew Sutton wrote:
> >
> >  > Attached is a new squashed revision of the patch sans ChangeLogs.
> The
> >  > current work is now being done on github:
> >  > https://github.com/lock3/gcc/tree/contracts-jac-alt
> > 
> >
> > I'm starting to review this now, sorry for the delay. Is this still
> the
> > branch you want me to consider for GCC 11?  I notice that the
> > -constexpr
> > and -mangled-config branches are newer.
> >
> >
> > I think so. Jeff can answer more authoritatively. I know we had one set
> > of changes to the design (how contracts) work aimed at improving the
> > debugging experience for violated contracts. I'm not sure if that's in
> > the jac-alt branch though.
> >
> > The -constexpr branch checks for trivially satisfied contracts (e.g.,
> > [[assert: true]]) and issues warnings. It also preemptively checks
> > preconditions against constant function arguments. It's probably worth
> > reviewing that separately.
> >
> > I'm not sure the -manged-config branch is worth considering for merging
> > at this point. It's trying to solve a problem that might not be worth
> > solving.
>
> OK, I'll start with -alt then, thanks.
>
> > Out of curiosity, are you concerned that future versions of contracts
> > might have considerably different syntax or configurability? I'd hope it
> > wouldn't, but who knows where SG21 is going :)
>
> Not particularly; I figure that most of the implementation would be
> unaffected.
>
> Jason
>
>

Re: [PATCH] implement pre-c++20 contracts

[Jason Merrill via Gcc-patches]

On 12/3/20 12:07 PM, Andrew Sutton wrote:


 > Attached is a new squashed revision of the patch sans ChangeLogs. The
 > current work is now being done on github:
 > https://github.com/lock3/gcc/tree/contracts-jac-alt


I'm starting to review this now, sorry for the delay. Is this still the
branch you want me to consider for GCC 11?  I notice that the
-constexpr
and -mangled-config branches are newer.


I think so. Jeff can answer more authoritatively. I know we had one set 
of changes to the design (how contracts) work aimed at improving the 
debugging experience for violated contracts. I'm not sure if that's in 
the jac-alt branch though.


The -constexpr branch checks for trivially satisfied contracts (e.g., 
[[assert: true]]) and issues warnings. It also preemptively checks 
preconditions against constant function arguments. It's probably worth 
reviewing that separately.


I'm not sure the -manged-config branch is worth considering for merging 
at this point. It's trying to solve a problem that might not be worth 
solving.


OK, I'll start with -alt then, thanks.

Out of curiosity, are you concerned that future versions of contracts 
might have considerably different syntax or configurability? I'd hope it 
wouldn't, but who knows where SG21 is going :)


Not particularly; I figure that most of the implementation would be 
unaffected.


Jason

Re: [PATCH] implement pre-c++20 contracts

[Andrew Sutton via Gcc-patches]

>
>
> > Attached is a new squashed revision of the patch sans ChangeLogs. The
> > current work is now being done on github:
> > https://github.com/lock3/gcc/tree/contracts-jac-alt
>
> I'm starting to review this now, sorry for the delay. Is this still the
> branch you want me to consider for GCC 11?  I notice that the -constexpr
> and -mangled-config branches are newer.


I think so. Jeff can answer more authoritatively. I know we had one set of
changes to the design (how contracts) work aimed at improving the debugging
experience for violated contracts. I'm not sure if that's in the jac-alt
branch though.

The -constexpr branch checks for trivially satisfied contracts (e.g.,
[[assert: true]]) and issues warnings. It also preemptively checks
preconditions against constant function arguments. It's probably worth
reviewing that separately.

I'm not sure the -manged-config branch is worth considering for merging at
this point. It's trying to solve a problem that might not be worth solving.

Out of curiosity, are you concerned that future versions of contracts might
have considerably different syntax or configurability? I'd hope it
wouldn't, but who knows where SG21 is going :)

Andrew

Re: [PATCH] implement pre-c++20 contracts

[Jason Merrill via Gcc-patches]

On 7/10/20 1:53 PM, Jeff Chapman wrote:

Hello again :)

Attached is a new squashed revision of the patch sans ChangeLogs. The
current work is now being done on github:
https://github.com/lock3/gcc/tree/contracts-jac-alt


I'm starting to review this now, sorry for the delay. Is this still the 
branch you want me to consider for GCC 11?  I notice that the -constexpr 
and -mangled-config branches are newer.




Please let me know if there's a better way to share revisions.


+  /* Check that assertions are null statements.  */
+  if (attribute_contract_assert_p (contract_attrs))
+if (token->type != CPP_SEMICOLON)
+  error_at (token->location, "assertions must be followed by
%<;%>");


Better I think to handle this further down where [[fallthrough]] has the
same requirement.



I'm wondering if it would be better to move [[fallthrough]] up, since
the later check is not always executed and in the case of [[assert]] we
actually need to error.

   [[fallthrough]] int x;

for instance just gets a generic 'attribute ignored' warning. I'm not
entirely happy with how we prevent assert/pre/post from appearing in
invalid locations in general which I'll try to improve. If you have
concrete suggestions please let me know.


Improvements have been made to handling contract attributes appearing in
places they don't belong. Any feedback about moving the logic dealing
with [[fallthrough]]?



Why not leave the function the user declared as the unchecked function
(just changing some linkage properties) and create a new function for
the checked wrapper?



This revision of the patch does not include changes to the
checked/unchecked function split. We're exploring an alternative rewrite
that leaves the original function declaration alone and should address
or sidestep a number of these comments.

Specifically, we're exploring generating pre and post functions with
calls to them in the correct places (upon entering a guarded function,
wrapping the return value):

   int f(int n) [[ pre: n > 0 ]] [[ post r: r < 0 ]] { return -n; }

turns into

   void __pre_f(int n) { [[ assert: n > 0 ]]; }
   int __post_f(int r) { [[ assert: r < 0 ]]; return r; }
   int f(int n) {
 __pre_f(n);
 return __post_f(-n);
   }

with whatever hints we can give to optimize this as much as possible.


This is the main change since the last submission. We now leave the
original decl intact and instead generate a pre and post function.
Naming and mangling of these pre/post functions needs addressed.

Beyond that, more cleanup has been done. There's still outstanding
cleanup work, mostly around investigating and improving performance.
Feedback on the main direction of the patch would be appreciated, and
any specific commentary or directions of investigation would be helpful.



+/* Return the source text between two locations.  */
+
+static char *
+get_source (location_t start, location_t end)


This seems like it belongs in libcpp.  It also needs to



This has been moved to input since it uses input functions, but needs
more work. Was there another comment you had that cutoff?

...snip...


+  /* We never want to accidentally instantiate templates.  */
+  if (code == TEMPLATE_DECL)
+return *tp; /* FIXME? */


This seems unlikely to have the desired effect; we should see template
instantiations as FUNCTION_DECL or VAR_DECL.  I'm also not sure what the
cp_tree_defined_p check is trying to do; surely using defined functions
and variables can also lead to runtime code?



This is an result of the transform we do for axioms when they're enabled
that you may know of a better way to do. Essentially we turn this:

   [[ assert axiom: f() > 0 ]];

into this:

   if (!(f() > 0))
 __builtin_unreachable ();

but we potentially run into issues later if f isn't (yet) defined or is
defined in a different translation unit, such as constexpr time. We're
avoiding those errors by generating a nop if there's any chance can't be
evaluated. We'd prefer something like

   __builtin_assume (f() > 0);

where the condition is simply ignored if enough information isn't
present to affect codegen. Is there a better way to handle this?

I should mention that there are likely significant further changes
around axiom coming down the pike that may tie into or replace the
"is defined" check. Specifically for expressions that _cannot_ be
defined rather than ones that are simply not yet defined.



Not much has changed yet regarding axiom, but if you have any
suggestions for handling any of the above then I'm all ears.

Re: [PATCH] implement pre-c++20 contracts

[Jeff Chapman via Gcc-patches]

Ping. Any feedback would be appreciated :)

re: https://gcc.gnu.org/pipermail/gcc-patches/2020-July/549868.html
older reply: https://gcc.gnu.org/pipermail/gcc-patches/2020-May/545339.html

On 7/10/20, Jeff Chapman  wrote:
> Hello again :)
>
> Attached is a new squashed revision of the patch sans ChangeLogs. The
> current work is now being done on github:
> https://github.com/lock3/gcc/tree/contracts-jac-alt
>
> Please let me know if there's a better way to share revisions.
>
 +  /* Check that assertions are null statements.  */
 +  if (attribute_contract_assert_p (contract_attrs))
 +if (token->type != CPP_SEMICOLON)
 +  error_at (token->location, "assertions must be followed by
 %<;%>");
>>>
>>> Better I think to handle this further down where [[fallthrough]] has the
>>> same requirement.
>>>
>>
>> I'm wondering if it would be better to move [[fallthrough]] up, since
>> the later check is not always executed and in the case of [[assert]] we
>> actually need to error.
>>
>>   [[fallthrough]] int x;
>>
>> for instance just gets a generic 'attribute ignored' warning. I'm not
>> entirely happy with how we prevent assert/pre/post from appearing in
>> invalid locations in general which I'll try to improve. If you have
>> concrete suggestions please let me know.
>
> Improvements have been made to handling contract attributes appearing in
> places they don't belong. Any feedback about moving the logic dealing
> with [[fallthrough]]?
>
>
>>> Why not leave the function the user declared as the unchecked function
>>> (just changing some linkage properties) and create a new function for
>>> the checked wrapper?
>>>
>>
>> This revision of the patch does not include changes to the
>> checked/unchecked function split. We're exploring an alternative rewrite
>> that leaves the original function declaration alone and should address
>> or sidestep a number of these comments.
>>
>> Specifically, we're exploring generating pre and post functions with
>> calls to them in the correct places (upon entering a guarded function,
>> wrapping the return value):
>>
>>   int f(int n) [[ pre: n > 0 ]] [[ post r: r < 0 ]] { return -n; }
>>
>> turns into
>>
>>   void __pre_f(int n) { [[ assert: n > 0 ]]; }
>>   int __post_f(int r) { [[ assert: r < 0 ]]; return r; }
>>   int f(int n) {
>> __pre_f(n);
>> return __post_f(-n);
>>   }
>>
>> with whatever hints we can give to optimize this as much as possible.
>
> This is the main change since the last submission. We now leave the
> original decl intact and instead generate a pre and post function.
> Naming and mangling of these pre/post functions needs addressed.
>
> Beyond that, more cleanup has been done. There's still outstanding
> cleanup work, mostly around investigating and improving performance.
> Feedback on the main direction of the patch would be appreciated, and
> any specific commentary or directions of investigation would be helpful.
>
>
 +/* Return the source text between two locations.  */
 +
 +static char *
 +get_source (location_t start, location_t end)
>>>
>>> This seems like it belongs in libcpp.  It also needs to
>>>
>>
>> This has been moved to input since it uses input functions, but needs
>> more work. Was there another comment you had that cutoff?
>>
>> ...snip...
>>
 +  /* We never want to accidentally instantiate templates.  */
 +  if (code == TEMPLATE_DECL)
 +return *tp; /* FIXME? */
>>>
>>> This seems unlikely to have the desired effect; we should see template
>>> instantiations as FUNCTION_DECL or VAR_DECL.  I'm also not sure what the
>>> cp_tree_defined_p check is trying to do; surely using defined functions
>>> and variables can also lead to runtime code?
>>>
>>
>> This is an result of the transform we do for axioms when they're enabled
>> that you may know of a better way to do. Essentially we turn this:
>>
>>   [[ assert axiom: f() > 0 ]];
>>
>> into this:
>>
>>   if (!(f() > 0))
>> __builtin_unreachable ();
>>
>> but we potentially run into issues later if f isn't (yet) defined or is
>> defined in a different translation unit, such as constexpr time. We're
>> avoiding those errors by generating a nop if there's any chance can't be
>> evaluated. We'd prefer something like
>>
>>   __builtin_assume (f() > 0);
>>
>> where the condition is simply ignored if enough information isn't
>> present to affect codegen. Is there a better way to handle this?
>>
>> I should mention that there are likely significant further changes
>> around axiom coming down the pike that may tie into or replace the
>> "is defined" check. Specifically for expressions that _cannot_ be
>> defined rather than ones that are simply not yet defined.
>>
>
> Not much has changed yet regarding axiom, but if you have any
> suggestions for handling any of the above then I'm all ears.
>

Re: [PATCH] implement pre-c++20 contracts

[Jeff Chapman via Gcc-patches]

Hello again :)

Attached is a new squashed revision of the patch sans ChangeLogs. The
current work is now being done on github:
https://github.com/lock3/gcc/tree/contracts-jac-alt

Please let me know if there's a better way to share revisions.

>>> +  /* Check that assertions are null statements.  */
>>> +  if (attribute_contract_assert_p (contract_attrs))
>>> +if (token->type != CPP_SEMICOLON)
>>> +  error_at (token->location, "assertions must be followed by
>>> %<;%>");
>>
>> Better I think to handle this further down where [[fallthrough]] has the
>> same requirement.
>>
>
> I'm wondering if it would be better to move [[fallthrough]] up, since
> the later check is not always executed and in the case of [[assert]] we
> actually need to error.
>
>   [[fallthrough]] int x;
>
> for instance just gets a generic 'attribute ignored' warning. I'm not
> entirely happy with how we prevent assert/pre/post from appearing in
> invalid locations in general which I'll try to improve. If you have
> concrete suggestions please let me know.

Improvements have been made to handling contract attributes appearing in
places they don't belong. Any feedback about moving the logic dealing
with [[fallthrough]]?


>> Why not leave the function the user declared as the unchecked function
>> (just changing some linkage properties) and create a new function for
>> the checked wrapper?
>>
>
> This revision of the patch does not include changes to the
> checked/unchecked function split. We're exploring an alternative rewrite
> that leaves the original function declaration alone and should address
> or sidestep a number of these comments.
>
> Specifically, we're exploring generating pre and post functions with
> calls to them in the correct places (upon entering a guarded function,
> wrapping the return value):
>
>   int f(int n) [[ pre: n > 0 ]] [[ post r: r < 0 ]] { return -n; }
>
> turns into
>
>   void __pre_f(int n) { [[ assert: n > 0 ]]; }
>   int __post_f(int r) { [[ assert: r < 0 ]]; return r; }
>   int f(int n) {
> __pre_f(n);
> return __post_f(-n);
>   }
>
> with whatever hints we can give to optimize this as much as possible.

This is the main change since the last submission. We now leave the
original decl intact and instead generate a pre and post function.
Naming and mangling of these pre/post functions needs addressed.

Beyond that, more cleanup has been done. There's still outstanding
cleanup work, mostly around investigating and improving performance.
Feedback on the main direction of the patch would be appreciated, and
any specific commentary or directions of investigation would be helpful.


>>> +/* Return the source text between two locations.  */
>>> +
>>> +static char *
>>> +get_source (location_t start, location_t end)
>>
>> This seems like it belongs in libcpp.  It also needs to
>>
>
> This has been moved to input since it uses input functions, but needs
> more work. Was there another comment you had that cutoff?
>
> ...snip...
>
>>> +  /* We never want to accidentally instantiate templates.  */
>>> +  if (code == TEMPLATE_DECL)
>>> +return *tp; /* FIXME? */
>>
>> This seems unlikely to have the desired effect; we should see template
>> instantiations as FUNCTION_DECL or VAR_DECL.  I'm also not sure what the
>> cp_tree_defined_p check is trying to do; surely using defined functions
>> and variables can also lead to runtime code?
>>
>
> This is an result of the transform we do for axioms when they're enabled
> that you may know of a better way to do. Essentially we turn this:
>
>   [[ assert axiom: f() > 0 ]];
>
> into this:
>
>   if (!(f() > 0))
> __builtin_unreachable ();
>
> but we potentially run into issues later if f isn't (yet) defined or is
> defined in a different translation unit, such as constexpr time. We're
> avoiding those errors by generating a nop if there's any chance can't be
> evaluated. We'd prefer something like
>
>   __builtin_assume (f() > 0);
>
> where the condition is simply ignored if enough information isn't
> present to affect codegen. Is there a better way to handle this?
>
> I should mention that there are likely significant further changes
> around axiom coming down the pike that may tie into or replace the
> "is defined" check. Specifically for expressions that _cannot_ be
> defined rather than ones that are simply not yet defined.
>

Not much has changed yet regarding axiom, but if you have any
suggestions for handling any of the above then I'm all ears.


0001-Implement-pre-c-20-contracts.patch.gz
Description: GNU Zip compressed data

Re: [PATCH] implement pre-c++20 contracts

[Jeff Chapman via Gcc-patches]

Hello,

On 12/10/19, Jason Merrill wrote:
> On 11/13/19, Jeff Chapman wrote:
>> Attached is a patch that implements pre-c++20 contracts. This comes
>> from a long running development branch which included ChangeLog entries
>> as we went, which are included in the patch itself. The repo and
>> initial wiki are located here:
>> https://gitlab.com/lock3/gcc-new/wikis/GCC-with-Contracts
>
> Thanks.  I've mostly been referring to the repo rather than the attached
> patch.  Below are a bunch of comments about the implementation, in no
> particular order.
>

I've attached a new squashed revision of the patch, and you can see the
changes I've made from your input on the contracts-jac-prep branch:
https://gitlab.com/lock3/gcc-new/-/tree/contracts-jac-prep . If there's
an easier format for you to review that I can produce please let me
know.

I'll address a few things inline below. Everything else should either be
handled or explained by Andrew's last email. If anything needs further
addressing or something hasn't been brought up yet please let me know :)


>> +handle_OPT_fcontract_build_level_ (const char *arg)
>> +{
>> +  if (contracts_p1332_default || contracts_p1332_review ||
>> contracts_p1429)
>> +{
>> +  error ("-fcontract-build-level= cannot be mixed with p1332/p1429");
>
> Hmm, P1429 includes the notion of build level, it's just checked after
> explicit semantics.  In general, P1429 seems like a compatible extension
> of the semantics previously in the working paper.
>
> P1332 could also be treated as compatible if we consider the P0542 build
> level to affect the default role as specified in P1429.  P1680 seems to
> suggest that this is what you had in mind.
>

These could possibly be made compatible, but in some cases the flags are
changing the same entries in the table. That would require deciding
whether flag ordering matters or whether a certain flags can't change
values set by other flags.

I'm not sure it's a worthwhile change. While it increases the valid
space of command line invocations, it doesn't actually increase the the
result space. I'd prefer an eventual solution that removed flags
entirely instead.


>> +  /* Check that assertions are null statements.  */
>> +  if (attribute_contract_assert_p (contract_attrs))
>> +if (token->type != CPP_SEMICOLON)
>> +  error_at (token->location, "assertions must be followed by
>> %<;%>");
>
> Better I think to handle this further down where [[fallthrough]] has the
> same requirement.
>

I'm wondering if it would be better to move [[fallthrough]] up, since
the later check is not always executed and in the case of [[assert]] we
actually need to error.

  [[fallthrough]] int x;

for instance just gets a generic 'attribute ignored' warning. I'm not
entirely happy with how we prevent assert/pre/post from appearing in
invalid locations in general which I'll try to improve. If you have
concrete suggestions please let me know.


> Why not leave the function the user declared as the unchecked function
> (just changing some linkage properties) and create a new function for
> the checked wrapper?
>

This revision of the patch does not include changes to the
checked/unchecked function split. We're exploring an alternative rewrite
that leaves the original function declaration alone and should address
or sidestep a number of these comments.

Specifically, we're exploring generating pre and post functions with
calls to them in the correct places (upon entering a guarded function,
wrapping the return value):

  int f(int n) [[ pre: n > 0 ]] [[ post r: r < 0 ]] { return -n; }

turns into

  void __pre_f(int n) { [[ assert: n > 0 ]]; }
  int __post_f(int r) { [[ assert: r < 0 ]]; return r; }
  int f(int n) {
__pre_f(n);
return __post_f(-n);
  }

with whatever hints we can give to optimize this as much as possible.


>> +/* Return the source text between two locations.  */
>> +
>> +static char *
>> +get_source (location_t start, location_t end)
>
> This seems like it belongs in libcpp.  It also needs to
>

This has been moved to input since it uses input functions, but needs
more work. Was there another comment you had that cutoff?


>> +  tree level_str = build_string_literal (strlen (level) + 1, level);
>> +  tree role_str = build_string_literal (strlen (role) + 1, role);
>
> Maybe combine these into a single string argument?
>

These are used separately in std::contract_violation and to my
understanding building them separately will collapse duplicate levels
and roles instead of duplicating them unless they both match -- is that
correct?


>> +  /* We never want to accidentally instantiate templates.  */
>> +  if (code == TEMPLATE_DECL)
>> +return *tp; /* FIXME? */
>
> This seems unlikely to have the desired effect; we should see template
> instantiations as FUNCTION_DECL or VAR_DECL.  I'm also not sure what the
> cp_tree_defined_p check is trying to do; surely using defined functions
> and variables can also lead to runtime code?
>

This is 

Re: [PATCH] implement pre-c++20 contracts

[Andrew Sutton via Gcc-patches]

Hi Jason,

Sorry I haven't been able to get back to this. I've been swamped with
other work, and we haven't had the resources to properly address this.
Jeff Chapman will be working on cleaning this up for when master/trunk
re-opens.


> I find the proposed always_continue semantics kind of nonsensical, as
> somewhat evidenced by the contortions the implementation gets into with
> marking the violation handler as pure.  The trick of assigning the
> result to a local variable won't work with optimization.

We tend to agree and think it's practically non-implementable. IIRC,
later contracts proposals steered away from adding this as a concrete
semantic. I suspect killing this would be fine.

> > +/* Definitions for C++ contract levels
> > +   Copyright (C) 1987-2018 Free Software Foundation, Inc.
>
> Should just be 2019 for a new file.

Probably 2020 now :)

> > +   Contributed by Michael Tiemann (tiem...@cygnus.com)
>
> This seems inaccurate.  :)

Indeed :)


> This change to member function redeclaration seems undesirable; your
> rationale in P1680 is "for generality", but member functions are already
> stricter about redeclaration than non-member functions; I don't see a
> reason to relax this rule just for contracts.  With member functions,
> there *is* always a canonical first declaration, and people expect the
> class definition to have its complete interface, of which contracts are
> a part.

There was a proposal that gave more motivation than just "for
generality". Apparently, I was a co-author -- I think I just helped
write the wording.

http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1320r2.html


> For non-member functions, we still need to complain if contracts are
> added after we've seen an ODR-use of the function, just like with
> explicit specializations.

We could do that, but it doesn't seem necessary with this model.
Whether the contracts have been seen or not doesn't affect which
function is called.


> > +  /* TODO is there any way to relax this requirement?  */
> > +  if (DECL_VIRTUAL_P (olddecl) && !TYPE_BEING_DEFINED (DECL_CONTEXT 
> > (olddecl)))
>
> Relatedly, I don't think we want to relax this requirement.

Probably. Virtual functions and contracts have complex interactions.
There are going to be more EWG papers about this, I'm sure.


> > +  /* FIXME Is this right for friends? Can a friend refer to a static 
> > member?
> > + Can a friend's contracts refer to our privates? Turning them into
> > + [[assert]]s inside the body of the friend itself certainly lets them 
> > do
> > + so. */
>
> P0542 says contracts are limited to the accessibility of the function
> itself, so a friend cannot refer to private members.

That will almost certainly be changed. This was the proposal:

http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p1289r0.pdf

There was near-unanimous support for removing that (19 for, 1 against).

Andrew

Re: [PATCH] implement pre-c++20 contracts

[Jason Merrill]

On 12/10/19 12:58 AM, Jason Merrill wrote:

On 11/13/19 2:07 PM, Jeff Chapman wrote:

Attached is a patch that implements pre-c++20 contracts. This comes
from a long running development branch which included ChangeLog entries
as we went, which are included in the patch itself. The repo and
initial wiki are located here:
https://gitlab.com/lock3/gcc-new/wikis/GCC-with-Contracts


Thanks.  I've mostly been referring to the repo rather than the attached 
patch.  Below are a bunch of comments about the implementation, in no 
particular order.



We've previously circulated a paper (P1680) which documents our
implementation experience which largely covers new flags and features.
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1680r0.pdf

That paper documents our changes in depth, barring the recently added
-fcontracts flag which is a global opt-in for contracts functionality.
As an overview of what we've done is included below for convenience.

The following switches have been added:

-fcontracts
   enable contract features in general

Flags from the old Working Draft:

-fcontract-build-level=[off|default|audit]
   specify max contract level to generate runtime checks for

-fcontract-continuation-mode=[on|off]
   toggle whether execution resumes after contract failure

Flags from P1290:

-fcontract-assumption-mode=[on|off]
   enable treating axiom level contracts as compile time assumptions
   (default on)

Flags from P1429:

-fcontract-mode=[on|off]
   enable or disable all contract facilities (default on)

-fcontract-semantic=:
   specify the concrete semantics for a level

Flags from P1332:

-fcontract-role=:
   specify semantics for all levels in a role (default, review, or a
 custom role)
   (ex: opt:assume,assume,assume)

Additional flags:

-fcontract-strict-declarations=[on|off]
   toggle warnings on generalized redecl of member functions
 without contracts (default off)


One assert contract may be present on any block scope empty statement:
   [[ assert contract-mode_opt : conditional-expression ]]

Function declarations have an optional, ordered, list of pre and post
contracts:
   [[ pre contract-mode_opt : conditional-expression ]]
   [[ post contract-mode_opt identifier_opt : conditional-expression ]]


The grammar for the contract-mode_opt portion which configures the
concrete semantics of the contracts is:

contract-mode
   contract-semantic
   contract-level_opt contract-role_opt

contract-semantic
   ignore
   check_never_continue
   check_maybe_continue
   check_always_continue
   assume

contract-level
   default
   audit
   axiom

contract-role
   %default
   %identifier


Contracts are configured via concrete semantics or by an optional
level and role which map to one of the concrete semantics:

   ignore – The contract does not affect the behavior of the program.
   assume – The condition may be used for optimization.
   never_continue – The program terminates if the contract is
    not satisfied.
   maybe_continue – A user-defined violation handler may terminate the
    program.
   always_continue – The program continues even if the contract is not
 satisfied.


I find the proposed always_continue semantics kind of nonsensical, as 
somewhat evidenced by the contortions the implementation gets into with 
marking the violation handler as pure.  The trick of assigning the 
result to a local variable won't work with optimization.


It also depends on the definition of a function that can be overridden 
to in fact never return.  This seems pretty fatal to it ever getting 
into the standard.


It's also unclear to me why anyone would want the described semantics. 
Why would you want a contract check that can be optimized away due to 
later undefined behavior?  The 0.2 use case from P1332 seems better 
suited to maybe_continue, because with always_continue such a check will 
have false negatives, leading to an unpleasant surprise when switching 
to never_continue.


I'd prefer to treat always_continue as equivalent to maybe_continue. 
Perhaps with ECF_NOTHROW|ECF_LEAF|ECF_NOVOPS to indicate that it doesn't 
clobber anything the caller can see, but that's risky if the handler is 
in fact defined in the same TU with anything that uses contracts.



+  if (strcmp (name, "check_always_continue") == 0
+  || strcmp (name, "always") == 0
+  || strcmp (name, "continue") == 0)


Accordingly, "continue" should mean maybe_continue.


+/* Definitions for C++ contract levels
+   Copyright (C) 1987-2018 Free Software Foundation, Inc.


Should just be 2019 for a new file.


+   Contributed by Michael Tiemann (tiem...@cygnus.com)


This seems inaccurate.  :)

It would also be good to have a reference to P1332 in this header.


+/* Assertion role info.
+
+   FIXME: Why is this prefixed cpp?  */
+struct cpp_contract_role


There seems to be no reason for it, since the struct definition is 
followed by a typedef; let's remove the prefix.

Re: [PATCH] implement pre-c++20 contracts

[Jason Merrill]

On 11/13/19 2:07 PM, Jeff Chapman wrote:

Attached is a patch that implements pre-c++20 contracts. This comes
from a long running development branch which included ChangeLog entries
as we went, which are included in the patch itself. The repo and
initial wiki are located here:
https://gitlab.com/lock3/gcc-new/wikis/GCC-with-Contracts


Thanks.  I've mostly been referring to the repo rather than the attached 
patch.  Below are a bunch of comments about the implementation, in no 
particular order.



We've previously circulated a paper (P1680) which documents our
implementation experience which largely covers new flags and features.
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1680r0.pdf

That paper documents our changes in depth, barring the recently added
-fcontracts flag which is a global opt-in for contracts functionality.
As an overview of what we've done is included below for convenience.

The following switches have been added:

-fcontracts
   enable contract features in general

Flags from the old Working Draft:

-fcontract-build-level=[off|default|audit]
   specify max contract level to generate runtime checks for

-fcontract-continuation-mode=[on|off]
   toggle whether execution resumes after contract failure

Flags from P1290:

-fcontract-assumption-mode=[on|off]
   enable treating axiom level contracts as compile time assumptions
   (default on)

Flags from P1429:

-fcontract-mode=[on|off]
   enable or disable all contract facilities (default on)

-fcontract-semantic=:
   specify the concrete semantics for a level

Flags from P1332:

-fcontract-role=:
   specify semantics for all levels in a role (default, review, or a
 custom role)
   (ex: opt:assume,assume,assume)

Additional flags:

-fcontract-strict-declarations=[on|off]
   toggle warnings on generalized redecl of member functions
 without contracts (default off)


One assert contract may be present on any block scope empty statement:
   [[ assert contract-mode_opt : conditional-expression ]]

Function declarations have an optional, ordered, list of pre and post
contracts:
   [[ pre contract-mode_opt : conditional-expression ]]
   [[ post contract-mode_opt identifier_opt : conditional-expression ]]


The grammar for the contract-mode_opt portion which configures the
concrete semantics of the contracts is:

contract-mode
   contract-semantic
   contract-level_opt contract-role_opt

contract-semantic
   ignore
   check_never_continue
   check_maybe_continue
   check_always_continue
   assume

contract-level
   default
   audit
   axiom

contract-role
   %default
   %identifier


Contracts are configured via concrete semantics or by an optional
level and role which map to one of the concrete semantics:

   ignore – The contract does not affect the behavior of the program.
   assume – The condition may be used for optimization.
   never_continue – The program terminates if the contract is
not satisfied.
   maybe_continue – A user-defined violation handler may terminate the
program.
   always_continue – The program continues even if the contract is not
 satisfied.


I find the proposed always_continue semantics kind of nonsensical, as 
somewhat evidenced by the contortions the implementation gets into with 
marking the violation handler as pure.  The trick of assigning the 
result to a local variable won't work with optimization.


It also depends on the definition of a function that can be overridden 
to in fact never return.  This seems pretty fatal to it ever getting 
into the standard.


It's also unclear to me why anyone would want the described semantics. 
Why would you want a contract check that can be optimized away due to 
later undefined behavior?  The 0.2 use case from P1332 seems better 
suited to maybe_continue, because with always_continue such a check will 
have false negatives, leading to an unpleasant surprise when switching 
to never_continue.


I'd prefer to treat always_continue as equivalent to maybe_continue. 
Perhaps with ECF_NOTHROW|ECF_LEAF|ECF_NOVOPS to indicate that it doesn't 
clobber anything the caller can see, but that's risky if the handler is 
in fact defined in the same TU with anything that uses contracts.



+  if (strcmp (name, "check_always_continue") == 0
+  || strcmp (name, "always") == 0
+  || strcmp (name, "continue") == 0)


Accordingly, "continue" should mean maybe_continue.


+/* Definitions for C++ contract levels
+   Copyright (C) 1987-2018 Free Software Foundation, Inc.


Should just be 2019 for a new file.


+   Contributed by Michael Tiemann (tiem...@cygnus.com)


This seems inaccurate.  :)

It would also be good to have a reference to P1332 in this header.


+/* Assertion role info.
+
+   FIXME: Why is this prefixed cpp?  */
+struct cpp_contract_role


There seems to be no reason for it, since the struct definition is 
followed by a typedef; let's remove the prefix.


Any cpp_ prefixes we want to keep should