Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.c++ > #86886 > unrolled thread

Type conversion for hundreds of lines

Back to article view | Back to comp.lang.c++

Contents

  Type conversion for hundreds of lines JiiPee <kerrttuPoistaTama11@gmail.com> - 2022-10-12 23:48 +0300
    Re: Type conversion for hundreds of lines JiiPee <kerrttuPoistaTama11@gmail.com> - 2022-10-12 23:54 +0300
      Re: Type conversion for hundreds of lines JiiPee <kerrttuPoistaTama11@gmail.com> - 2022-10-12 23:55 +0300
        Re: Type conversion for hundreds of lines Ben Bacarisse <ben.usenet@bsb.me.uk> - 2022-10-12 23:26 +0100
          Re: Type conversion for hundreds of lines JiiPee <kerrttuPoistaTama11@gmail.com> - 2022-10-13 07:30 +0300
            Re: Type conversion for hundreds of lines Ben Bacarisse <ben.usenet@bsb.me.uk> - 2022-10-13 12:19 +0100
      Re: Type conversion for hundreds of lines Paavo Helde <eesnimi@osa.pri.ee> - 2022-10-13 00:37 +0300
        Re: Type conversion for hundreds of lines JiiPee <kerrttuPoistaTama11@gmail.com> - 2022-10-13 07:34 +0300
          Re: Type conversion for hundreds of lines Paavo Helde <eesnimi@osa.pri.ee> - 2022-10-13 11:43 +0300
    Re: Type conversion for hundreds of lines Paavo Helde <eesnimi@osa.pri.ee> - 2022-10-13 00:27 +0300
      Re: Type conversion for hundreds of lines JiiPee <kerrttuPoistaTama11@gmail.com> - 2022-10-13 07:24 +0300
        Re: Type conversion for hundreds of lines Paavo Helde <eesnimi@osa.pri.ee> - 2022-10-13 12:00 +0300
          Re: Type conversion for hundreds of lines JiiPee <kerrttuPoistaTama11@gmail.com> - 2022-10-13 20:27 +0300
        Re: Type conversion for hundreds of lines David Brown <david.brown@hesbynett.no> - 2022-10-13 11:17 +0200
          Re: Type conversion for hundreds of lines JiiPee <kerrttuPoistaTama11@gmail.com> - 2022-10-13 20:32 +0300
            Re: Type conversion for hundreds of lines Paavo Helde <eesnimi@osa.pri.ee> - 2022-10-13 21:13 +0300
              Re: Type conversion for hundreds of lines JiiPee <kerrttuPoistaTama11@gmail.com> - 2022-10-13 21:28 +0300
            Re: Type conversion for hundreds of lines David Brown <david.brown@hesbynett.no> - 2022-10-13 23:10 +0200
            Re: Type conversion for hundreds of lines Mike Terry <news.dead.person.stones@darjeeling.plus.com> - 2022-10-13 22:36 +0100
              Re: Type conversion for hundreds of lines JiiPee <kerrttuPoistaTama11@gmail.com> - 2022-10-14 07:29 +0300
    Re: Type conversion for hundreds of lines scott@slp53.sl.home (Scott Lurndal) - 2022-10-12 21:27 +0000
      Re: Type conversion for hundreds of lines JiiPee <kerrttuPoistaTama11@gmail.com> - 2022-10-13 07:25 +0300
    Re: Type conversion for hundreds of lines Keith Thompson <Keith.S.Thompson+u@gmail.com> - 2022-10-12 14:30 -0700
      Re: Type conversion for hundreds of lines JiiPee <kerrttuPoistaTama11@gmail.com> - 2022-10-13 07:28 +0300
        Re: Type conversion for hundreds of lines "Chris M. Thomasson" <chris.m.thomasson.1@gmail.com> - 2022-10-12 21:30 -0700
          Re: Type conversion for hundreds of lines JiiPee <kerrttuPoistaTama11@gmail.com> - 2022-10-13 07:37 +0300
            Re: Type conversion for hundreds of lines "Chris M. Thomasson" <chris.m.thomasson.1@gmail.com> - 2022-10-13 14:18 -0700
              Re: Type conversion for hundreds of lines JiiPee <kerrttuPoistaTama11@gmail.com> - 2022-10-14 00:27 +0300
                Re: Type conversion for hundreds of lines "Chris M. Thomasson" <chris.m.thomasson.1@gmail.com> - 2022-10-14 00:33 -0700
    Re: Type conversion for hundreds of lines Juha Nieminen <nospam@thanks.invalid> - 2022-10-13 07:52 +0000
      Re: Type conversion for hundreds of lines JiiPee <kerrttuPoistaTama11@gmail.com> - 2022-10-13 20:35 +0300
    Re: Type conversion for hundreds of lines Frederick Virchanza Gotham <cauldwell.thomas@gmail.com> - 2022-10-14 01:43 -0700

Page 1 of 2  [1] 2  Next page →

#86886 — Type conversion for hundreds of lines

I have been pondering this many times. I keep it short:
If I have hundreds of lines like this:

short a;
std::vector<int> v;
...
a = v.size();

This gives a warning: "warning, assigning size_t to short".
I know this can be fixed:
a = static_cast<short>(v.size());

but if we have hundreds of those lines, how would you fix this? Place a 
static cast in all of them? Of create some helper funktion to do this?

How about if I have a function created by me before:
size_t CalcLength(int a);

and then I call it 100 places like this:
short b;
b = CalcLength(466);

again type warning. Would you fix it by placing static cast in 100 
places like this:

b = static_cast<short>(CalcLength(466));

wherever this cast can be done? And what if CalcLength() can in some 
code places return a large number also? So we cannot just blindly change 
the return type to "short" to solve this.

[toc] | [next] | [standalone]

#86887

... also, how to you guys do this kind of situation:

int sum  = 0;
for(int i = v.size() - 1; i >= 0; --i)
	sum  += i;

..so assuming we cannot change the type of i.

again types different, so would you do:
for(int i = static_cast<int>(v.size()) - 1; i >= 0; --i)
?

there are alot of place where vectors size() is assigned to an int. So 
you always use static cast there?

[toc] | [prev] | [next] | [standalone]

#86888

On 12/10/2022 23:54, JiiPee wrote:
> ... also, how to you guys do this kind of situation:
> 
> int sum  = 0;
> for(int i = v.size() - 1; i >= 0; --i)
>      sum  += i;
> 

forgot to add that v is std::vector<int>.

[toc] | [prev] | [next] | [standalone]

#86893

JiiPee <kerrttuPoistaTama11@gmail.com> writes:

> On 12/10/2022 23:54, JiiPee wrote:
>> ... also, how to you guys do this kind of situation:
>> int sum  = 0;
>> for(int i = v.size() - 1; i >= 0; --i)
>>      sum  += i;
>
> forgot to add that v is std::vector<int>.

... and presumably you meant sum += v[i]; or the loop is pointless!

And why run the indexes backwards?  It might be needed for some vectors
of floating-point numbers, but not for int.

It seems that a lot of peculiar choices have been made in the code base.

-- 
Ben.

[toc] | [prev] | [next] | [standalone]

#86905

On 13/10/2022 01:26, Ben Bacarisse wrote:
> ... and presumably you meant sum += v[i]; or the loop is pointless!
> 
> And why run the indexes backwards?  It might be needed for some vectors
> of floating-point numbers, but not for int.
> 
> It seems that a lot of peculiar choices have been made in the code base.

the example is only created to illustrate the copy propblem... its not 
an example from a real code.

The point is, that if I copy in a for-loop vector::size() to an integer, 
do you always do the casting for it? Because there are many places that 
can happen....

[toc] | [prev] | [next] | [standalone]

#86921

JiiPee <kerrttuPoistaTama11@gmail.com> writes:

> On 13/10/2022 01:26, Ben Bacarisse wrote:
>> ... and presumably you meant sum += v[i]; or the loop is pointless!
>> And why run the indexes backwards?  It might be needed for some vectors
>> of floating-point numbers, but not for int.
>> It seems that a lot of peculiar choices have been made in the code base.
>
> the example is only created to illustrate the copy propblem... its not
> an example from a real code.

Sure, but the loop running backwards, starting at size-1, introduces other
things to be careful about.

> The point is, that if I copy in a for-loop vector::size() to an
> integer, do you always do the casting for it?

No.  Since the scope is small in a for loop, I'd change the declaration
and run the loop forward.  It's more changes, but it will make the code
clearer.

If I had reliable regression tests, I'd consider writing

  int sum = std::accumulate(v.begin(), v.end(), 0);

> Because there are many places that can happen....

Roughly how many?  I think it makes sense to tidy up the code rather
than spray it with casts.

-- 
Ben.

[toc] | [prev] | [next] | [standalone]

#86892

12.10.2022 23:54 JiiPee kirjutas:
> ... also, how to you guys do this kind of situation:
> 
> int sum  = 0;
> for(int i = v.size() - 1; i >= 0; --i)
>      sum  += i;
> 
> ..so assuming we cannot change the type of i.
> 
> again types different, so would you do:
> for(int i = static_cast<int>(v.size()) - 1; i >= 0; --i)
> ?
> 
> there are alot of place where vectors size() is assigned to an int. So 
> you always use static cast there?

int is a wrong type here. A correct one would be std::ptrdiff_t.

Anyway, even if I were convinced that int would be large enough in all 
imagined circumstances, I still wouldn't use static_cast here, but e.g. 
boost::numeric_cast, or at least my own debug_cast, which resolves to 
boost::numeric_cast in debug builds and static_cast in release builds.

Note: boost::numeric_cast will throw an exception in case my reasoning 
was wrong, which is a very good thing.

[toc] | [prev] | [next] | [standalone]

#86907

On 13/10/2022 00:37, Paavo Helde wrote:
> wouldn't use static_cast here, but e.g. boost::numeric_cast,

OK, interesting option.

Ok, but you would always cast it somehow and not for example create some 
conversion function etc?

I mean, if this conversion is needed in 500 places, you would just add 
that boost::numeric_cast (or some other cast) into these places?

It makes the code quite "long" and a little difficult to read if adding 
a lot of casts... but maybe its the only way.

[toc] | [prev] | [next] | [standalone]

#86913

13.10.2022 07:34 JiiPee kirjutas:
> On 13/10/2022 00:37, Paavo Helde wrote:
>> wouldn't use static_cast here, but e.g. boost::numeric_cast,
> 
> OK, interesting option.
> 
> Ok, but you would always cast it somehow and not for example create some 
> conversion function etc?
> 
> I mean, if this conversion is needed in 500 places, you would just add 
> that boost::numeric_cast (or some other cast) into these places?

boost::numeric_cast is technically a function, not a cast.

> 
> It makes the code quite "long" and a little difficult to read if adding 
> a lot of casts... but maybe its the only way.

Right. In real code I'm using my own checked_cast and debug_cast, which 
names I consider reasonable readable and greppable. These are a bit 
involved because the boost::numeric_cast did not always quite do what we 
wanted exactly, but for brevity I leave out these details from here.

template<typename T, typename U>
inline constexpr T checked_cast(U x) {
   // Some code to avoid false alarms on MS "smaller type check.
   // ...
   // Some code to refuse converting NaN to uint64
   // ...
   return boost::numeric_cast<T, U>(x);
}

// A specialization for double->float conversion
// to avoid false alarm on -inf, and to allow converting
// large finite double values to float inf.
template<>
inline constexpr float checked_cast<float, double>(double x) {
	return static_cast<float>(x);
}

template<typename T, typename U>
inline constexpr T debug_cast(U x) {
#ifdef NDEBUG
   return static_cast<T>(x);
#else
   return checked_cast<T,U>(x);
#endif
}


I'm using debug_cast whenever I am quite sure the cast should always 
work, and checked_cast if there is any doubt.

[toc] | [prev] | [next] | [standalone]

#86889

12.10.2022 23:48 JiiPee kirjutas:
> I have been pondering this many times. I keep it short:
> If I have hundreds of lines like this:
> 
> short a;
> std::vector<int> v;
> ...
> a = v.size();
> 
> This gives a warning: "warning, assigning size_t to short".
> I know this can be fixed:
> a = static_cast<short>(v.size());
> 
> but if we have hundreds of those lines, how would you fix this? Place a 
> static cast in all of them? Of create some helper funktion to do this?
> 
> How about if I have a function created by me before:
> size_t CalcLength(int a);
> 
> and then I call it 100 places like this:
> short b;
> b = CalcLength(466);
> 
> again type warning. Would you fix it by placing static cast in 100 
> places like this:
> 
> b = static_cast<short>(CalcLength(466));
> 
> wherever this cast can be done? And what if CalcLength() can in some 
> code places return a large number also? So we cannot just blindly change 
> the return type to "short" to solve this.
> 

Why on earth are you having local variables of type short?

I would fix this ASAP either by s/short/auto/ or s/short/size_t/.

[toc] | [prev] | [next] | [standalone]

#86902

On 13/10/2022 00:27, Paavo Helde wrote:
> 12.10.2022 23:48 JiiPee kirjutas:

> 
> Why on earth are you having local variables of type short?
> 
> I would fix this ASAP either by s/short/auto/ or s/short/size_t/.
> 
> 

But if its old code and hundreds of places, that would require a lot of 
testing, right? But you would still change it even though might cause 
other issues?

[toc] | [prev] | [next] | [standalone]

#86914

13.10.2022 07:24 JiiPee kirjutas:
> On 13/10/2022 00:27, Paavo Helde wrote:
>> 12.10.2022 23:48 JiiPee kirjutas:
> 
>>
>> Why on earth are you having local variables of type short?
>>
>> I would fix this ASAP either by s/short/auto/ or s/short/size_t/.
>>
>>
> 
> But if its old code and hundreds of places, that would require a lot of 
> testing, right? But you would still change it even though might cause 
> other issues?

If it's old code without unit tests, then one should start with writing 
tests.

When the code gets covered by tests enough, only then it becomes 
possible to make major code refactorings.

Not sure if fixing short would qualify as a major refactoring or not. I 
understand that you want to get rid of warnings. It means you have to 
change all those hundreds of places anyway, so why not fix it properly 
instead of hiding the potential problems with a cast?

The warning is there for a reason. and hiding it with a cast might just 
hide a bug.

In general it should be not so tricky to fix vec.size() result from 
short to size_t or auto. One just needs to inspect the code to see that 
the value never becomes negative. In my experience old dirty codebases 
contain a lot of copy-paste, so it should be possible to recognize the 
patterns quite soon. Of course, a proper refactoring would get rid of 
copy-pasted code, and I bet a lot of those 'short' variables could be 
just eliminated by switching over to range-based for loops.

[toc] | [prev] | [next] | [standalone]

#86928

On 13/10/2022 12:00, Paavo Helde wrote:
> If it's old code without unit tests, then one should start with writing 
> tests.
> 
> When the code gets covered by tests enough, only then it becomes 
> possible to make major code refactorings.

ye I agree, best way if possible to make tests to be sure changes will 
not harm other code.

> 
> Not sure if fixing short would qualify as a major refactoring or not. I 
> understand that you want to get rid of warnings. It means you have to 
> change all those hundreds of places anyway, so why not fix it properly 
> instead of hiding the potential problems with a cast?

hmm, fair point. Especially if those places are not too many, like only 
40 for example.

[toc] | [prev] | [next] | [standalone]

#86917

On 13/10/2022 06:24, JiiPee wrote:
> On 13/10/2022 00:27, Paavo Helde wrote:
>> 12.10.2022 23:48 JiiPee kirjutas:
> 
>>
>> Why on earth are you having local variables of type short?
>>
>> I would fix this ASAP either by s/short/auto/ or s/short/size_t/.
>>
>>
> 
> But if its old code and hundreds of places, that would require a lot of 
> testing, right? But you would still change it even though might cause 
> other issues?
> 

With old code that has techniques that you consider questionable today, 
there are usually three options.

One is to consider the code as "tried and tested" and assume it is 
correct.  Add warning disable pragmas at the start of the code to 
minimise noise.  Obviously the details here depend on the compiler and 
the warnings - as an example, you might have :

	#pragma GCC diagnostic ignored "-Wsign-conversion"

Clearly this is a dangerous path - hiding the potential problems.  But 
maybe the code works fine in practice, as many compiler warnings are 
about /potential/ problems rather than real ones.


Number two is to pick compiler options that change the semantics of the 
language to match the code's assumptions.  A common example is to have 
"-fwrapv" (or a GCC/clang/icc pragma - it's often better to have a 
pragma in the code than a compiler flag) to turn old code that assumes 
wrapping integer overflow into correct code.  This only applies to 
certain cases, however.


Number three is to fix the code.  That might mean changing the types of 
variables, refactorising, modernising, or otherwise changing the code. 
I'd be sceptical about simply adding static casts - the suggestion of 
using a function that supports optional run-time checks is probably better.

[toc] | [prev] | [next] | [standalone]

#86929

On 13/10/2022 12:17, David Brown wrote:
> One is to consider the code as "tried and tested" and assume it is 
> correct.  Add warning disable pragmas at the start of the code to 
> minimise noise.  Obviously the details here depend on the compiler and 
> the warnings - as an example, you might have :
> 
>      #pragma GCC diagnostic ignored "-Wsign-conversion"

