Skip to content

Explicit casts to avoid conversion warnings [blocks: #2310]#2551

Closed
tautschnig wants to merge 8 commits intodiffblue:developfrom
tautschnig:vs-pragma-conversion
Closed

Explicit casts to avoid conversion warnings [blocks: #2310]#2551
tautschnig wants to merge 8 commits intodiffblue:developfrom
tautschnig:vs-pragma-conversion

Conversation

@tautschnig
Copy link
Copy Markdown
Collaborator

@tautschnig tautschnig commented Jul 7, 2018

Visual Studio warns about signed/unsigned conversion and limited range of types.

@smowton
Copy link
Copy Markdown
Contributor

smowton commented Jul 7, 2018

These really can't be silenced with an explicit typecast?

@smowton
Copy link
Copy Markdown
Contributor

smowton commented Jul 7, 2018

Also a lot of these don't appear to be related to standard library calls?

Copy link
Copy Markdown
Contributor

@allredj allredj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passed Diffblue compatibility checks (cbmc commit: 5d40b60).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/78358680

@tautschnig tautschnig self-assigned this Sep 4, 2018
@tautschnig tautschnig changed the title Pragma to silence Visual Studio conversion warnings Pragma to silence Visual Studio conversion warnings [blocks: #2310] Nov 7, 2018
@tautschnig tautschnig force-pushed the vs-pragma-conversion branch 3 times, most recently from 2acc9aa to bace090 Compare November 15, 2018 09:08
@tautschnig tautschnig changed the title Pragma to silence Visual Studio conversion warnings [blocks: #2310] Explicit casts to avoid conversion warnings [blocks: #2310] Dec 28, 2018
@tautschnig
Copy link
Copy Markdown
Collaborator Author

This is now re-implemented using explicit type casts.

@tautschnig tautschnig removed their assignment Dec 28, 2018
Copy link
Copy Markdown
Contributor

@allredj allredj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✔️
Passed Diffblue compatibility checks (cbmc commit: db43e7a).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/95906397

@tautschnig tautschnig force-pushed the vs-pragma-conversion branch 2 times, most recently from b72b9d0 to 7c61bdb Compare January 4, 2019 16:29
Copy link
Copy Markdown
Contributor

@allredj allredj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✔️
Passed Diffblue compatibility checks (cbmc commit: c332386).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/102222178

@tautschnig tautschnig self-assigned this Mar 8, 2019
@tautschnig tautschnig force-pushed the vs-pragma-conversion branch from c332386 to d96a361 Compare March 14, 2019 20:49
Copy link
Copy Markdown
Contributor

@allredj allredj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️
This PR failed Diffblue compatibility checks (cbmc commit: d96a361).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/104504752
Status will be re-evaluated on next push.
Common spurious failures:

  • the cbmc commit has disappeared in the mean time (e.g. in a force-push)

  • the author is not in the list of contributors (e.g. first-time contributors).

  • the compatibility was already broken by an earlier merge.

@tautschnig tautschnig force-pushed the vs-pragma-conversion branch from d96a361 to 13856db Compare March 30, 2019 00:27
@tautschnig tautschnig removed their assignment Mar 30, 2019
Copy link
Copy Markdown
Contributor

@allredj allredj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✔️
Passed Diffblue compatibility checks (cbmc commit: 13856db).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/106408234

@kroening kroening assigned thk123 and unassigned kroening Apr 30, 2019
Copy link
Copy Markdown
Contributor

@thk123 thk123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of changes I think you should double check, but otherwise lgtm

for(unsigned i=0; i<src.size(); i++)
{
T ch=(unsigned char)src[i];
T ch = narrow_cast<T>(narrow_cast<unsigned char>(src[i]));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ Why narrow to unsigned char first? 🐭

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to preserve the existing behaviour, as T might actually be wider than unsigned char. That said, I guess narrow_cast would fail in a case where that makes a difference, so I can probably get rid of it.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nested cast removed.

assert(i<src.size()); // backslash can't be last character

ch=(unsigned char)src[i];
ch = narrow_cast<T>(narrow_cast<unsigned char>(src[i]));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐭

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

if(elf_reader.section_name(i)=="goto-cc")
{
in.seekg(elf_reader.section_offset(i));
in.seekg(std::streamoff{elf_reader.section_offset(i)});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ Is this change meant to be here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, because section_offset returns a std::streampos (which makes sense in other contexts).

@tautschnig tautschnig force-pushed the vs-pragma-conversion branch 4 times, most recently from 02abb9a to 36b9d0c Compare May 30, 2019 14:01
Copy link
Copy Markdown
Contributor

@allredj allredj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✔️
Passed Diffblue compatibility checks (cbmc commit: 36b9d0c).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/113768512

Visual Studio warns about signed/unsigned conversion and limited range of types.
Visual Studio warns about signed/unsigned conversion and limited range of types.
Visual Studio warns about signed/unsigned conversion and limited range of types.
Visual Studio warns about signed/unsigned conversion and limited range of types.
Visual Studio warns about signed/unsigned conversion and limited range of types.
…d long long

Visual Studio warns about signed/unsigned conversion and limited range of types.
This avoids conversion warnings with Visual Studio.
If the inner cast actually had any effect then that would amount to a
loss of precision, which in turn would necessarily fail with
narrrow_cast.
@tautschnig tautschnig force-pushed the vs-pragma-conversion branch from 36b9d0c to bf8ae77 Compare May 31, 2019 21:47
Copy link
Copy Markdown
Contributor

@allredj allredj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️
This PR failed Diffblue compatibility checks (cbmc commit: bf8ae77).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/113953221
Status will be re-evaluated on next push.
Common spurious failures include: the cbmc commit has disappeared in the mean time (e.g. in a force-push); the author is not in the list of contributors (e.g. first-time contributors); compatibility was already broken by an earlier merge.

@tautschnig tautschnig self-assigned this Nov 20, 2022
@TGWDB
Copy link
Copy Markdown
Contributor

TGWDB commented May 3, 2023

Closing due to age (no further comment on PR content), please reopen with rebase on develop if you intent to continue this work.

@TGWDB TGWDB closed this May 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants