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
