Re: [PATCH, Fortran, pr78672, ctp1, v1] Gfortran test suite failures with a sanitized compiler

[Andre Vehreschild <vehre at gmx dot de>]

Hi all,

apologies to all who's feet I am now trampling on...

<snipp>

> The issue for me has less to do with C++ syntax and more to do with not
> knowing the classes available.  Zooming out on the subject chunk of code:
> 
> static int
> gfc_conv_cst_int_power (gfc_se * se, tree lhs, tree rhs)
> {
>    tree cond;
>    tree tmp;
>    tree type;
>    tree vartmp[POWI_TABLE_SIZE];
>    HOST_WIDE_INT m;
>    unsigned HOST_WIDE_INT n;
>    int sgn;
>    wide_int wrhs = rhs;
> 
>    /* If exponent is too large, we won't expand it anyway, so don't bother
>       with large integer values.  */
>    if (!wi::fits_shwi_p (wrhs))
>      return 0;
> 
>    m = wrhs.to_shwi ();
>    /* There's no ABS for HOST_WIDE_INT, so here we go. It also takes care
>       of the asymmetric range of the integer type.  */
>    n = (unsigned HOST_WIDE_INT) (m < 0 ? -m : m);
> 
> We see wide_int. I was not right away familiar with where that was defined

wide_int is not new. It's old code. So nothing to complain about (see above).

> and whether it is a class or not. The :: tells me wi is a class. Then I see

wi is not a class, but a namespace. This construct is similar to modules in
Fortran, just a little bit more powerful. That could have been easily deduced
by using ctags for which you do not need any fancy IDE. Each vi can be made to
interface to it.

> we have another snippet of C++ already there on trunk. So then I have to
> wonder where is this class defined and how do I find documentation for its
> methods. Then I have to either ignore it and trust it is right or spend a lot
> of time digging around to find the documentation. I can sort of infer it from
> the abbreviated function names, but really don't know it from rote.

Programming is about constant learning. So you are arguing, that things should
not improve? So that you don't have to learn something new?

And the argument that you don't "know it from rote" will probably be true for
most of the types in gfortran. You can't know all of them.

> Although C++ has its good qualities, one of the downsides is it introduces
> you to so much abstraction and hidden layers of functions that you are forced
> to find an editor or IDE with a class browser so you can find and look at the 
> definitions. I find it ironic that the very intended concept of hiding the 
> details actually leads to obfuscation, the unintended result, simplification 
> leading to complexity. (As opposed to C which I can hold a small book in my 
> hand, or a pdf file that contains all I need to know.)

Well, now you are mixing abstraction and language. The wide_int constructs are
not part of C++. C++ is only used to implement them in a concise and easy to
understand way. I wonder how this would look in C, where one will have to use a
bunch of preprocessor macros, functions that encode the parameter type in their
name and a lot of copied code instead of a generic formulation of the problem's
solution in C++. Btw, the implementation of abs is using plain code, there is no
sophisticated template specialisation or partial specialisation. It is just
plain and straight forward. Digging around it shows that the old statement:

 n = (unsigned HOST_WIDE_INT) (m < 0 ? -m : m);

is also not 100 % portable, because to be numerically correct it should have
been something more like:

 n = m < 0 ? wi::sub (0, m) : (unsigned HOST_WIDE_INT) m;
 
> And all of the above I say regarding what we do to flip a bit in an integer
> and cast it to a different size. ( and to stop the sanitizer from complaining
> )

Because it is not flipping one bit in an integer on all platforms gfortran
will be used. Please take out your textbook about integer logic. The most common
systems use the 1-complement to represent negative numbers. With the asymmetry
I have pointed out in a previous mail you would get a 0 from the above code,
when the HOST_WIDE_INT has not been so cleverly implemented and is (at least on
my machine) implemented as three longs to hold even overflow information.

I don't understand why this discussion is needed for just making use of the
expertise of some clever folks that know more about handling large integers. And
in the patch nothing new is presented. All constructs used in the patch have
been used in the lines directly above.

> I am forced to wonder why we are doing this exercise. Was this a false
> positive from the sanitizer? I am not asking you to change it. If this is the
> best way one can find, then it is what it is.

So you are wondering why other systems, that would not use the 1-complement for
representing negative numbers would accept runtime errors, because the compiler
is doing something wrong on their hardware? I can't believe that. The sanitizer
bickering here is not a false positive in general. You are just lucky to not
see an issue on your hardware. So I am not convinced by your objections, that
the change is unnecessary.

- Andre
-- 
Andre Vehreschild * Email: vehre ad gmx dot de