Fix all uninitialized variable warnings (C26495)

[Pokechu22]

This fixes all occurrences of C26495 within Dolphin code, by adding initializers for everything. I have mixed feelings about this approach, but it seems like there isn't any simpler approach (a default constructor won't work for instance).

Note that C26495 only shows up when Build → Run Code Analysis on Solution is used; it doesn't show up with a normal build. (Also note that after using Build → Run Code Analysis on Solution, a clean build needs to be used because the precompiled headers are incompatible.)

See #9956 (comment) for an example of where a value not being initialized caused problems.

[sepalani]

Some remarks:

• Is there a reason to initialise some integers/bool/float types with {} where as in other places it uses an assignation =?
• Can't braces be used everywhere to keep it consistent?
• Should float be initialised with 0.0f rather than 0 or false?

[BhaaLseN]

Changes seem mostly fine, but I'm a bit worried about consistency. Some places use assignments, others use uniform initialization for the same thing; and mixing those two all over the place may lead to more inconsistencies down the road (depending on what file one looks at for inspiration).
Then again, this is likely something we should first address in the code style docs before we force one over the other on a whim.

I do wonder about those int i = false, float f = false etc. though that sepalani pointed out, seems like copy&paste gone wrong.

[iwubcode]

Throughout most of the files you used bool x = false or int x = 0. Here you use bool x{} and int x{}. Can we make this consistent?

[Pokechu22]

I've changed them to = false or = 0.

[sepalani]

Reviewed 1 of 2 files at r1, 110 of 110 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Pokechu22)

[sepalani]

I don't think making this PR stale for too long is a good idea because it will be a pain to maintain.

PR tested on Windows debug build with some games. LGTM besides these 2 nitpicks.

[sepalani]

Can't it use std::array<T, N>* instead as it doesn't sound compliant with Dolphin's coding style?

[Pokechu22]

I was mostly modeling it around ChunkFile (the magic code used for save-states), but probably it's better to not do that. ChunkFile needs to be magic, but IOFile doesn't.

[sepalani]

Is it necessary to move them back in the header file?

[Pokechu22]

Rebased and updated. I also had to replace TryReadResult with std::optional in CheatSearch since #9886 was merged; that commit should be re-reviewed.

[sepalani]

LGTM

[BhaaLseN]

Did we miss those two, or are they set elsewhere?

Plus, it would probably be a good idea to make this into two lines while we're at it.

[Pokechu22]

They're set in the constructor. But so are a few other things which I did initialize - I'll move those values out of the constructor.

[Pokechu22]

Note: The fifoci comment is because for some reason it compared with 5.0-15257 even though this PR was based on 5.0-15259 (#10138). The more recent comparision has no differences (though the same things still fail to render, which is expected), but fifoci doesn't delete old comments if there are no changes.

[leoetlino]

Reviewed 112 of 112 files at r4, 1 of 1 files at r5, 3 of 3 files at r6, 111 of 111 files at r7, all commit messages.
Dismissed @sepalani from a discussion.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @BhaaLseN and @Pokechu22)

---

Source/Core/InputCommon/ControlReference/ControlReference.h, line 49 at r7 (raw file):

  ControlReference();
  std::string m_expression;
  std::unique_ptr<ciface::ExpressionParser::Expression> m_parsed_expression = nullptr;

This is unnecessary; a unique_ptr defaults to being empty (std::unique_ptr<T>() == nullptr is true).

[leoetlino]

Reviewed 114 of 114 files at r8, 1 of 1 files at r9, 3 of 3 files at r10, 1 of 111 files at r11, all commit messages.
Reviewable status: 4 of 114 files reviewed, 1 unresolved discussion (waiting on @BhaaLseN and @leoetlino)

[shuffle2]

@Pokechu22 I just looked for uninit vars the same way again. Among a few we don't really care about (but probably should just initialize and let the compiler deal with dead store elimination), there's also:

Severity	Code	Description	Project	File	Line	Suppression State	Detail Description
Warning	C6001	Using uninitialized memory 'scanline'.	DolphinLib	C:\src\dolphin\Source\Core\VideoBackends\Software\EfbInterface.cpp	615	

It has correctly detected that if source_rect.left >= source_rect.right, then body of for (int i = 1, x = left; x < right; i++, x++) won't be executed, and scanline won't be initialized. Is that a condition we know never can happen (particularly the equals case)? Should something be done about it?

[Pokechu22]

It's impossible for left >= right since the copy size is specified as a width/height (and those are unsigned, with a value of 0 indicating 1 pixel). See this code (which also has a rather dubious comment):

dolphin/Source/Core/VideoCommon/BPStructs.cpp

Lines 225 to 228 in e627718

	// Here Width+1 like Height, otherwise some textures are corrupted already since the native
	// resolution.
	srcRect.right = bpmem.copyTexSrcXY.x + bpmem.copyTexSrcWH.x + 1;
	srcRect.bottom = bpmem.copyTexSrcXY.y + bpmem.copyTexSrcWH.y + 1;

So, even if a game for some reason decided that the XFB should be as thin as possible, it'd still render correctly (at least here; I don't know what hardware will actually do when you try to put a 1-pixel wide framebuffer to screen, but that'd be a VideoInterface problem I think).

[shuffle2]

OK, thanks for pointing out caller.

unrelated, but interesting that their source is 10 bit integers...wonder what happens on 10bit overflow or so (which we wouldn't detect afaict) :p

[Pokechu22]

Well, the max texture size is 1024 by 1024, which is the biggest size for a 10-bit value, but the EFB itself is 640 by 528, so a full-sized one would include out of bounds data. I suspect that an overflow would just wrap around (that's roughly what happens without of bounds scissor values, though that also has wrapping at 2048 for other aspects), but I don't know what actually happens in the 640-1023 region (though apparently that sometimes does happen with Mario Kart Wii). Dolphin currently clamps it to stay within the 640 by 528 region, I guess leaving out-of-bounds memory untouched, so that overflow wouldn't cause any problems.