Contracts p2900 contract violation #8

NinaRanns · 2024-10-10T09:21:30Z

Thanks for taking the time to contribute to GCC! Please be advised that if you are
viewing this on github.com, that the mirror there is unofficial and unmonitored.
The GCC community does not use github.com for their contributions. Instead, we use
a mailing list (gcc-patches@gcc.gnu.org) for code submissions, code reviews, and
bug reports. Please send patches there instead.

Contract fix warning

iains

some initial comments ..

iains · 2024-10-10T12:24:53Z

gcc/cp/contracts.cc

@@ -1305,6 +1305,11 @@ match_deferred_contracts (tree decl)
 static GTY(()) hash_map<tree, tree> *decl_pre_fn;
 static GTY(()) hash_map<tree, tree> *decl_post_fn;

+/* tree that holds the pseude source location type */
+static GTY(()) tree pseudo_source_location_impl_type;


(not a big deal but) I'm not entirely sure why Andrew chose to label things "pseudo" - to me that indicates it's not necessarily the same as a "real" one.. We could just s/pseudo/constracts/ if the motivation is to avoid name clashes.

left Andrew's code as is, replaced new uses of pseudo with contracts

iains · 2024-10-10T12:35:25Z

gcc/cp/contracts.cc

+    case PRECONDITION_STMT:	return CAK_PRE;
+    case POSTCONDITION_STMT:	return CAK_POST;
+    default:
+      error ("Invalid contract kind");


is this intended to be a user-facing error? (if so, then it probably should be error_at and use the source location that triggered this action.

modified to gcc_unreachable

iains · 2024-10-10T12:35:52Z

gcc/cp/contracts.cc

+  contract_semantic semantic = get_contract_semantic (contract);
+
+  if (checked_contract_p(semantic))
+    {


you don't need these braces (and in other similar cases)

removed the ones i could spot

iains · 2024-10-10T12:39:09Z

gcc/cp/contracts.cc

+
+  if (flag_contracts_nonattr)
+    {
+      id_exp = get_identifier ("contracts");


We should consider putting this in experimental/contracts until it's finalised (but we can defer until we want to upstream)

iains · 2024-10-10T12:40:26Z

gcc/cp/contracts.h

@@ -75,6 +76,27 @@ struct contract_role
  contract_semantic axiom_semantic;
 };

+/* P2900 contract clasification */


We should have a comment here that these need to be kept in sync with libstdc++/src/std{/experimental}/contracts.

iains · 2024-10-10T12:46:21Z

gcc/cp/contracts.cc

-  return "default";
+
+  expanded_location loc = expand_location (EXPR_LOCATION (contract));
+  const char *function = fndecl_name (DECL_ORIGIN (current_function_decl));


I guess we are using DECL_ORIGIN() because we expect this to be called for helper functions? (this does use D_A_O).

This is a direct copy paste from the previous way we got function names, I have not given it much thought. For the wrapper, it has the same problem that it prints out the wrapper name. At the moment, we have no way of recognising a wrapper function and getting to the name of the original function. This needs fixing. I've added an issue to the github

iains · 2024-10-10T12:47:45Z

gcc/cp/contracts.cc

@@ -1692,8 +1844,30 @@ get_pseudo_contract_violation_type ()

 /* Return a VAR_DECL to pass to handle_contract_violation.  */

+static const char *


the comment above now seems to be in the wrong place (or is no longer correct)

iains · 2024-10-10T12:49:00Z

gcc/cp/contracts.cc

 static tree
-build_contract_violation (tree contract, contract_continuation cmode)
+build_contract_violation_cpp20 (tree contract, contract_continuation cmode)


all the new static functions should have at least a brief comment before to say what they do/are for.

iains · 2024-10-10T12:52:03Z

gcc/testsuite/g++.dg/contracts/cpp26/contract-assert-run.C

@@ -10,7 +10,7 @@
 // { dg-do run }
 // { dg-options "-std=c++2a -fcontracts -fcontracts-nonattr -fcontract-continuation-mode=on" }
 #include <iostream>
-#include <experimental/contract>
+#include "../../../../../libstdc++-v3/include/std/contracts"


that does not look right;
we need to arrange to install the header in either the eventual position - or (with conditionalised content) keep it in <experimental/contracts>. We can add a __cpp_xxxxx to control what the include does - is there one defined for P2900? .. it looks like there might be several from the code below?

iains · 2024-10-10T12:55:33Z

libstdc++-v3/configure

@@ -893,6 +893,7 @@ infodir
 docdir
 oldincludedir
 includedir
+runstatedir


this is indicative of using the wrong version of autoreconf (some of the distro ones are patched - we need to use exactly what the GCC requirements are - or we end up with these spurious changes)

iains

a few more comments

iains · 2024-10-10T13:30:44Z

gcc/cp/contracts.cc

+			     fields, NULL_TREE);
+      CLASSTYPE_AS_BASE (pseudo_source_location_impl_type)
+	= pseudo_source_location_impl_type;
+      DECL_CONTEXT (TYPE_NAME (pseudo_source_location_impl_type))


if this is private to the contracts impl, I wonder why we don't make it anonymous... (maybe there's a reason)?
likewise for the other cases below

