Hello James.
Thanks for the patch.  Please see a couple of comments inlined below.

> Signed-off-by: James Bohl <[email protected]>
>
> gcc/algol68/ChangeLog
>
>         * a68-low-units.cc (a68_lower_denotation): Add error on
>         integral denotation overflow.
>
> gcc/testsuite/ChangeLog
>
>         * algol68/compile/error-denotation-1.a68: New Test.
>         * algol68/execute/plusab-1.a68: Fixed denotation overflow.
> ---
>  gcc/algol68/a68-low-units.cc                         |  9 +++++++++
>  gcc/testsuite/algol68/compile/error-denotation-1.a68 |  5 +++++
>  gcc/testsuite/algol68/execute/plusab-1.a68           | 10 +++++-----
>  3 files changed, 19 insertions(+), 5 deletions(-)
>  create mode 100644 gcc/testsuite/algol68/compile/error-denotation-1.a68
>
> diff --git a/gcc/algol68/a68-low-units.cc b/gcc/algol68/a68-low-units.cc
> index cc1fd15fcca..e99f4b2cd63 100644
> --- a/gcc/algol68/a68-low-units.cc
> +++ b/gcc/algol68/a68-low-units.cc
> @@ -41,6 +41,7 @@
>  #include "convert.h"
>  
>  #include "a68.h"
> +#include "a68-pretty-print.h"
>  
>  /* Note that enclosed clauses, which are units, are handled in
>     a68-low-clauses.  */
> @@ -250,8 +251,16 @@ a68_lower_denotation (NODE_T *p, LOW_CTX_T ctx)
>       s = SUB (p);
>  
>        type = CTYPE (moid);
> +      errno = 0;
>        int64_t val = strtol (NSYMBOL (s), &end, 10);

I think the original code is wrong here, because it should be using
strtoll instead of strtol: the host long int may be 32 bits or even
less.

The proper idiom, for example in c-family/c-common.cc, seems to be:

#if defined(INT64_T_IS_LONG)
  val = strtol (NSYMBOL (s), &end, 10);
#else
  val = strtoll (NSYMBOL (s), &end, 10);
#endif

>        gcc_assert (end[0] == '\0');
> +      tree t = build_int_cst (long_long_integer_type_node, val);
> +      if (errno == ERANGE || !int_fits_type_p (t, type))
> +     {
> +       a68_moid_format_token m (moid);
> +       a68_error (s, "denotation is too large for %e", &m);
> +     }
> +

Perhaps it is possible to avoid allocating the temporary tree node `t'
by using `wi::min_value (type)' and `wi::max_value (type)' instead?

>        return build_int_cst (type, val);
>      }
>    if (moid == M_BITS
> diff --git a/gcc/testsuite/algol68/compile/error-denotation-1.a68 
> b/gcc/testsuite/algol68/compile/error-denotation-1.a68
> new file mode 100644
> index 00000000000..4b037aac5fa
> --- /dev/null
> +++ b/gcc/testsuite/algol68/compile/error-denotation-1.a68
> @@ -0,0 +1,5 @@
> +{ dg-options {-fstropping=supper} }
> +begin int i := 20000000000000000000; { dg-error "denotation is too large for 
> int" }
> +      int j := 2000000000000000000; { dg-error "denotation is too large for 
> int" }
> +      skip
> +end

Note that this tests will fail in targets in which integer_type_node is
not exactly 32 bit.

I see there is a check_effective_target_int32 defined in
gcc/testsuite/lib/target-supports.exp, so you must be able to:


 { dg-require-effective-target int32 }

There may be an existing dg-require check for
this... (effetive-target)

> diff --git a/gcc/testsuite/algol68/execute/plusab-1.a68 
> b/gcc/testsuite/algol68/execute/plusab-1.a68
> index 8de4e97b046..48865f8a1fe 100644
> --- a/gcc/testsuite/algol68/execute/plusab-1.a68
> +++ b/gcc/testsuite/algol68/execute/plusab-1.a68
> @@ -12,11 +12,11 @@ BEGIN BEGIN INT i := 10;
>              i PLUSAB SHORT 100;
>              ASSERT (i = SHORT 1200)
>        END;
> -      BEGIN SHORT SHORT INT i := SHORT SHORT 10000;
> -            i +:= SHORT SHORT 1000;
> -            ASSERT (i = SHORT SHORT 11000);
> -            i PLUSAB SHORT SHORT 1000;
> -            ASSERT (i = SHORT SHORT 12000)
> +      BEGIN SHORT SHORT INT i := SHORT SHORT 100;
> +            i +:= SHORT SHORT 10;
> +            ASSERT (i = SHORT SHORT 110);
> +            i PLUSAB SHORT SHORT 10;
> +            ASSERT (i = SHORT SHORT 120)

Oops :D

>        END;
>  
>        BEGIN LONG INT i := LONG 1000;

Reply via email to