good point, in old code can assume if its well tested.
Can I disable a warning blockwise or filewise in Visual Studio?

> 
> Clearly this is a dangerous path - hiding the potential problems.  But 
> maybe the code works fine in practice, as many compiler warnings are 
> about /potential/ problems rather than real ones.

yes, if its for example tested 20 years....

> 
> 
> 
> Number three is to fix the code.  That might mean changing the types of 
> variables, refactorising, modernising, or otherwise changing the code. 
> I'd be sceptical about simply adding static casts - the suggestion of 
> using a function that supports optional run-time checks is probably better.

good point... rather use more sophisticated conversion function than cast.

[toc] | [prev] | [next] | [standalone]

#86931

13.10.2022 20:32 JiiPee kirjutas:
> Can I disable a warning blockwise or filewise in Visual Studio?

MSVC also has pragmas to suppress warnings, with different syntax. Example:

#ifdef _MSC_VER
#pragma warning(disable:4100)	// unreferenced formal parameter
#endif

[toc] | [prev] | [next] | [standalone]

#86932

On 13/10/2022 21:13, Paavo Helde wrote:
> MSVC also has pragmas to suppress warnings, with different syntax. Example:
> 
> #ifdef _MSC_VER
> #pragma warning(disable:4100)    // unreferenced formal parameter
> #endif


ok thanks. so with this I can switch it on and off

[toc] | [prev] | [next] | [standalone]

#86934

On 13/10/2022 19:32, JiiPee wrote:
> On 13/10/2022 12:17, David Brown wrote:
>> One is to consider the code as "tried and tested" and assume it is 
>> correct.  Add warning disable pragmas at the start of the code to 
>> minimise noise.  Obviously the details here depend on the compiler and 
>> the warnings - as an example, you might have :
>>
>>      #pragma GCC diagnostic ignored "-Wsign-conversion"
> 
> good point, in old code can assume if its well tested.
> Can I disable a warning blockwise or filewise in Visual Studio?
> 

Not a clue, sorry.  I'd assume there are equivalent pragmas, but there's 
no point in my googling for you!

>>
>> Clearly this is a dangerous path - hiding the potential problems.  But 
>> maybe the code works fine in practice, as many compiler warnings are 
>> about /potential/ problems rather than real ones.
> 
> yes, if its for example tested 20 years....
> 

Of course, you can only be sure it works if your compiler today doesn't 
do any optimisations or different code generation techniques from the 
compiler of 20 years ago.

(Sometimes I work with projects that were written and tested up to 20+ 
years ago - but I archive the compiler along with the project, so that 
the generated binary is identical.)

>>
>>
>>
>> Number three is to fix the code.  That might mean changing the types 
>> of variables, refactorising, modernising, or otherwise changing the 
>> code. I'd be sceptical about simply adding static casts - the 
>> suggestion of using a function that supports optional run-time checks 
>> is probably better.
> 
> good point... rather use more sophisticated conversion function than cast.
> 

[toc] | [prev] | [next] | [standalone]

#86938

On 13/10/2022 18:32, JiiPee wrote:
> On 13/10/2022 12:17, David Brown wrote:
>> One is to consider the code as "tried and tested" and assume it is correct.  Add warning disable 
>> pragmas at the start of the code to minimise noise.  Obviously the details here depend on the 
>> compiler and the warnings - as an example, you might have :
>>
>>      #pragma GCC diagnostic ignored "-Wsign-conversion"
> 
> good point, in old code can assume if its well tested.
> Can I disable a warning blockwise or filewise in Visual Studio?

You can use

#pragma warning( push )

and

#pragma warning( pop )

to save/restore the current warning settings.  E.g. a header file could be structured:

---- start header ----
#pragma warning( push )
#pragma warning( disable : 4706 )
...
...
#pragma warning( pop )
---- end header ----

to disable warning 4706 for just that header file.

Mike.

[toc] | [prev] | [next] | [standalone]

#86945

On 14/10/2022 00:36, Mike Terry wrote:
> You can use
> 
> #pragma warning( push )
> 
> and
> 
> #pragma warning( pop )
> 
> to save/restore the current warning settings.

thanks. I ll save this and can try it :).

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | comp.lang.c++

csiph-web