Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix various type conversion and deprecation warnings reported by Visual studio #2310

Draft
wants to merge 18 commits into
base: develop
Choose a base branch
from

Conversation

tautschnig
Copy link
Collaborator

Reviews appreciated, but this is still incomplete as I am trying to get updated results from AppVeyor.

{
return class_index;
}
u1 get_name_and_type_index() const
u2 get_name_and_type_index() const
{
Copy link
Member

Choose a reason for hiding this comment

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

Uhh, these might be serious.

return -1; // we cannot distinguish the elements

return sub*i;
return sub * *size;
}
Copy link
Member

Choose a reason for hiding this comment

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

I really like (*size) here, otherwise looks a bit like a power in Verilog.

return -1; // we cannot distinguish the elements

return sub*i;
return sub * *size;
}
Copy link
Member

Choose a reason for hiding this comment

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

Same here

@tautschnig tautschnig force-pushed the visual-studio-warnings branch 2 times, most recently from 2714dac to c306c80 Compare June 8, 2018 18:34
@tautschnig
Copy link
Collaborator Author

I have started factoring out parts into digestable PRs. I will keep using this one as my testbed to gather input from AppVeyor. No need for reviews, though.

@tautschnig tautschnig self-assigned this Jun 12, 2018
@tautschnig tautschnig force-pushed the visual-studio-warnings branch 2 times, most recently from fb8ccf7 to 2cadc86 Compare June 20, 2018 10:32
@tautschnig tautschnig requested a review from forejtv as a code owner June 20, 2018 10:32
@tautschnig tautschnig force-pushed the visual-studio-warnings branch 5 times, most recently from d07e1c3 to 98f8962 Compare June 21, 2018 19:23
@tautschnig tautschnig force-pushed the visual-studio-warnings branch 2 times, most recently from 808818b to c52a4d7 Compare June 21, 2018 19:46
@tautschnig
Copy link
Collaborator Author

[...]

Is this large PR still of value in itself, or is it just useful as a branch to begin splitting out the work?

Mostly the latter, but there are other PRs that still have higher priority to be cleaned up and merged before I can spend time on this one.

@thomasspriggs
Copy link
Contributor

Mostly the latter, but there are other PRs that still have higher priority to be cleaned up and merged before I can spend time on this one.

Given that you are planning to raise separate PRs, rather than ever merging this one and that you can refer to the branch without keeping the PR open, are you ok with this PR being closed out for the moment? Just trying to keep the number of currently open PRs down.

@NlightNFotis
Copy link
Contributor

Going ahead with closing this PR as being very stale.

Feel free to reopen this PR if you believe that closing this was a mistake - it will need a rebase on top of develop to get it back into a mergeable state.

It will also probably need a rerun of everything on VS2019, given that this is what we are supporting on our CI platforms as well.

@tautschnig tautschnig reopened this Feb 7, 2025
@tautschnig tautschnig requested review from a team, TGWDB and remi-delmas-3000 as code owners February 7, 2025 12:30
@tautschnig tautschnig force-pushed the visual-studio-warnings branch from 03e6856 to 2f3ed10 Compare February 7, 2025 12:31
These are never used (and their implementation was awkwardly summing up
Booleans, yielding conversion warnings).
We can safely read signed values and don't need to defer the type
conversion to the call site.
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.
cmdlinet::parse has a different signature and we don't override it here. As an
alternative option, we could rename functions.
Constructing the arguments in place triggers a warning with Visual Studio. Just
construct them in a fixed order upfront and move each of them.
…em errors

Treat all Visual Studio warnings except those that we cannot do anything about
as errors.
@tautschnig tautschnig force-pushed the visual-studio-warnings branch from ff85b54 to 0ca98d2 Compare February 7, 2025 13:58
@tautschnig tautschnig marked this pull request as draft February 7, 2025 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants