2025/01/28 Warnings Hackathon

[cyrush]

Directory Partitioning:

Justin:

• https://github.com/visit-dav/visit/tree/develop/src/databases
  • Blueprint Plugin
• AVT: MFEM an Conduit Data Adaptors

Mark:

• https://github.com/visit-dav/visit/tree/develop/src/databases
  • Silo

Kathleen:

• https://github.com/visit-dav/visit/tree/develop/src/visit_vtk
• https://github.com/visit-dav/visit/tree/develop/src/avt/Plotter

Cyrus:

• AVT generally, State Objects?

[cyrush]

Link to discussion w/ warnings flags:
#19003

[cyrush]

Warnings Encountered:

• -Winconsistent-missing-override
• -Wdeprecated-declarations (sprintf)
• -Wold-style-cast

[cyrush]

options not understood by clang:

warning: unknown warning option '-Wduplicated-cond' [-Wunknown-warning-option]
warning: unknown warning option '-Wduplicated-branches' [-Wunknown-warning-option]
warning: unknown warning option '-Wlogical-op'; did you mean '-Wlong-long'? [-Wunknown-warning-option]
warning: unknown warning option '-Wuseless-cast' [-Wunknown-warning-option]

[JustinPrivitera]

https://stackoverflow.com/questions/332030/when-should-static-cast-dynamic-cast-const-cast-and-reinterpret-cast-be-used#332086

[biagas]

Are we fixing warnings like this where we are accessing elements of a std::vector with an int ?
I have found it isn't always feasible to change the type of the index being used.

warning: conversion to ‘std::vector<double>::size_type’ {aka ‘long unsigned int’} from ‘int’ may change the sign of the result [-Wsign-conversion]
  567 |         else if (val < enumerationRanges[2*mid])
      |                                          ~^~~~

[JustinPrivitera]

Why not static_cast it?

[biagas]

@JustinPrivitera Like this?
else if (val < enumerationRanges[static_cast<size_t>(2*mid)])

Doable, sure, but it gets very verbose and hard to read when the item being cast gets complicated (I have a number of them doing much arithmetic in between the [ ]). I guess I could use temporary variables to hold the arithmetic results.

[JustinPrivitera]

Yes that's what I was suggesting.

If there is any math being done within the [ ] that is done in multiple places than that is a good argument for pulling the calculations out into a new variable. I think it is also usually cleaner to define a new variable for indexing calculations.