Moving _values_manually_retrieved to after _get_array in PetscVector

[zachmprince]

Closes #3493

[moosebuild]

Job Coverage on 95a66d9 wanted to post the following:

Coverage

	f630ba	#3494 95a66d
	Total	Total	+/-	New
Rate	60.02%	60.02%	-	0.00%
Hits	48864	48864	-	0
Misses	32545	32545	-	3

Diff coverage report

Full coverage report

Warnings

• New new line coverage rate 0.00% is less than the suggested 90.0%

This comment will be updated on new commits.

[jwpeterson]

I see that you updated the unit test, but CIVET reports no coverage of the new lines of code. It's possible that's an error with the coverage itself, but probably good to confirm that we know exactly what's going on before merging...

[YaqiWang]

@loganharbour or @roystgnr any comments about @jwpeterson 's comment? I checked the full coverage report, which does not make sense on this change.

[loganharbour]

It's not hit in the current coverage either: https://mooseframework.inl.gov/libmesh/docs/coverage/include/numerics/petsc_vector.h.gcov.html

I would not be surprised here if we are missing coverage because we're running with -O2

[jwpeterson]

OK, so this patch fixes a correctness issue that is only relevant in debug-mode (or less aggressively optimized) binaries. That sounds good to me, but I think it's more interesting that the compiler can actually make this optimization... setting a class member boolean seems like an "observable side effect" that would not be considered a candidate for elision.