Modified gcc/clang warning suppression macros to account for some warnings flags being supported by one compiler but not the other

[seanm]

Added const where possible, then removed casts.

[seanm]

@qkoziol I managed to fix a few const warnings... but anything more I think needs someone that knows the codebase well.

[gnuoyd]

It looks like you are changing a public API here, H5Literate2, such that the non-const-ness of the op argument disagrees with the const-ness of the op_data. I think it's too late to change the API, and introducing blatant const disagreement is undesirable.

Maybe some of the const/non-const disagreements that prompted this PR can be avoided in another way? For example, you could create a container for a const pointer, struct t_container { const t *ptr; };, and pass a non-const pointer to the container into the iteration routine. The callback would need to be changed to expect a pointer to struct t_container, of course. There are other ways to solve the problem, this is just one idea that comes to mind.

Thanks for looking at these const-discarding statements, it will be nice to be rid of the warnings (and bugs).

[jhendersonHDF]

It looks like you are changing a public API here, H5Literate2, such that the non-const-ness of the op argument disagrees with the const-ness of the op_data. I think it's too late to change the API, and introducing blatant const disagreement is undesirable.

Definitely agreed on this point. The op_data pointer is meant to be non-const and there are several tests (currently external to HDF5) that rely on this behavior. For example, consider the case of passing a local variable for op_data that is used to count the number of links that have been iterated over. While it might eliminate the const warnings from HDF5, this would simply push the const warning down to those tests instead.

[qkoziol]

@qkoziol I managed to fix a few const warnings... but anything more I think needs someone that knows the codebase well.

The op_data must remain non-const (as @gnuoyd and @jhendersonHDF mention), so the problem is in the caller, not the iterator routines themselves. @gnuoyd's solution of creating a trivial wrapping container is a reasonable way to address it up there.

[seanm]

OK, I gave up on solving any of those warnings. Instead, I at least fixed the suppression of a few of them with clang. See new commit.

[derobins]

These macros are starting to get ugly

[seanm]

Yes, but having hundreds of warnings about trying to suppress non-existent warnings is even uglier IMHO.

[derobins]

Hi Sean,

I've given this some thought, and I don't think that adding things like H5_GCC_CLANG_DIAG_OFF is the way to go. I have a more generic solution in mind that will also work for platforms like Visual Studio. Let's leave this up, though, while I put that together.

[seanm]

Fixing the warnings instead of suppressing them is another nice option :)

[seanm]

@derobins any word on your alternative? It would be nice to have something, or merge this, because building with clang currently has many warnings about 'unknown pragmas' every time a gcc warning flag that clang doesn't know is disabled with the existing pragmas.

[seanm]

@derobins @gnuoyd @qkoziol I've rebased this. Could you review again? It's really annoying getting so many warnings with clang about trying to suppress gcc warnings that clang doesn't support.

[gnuoyd]

This looks like a step in the right direction.

[seanm]

So that's 2 approvals if I'm not mistaken... anything else needed to get this merged?

[seanm]

Thanks!