Re: position.hh compile error C4146 (VisualStudio 2017)

[Rici Lake]

Works for me. (Not that I tried it or anything :-) I don't have Visual
Studio anywhere.)

2018-08-18 10:33 GMT-05:00 Akim Demaille <address@hidden>:

>
> > Le 18 août 2018 à 17:10, Rici Lake <address@hidden> a écrit :
> >
> >> I don’t remember why I wrote it this way, but today I don’t care much
> >> about INT_MIN here.  I think the code has become too complex for what it
> >> meant to do.  Also, maybe I should have sticked to int instead of
> trying to
> >> unsigned, but it’s too late to change that :)
> >>
> >>
> > Yes, keeping everything int would have been easier. Or even templating
> the
> > class on a (signed) integer type, so that applications expecting enormous
> > files could deal with them.
>
> That’s the kind of things I’d be happy to deal with later, but I
> first want to push 3.1 outside (smashing all the not too hard bugs
> I can see), and then support move semantics in C++ in 3.2.
>
>
> >> What do you think of my proposal restoring std::max?
> >
> > It's certainly clearer, although of course there are issues with integer
> > overflow on huge inputs (but we don’t care about those, right? -:).
>
> Let’s wait for bug reports about that :)
>
> > Since min is a parameter to add_, you can just make it int instead of
> > unsigned. No point in static_cast<int>(min).
>
> Meh…  That way I was sure that min was nonnegative, hence that the
> unsigned I return is « really » nonnegative.
>
> But you’re right, let’s make it simpler.
>
> Good to go?
>
> commit 59f931a50fcd3f3687377507e6e13a6a236d236b
> Author: Akim Demaille <address@hidden>
> Date:   Sat Aug 18 16:37:47 2018 +0200
>
>     C++: fix portability issue with MSVC 2017
>
>     Visual Studio issues a C4146 warning on '-static_cast<unsigned>(rhs)'.
>     The code is weird, probably to cope with INT_MIN.  Let's go back to
>     using std::max (whose header is still included in position.hh...) like
>     originally, but with the needed casts.
>
>     Reported by 長田偉伸, and with help from Rici Lake.
>
>     See also
>     http://lists.gnu.org/archive/html/bug-bison/2013-02/msg00000.html
>     and commit 75ae8299840bbd854fa2474d38402bbb933c6511.
>
>     * data/location.cc (position::add_): Take min as an int.
>     Use std::max.
>     While here, get rid of a couple of useless inlines.
>
> diff --git a/THANKS b/THANKS
> index 33f23ed7..c655e3c6 100644
> --- a/THANKS
> +++ b/THANKS
> @@ -173,6 +173,7 @@ Wolfram Wagner            address@hidden
>  Wwp                       address@hidden
>  xolodho                   address@hidden
>  Zack Weinberg             address@hidden
> +長田偉伸                   address@hidden
>
>  Many people are not named here because we lost track of them.  We
>  thank them!  Please, help us keeping this list up to date.
> diff --git a/data/location.cc b/data/location.cc
> index 3cc949df..07f1ca62 100644
> --- a/data/location.cc
> +++ b/data/location.cc
> @@ -73,12 +73,10 @@ m4_define([b4_position_define],
>      unsigned column;
>
>    private:
> -    /// Compute max(min, lhs+rhs) (provided min <= lhs).
> -    static unsigned add_ (unsigned lhs, int rhs, unsigned min)
> +    /// Compute max(min, lhs+rhs).
> +    static unsigned add_ (unsigned lhs, int rhs, int min)
>      {
> -      return (0 < rhs || -static_cast<unsigned>(rhs) < lhs
> -              ? rhs + lhs
> -              : min);
> +      return static_cast<unsigned>(std::max(min, static_cast<int>(lhs) +
> rhs));
>      }
>    };
>
> @@ -134,7 +132,7 @@ m4_define([b4_position_define],
>     ** \param pos a reference to the position to redirect
>     */
>    template <typename YYChar>
> -  inline std::basic_ostream<YYChar>&
> +  std::basic_ostream<YYChar>&
>    operator<< (std::basic_ostream<YYChar>& ostr, const position& pos)
>    {
>      if (pos.filename)
> @@ -271,7 +269,7 @@ m4_define([b4_location_define],
>     ** Avoid duplicate information.
>     */
>    template <typename YYChar>
> -  inline std::basic_ostream<YYChar>&
> +  std::basic_ostream<YYChar>&
>    operator<< (std::basic_ostream<YYChar>& ostr, const location& loc)
>    {
>      unsigned end_col = 0 < loc.end.column ? loc.end.column - 1 : 0;
>
>