set TREE_PUBLIC to false for the new types

iains · 2024-10-10T13:36:52Z

gcc/cp/contracts.cc

-    if (tree level = TREE_VALUE (mode))
-      return IDENTIFIER_POINTER (level);
-  return "default";
+  if (!pseudo_source_location_impl_type)


[nit, not really a review] you could save a bunch of indenting by doing an early return here if pseudo_source_location_impl_type is already built (and in the following cases too) ...

iains · 2024-10-10T13:40:34Z

gcc/cp/contracts.cc

+  /* Must match the type layout in get_pseudo_contract_violation_type.  */
+  tree ctor = build_constructor_va
+    (init_list_type_node, 7,
+     NULL_TREE, build_int_cst (NULL_TREE, 0), 		// version


all the integer constants here should have a defined type, right? (as set up in get_pseudo_contract_violation_type)

changed to integer_type_node

iains · 2024-10-10T13:41:52Z

gcc/cp/contracts.cc

+     NULL_TREE, CONTRACT_COMMENT (contract),
+     NULL_TREE, build_int_cst (NULL_TREE, detection_mode),
+     NULL_TREE, build_int_cst (NULL_TREE, assertion_kind),
+     NULL_TREE, build_pseudo_source_location(contract),


missing space before (

…ion-reconfig reconfigure

Implement vddup and vidup using the new MVE builtins framework. We generate better code because we take advantage of the two outputs produced by the v[id]dup instructions. For instance, before: ldr r3, [r0] sub r2, r3, #8 str r2, [r0] mov r2, r3 vddup.u16 q3, r2, #1 now: ldr r2, [r0] vddup.u16 q3, r2, #1 str r2, [r0] 2024-08-21 Christophe Lyon <christophe.lyon@linaro.org> gcc/ * config/arm/arm-mve-builtins-base.cc (class viddup_impl): New. (vddup): New. (vidup): New. * config/arm/arm-mve-builtins-base.def (vddupq): New. (vidupq): New. * config/arm/arm-mve-builtins-base.h (vddupq): New. (vidupq): New. * config/arm/arm_mve.h (vddupq_m): Delete. (vddupq_u8): Delete. (vddupq_u32): Delete. (vddupq_u16): Delete. (vidupq_m): Delete. (vidupq_u8): Delete. (vidupq_u32): Delete. (vidupq_u16): Delete. (vddupq_x_u8): Delete. (vddupq_x_u16): Delete. (vddupq_x_u32): Delete. (vidupq_x_u8): Delete. (vidupq_x_u16): Delete. (vidupq_x_u32): Delete. (vddupq_m_n_u8): Delete. (vddupq_m_n_u32): Delete. (vddupq_m_n_u16): Delete. (vddupq_m_wb_u8): Delete. (vddupq_m_wb_u16): Delete. (vddupq_m_wb_u32): Delete. (vddupq_n_u8): Delete. (vddupq_n_u32): Delete. (vddupq_n_u16): Delete. (vddupq_wb_u8): Delete. (vddupq_wb_u16): Delete. (vddupq_wb_u32): Delete. (vidupq_m_n_u8): Delete. (vidupq_m_n_u32): Delete. (vidupq_m_n_u16): Delete. (vidupq_m_wb_u8): Delete. (vidupq_m_wb_u16): Delete. (vidupq_m_wb_u32): Delete. (vidupq_n_u8): Delete. (vidupq_n_u32): Delete. (vidupq_n_u16): Delete. (vidupq_wb_u8): Delete. (vidupq_wb_u16): Delete. (vidupq_wb_u32): Delete. (vddupq_x_n_u8): Delete. (vddupq_x_n_u16): Delete. (vddupq_x_n_u32): Delete. (vddupq_x_wb_u8): Delete. (vddupq_x_wb_u16): Delete. (vddupq_x_wb_u32): Delete. (vidupq_x_n_u8): Delete. (vidupq_x_n_u16): Delete. (vidupq_x_n_u32): Delete. (vidupq_x_wb_u8): Delete. (vidupq_x_wb_u16): Delete. (vidupq_x_wb_u32): Delete. (__arm_vddupq_m_n_u8): Delete. (__arm_vddupq_m_n_u32): Delete. (__arm_vddupq_m_n_u16): Delete. (__arm_vddupq_m_wb_u8): Delete. (__arm_vddupq_m_wb_u16): Delete. (__arm_vddupq_m_wb_u32): Delete. (__arm_vddupq_n_u8): Delete. (__arm_vddupq_n_u32): Delete. (__arm_vddupq_n_u16): Delete. (__arm_vidupq_m_n_u8): Delete. (__arm_vidupq_m_n_u32): Delete. (__arm_vidupq_m_n_u16): Delete. (__arm_vidupq_n_u8): Delete. (__arm_vidupq_m_wb_u8): Delete. (__arm_vidupq_m_wb_u16): Delete. (__arm_vidupq_m_wb_u32): Delete. (__arm_vidupq_n_u32): Delete. (__arm_vidupq_n_u16): Delete. (__arm_vidupq_wb_u8): Delete. (__arm_vidupq_wb_u16): Delete. (__arm_vidupq_wb_u32): Delete. (__arm_vddupq_wb_u8): Delete. (__arm_vddupq_wb_u16): Delete. (__arm_vddupq_wb_u32): Delete. (__arm_vddupq_x_n_u8): Delete. (__arm_vddupq_x_n_u16): Delete. (__arm_vddupq_x_n_u32): Delete. (__arm_vddupq_x_wb_u8): Delete. (__arm_vddupq_x_wb_u16): Delete. (__arm_vddupq_x_wb_u32): Delete. (__arm_vidupq_x_n_u8): Delete. (__arm_vidupq_x_n_u16): Delete. (__arm_vidupq_x_n_u32): Delete. (__arm_vidupq_x_wb_u8): Delete. (__arm_vidupq_x_wb_u16): Delete. (__arm_vidupq_x_wb_u32): Delete. (__arm_vddupq_m): Delete. (__arm_vddupq_u8): Delete. (__arm_vddupq_u32): Delete. (__arm_vddupq_u16): Delete. (__arm_vidupq_m): Delete. (__arm_vidupq_u8): Delete. (__arm_vidupq_u32): Delete. (__arm_vidupq_u16): Delete. (__arm_vddupq_x_u8): Delete. (__arm_vddupq_x_u16): Delete. (__arm_vddupq_x_u32): Delete. (__arm_vidupq_x_u8): Delete. (__arm_vidupq_x_u16): Delete. (__arm_vidupq_x_u32): Delete.

NinaRanns and others added 3 commits October 1, 2024 19:29

removing unused variable

c424e03

Merge pull request gcc-mirror#27 from NinaRanns/contract_fix_warning

761e5bb

Contract fix warning

initial draft of new contract_violation

e56f79d

iains reviewed Oct 10, 2024

View reviewed changes

iains and others added 4 commits October 10, 2024 15:57

reconfigure

288a0d8

first round of review

1934d5b

Merge pull request #11 from NinaRanns/contracts_p2900_contract_violat…

678db53

…ion-reconfig reconfigure

adding contract26.cc file

69cf292

NinaRanns closed this Oct 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Contracts p2900 contract violation #8

Contracts p2900 contract violation #8

NinaRanns commented Oct 10, 2024

iains left a comment

iains Oct 10, 2024

NinaRanns Oct 17, 2024

iains Oct 10, 2024

NinaRanns Oct 17, 2024

iains Oct 10, 2024

NinaRanns Oct 17, 2024

iains Oct 10, 2024

iains Oct 10, 2024

NinaRanns Oct 17, 2024

iains Oct 10, 2024

NinaRanns Oct 17, 2024

iains Oct 10, 2024

NinaRanns Oct 17, 2024

iains Oct 10, 2024

NinaRanns Oct 17, 2024

iains Oct 10, 2024

iains Oct 10, 2024

iains left a comment

iains Oct 10, 2024

NinaRanns Oct 17, 2024

iains Oct 10, 2024

NinaRanns Oct 17, 2024

iains Oct 10, 2024

NinaRanns Oct 17, 2024

iains Oct 10, 2024

		@@ -1692,8 +1844,30 @@ get_pseudo_contract_violation_type ()

		/* Return a VAR_DECL to pass to handle_contract_violation. */

		static const char *

Contracts p2900 contract violation #8

Contracts p2900 contract violation #8

Conversation

NinaRanns commented Oct 10, 2024

iains left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iains left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment