search

gs1 / gs1-syntax-engine

GS1 Syntax Engine
Apache License 2.0
27 stars 5 forks source link

const/non-const differences in c/h files #1

Closed schoebey closed 10 months ago

schoebey commented 10 months ago
schoebey commented 10 months ago

0001-fix-missing-const-const-correctness.patch

terryburton commented 10 months ago

Thank you for the patch, however it is misguided. In C/C++ top-level "const" qualifiers are ignored in function declarations (but not in function definitions).

A "const" in the function definition says that the implementation will not modify the value of the variable to which the function parameter is copied. The mechanics of the function implementation are not within the purview of the public header: Function implementations are free to change their internals provided that the function prototype remains the same as is declared in the public header. That's why "top-level consts" are ignored in declarations, because they would otherwise place restrictions on the internals of the function and not on its usage, i.e. are simply out of scope.

After producing the list of parameter types, any top-level cv-qualifiers modifying a parameter type are deleted when forming the function type.

This generally means that you won't see "const " in function declarations, especially public APIs (like gs1_encoders.h). It doesn't tell the user anything relevant to their use of the function, and must therefore be ignored by the compiler.

Older versions of Visual Studio had a bug where they failed to remove top-level consts from function declarations before evaluating the functions type. Mature C/C++ toolchains respect the standard.

schoebey commented 10 months ago

Older versions of Visual Studio had a bug where they failed to remove top-level consts from function declarations before evaluating the functions type. Mature C/C++ toolchains respect the standard.

I was wondering why VS was complaining about this. Since only older versions of VS are affected by this, it makes sense to leave it as-is.

This leaves us with either having to patch the 'consts' in, or changing the compiler flag "/WX" to "/WX-". for our local build.

Changing the signature of gs1_encoder_getDLuri would be helpful though :)

terryburton commented 10 months ago

This leaves us with either having to patch the 'consts' in, or changing the compiler flag "/WX" to "/WX-". for our local build.

I would suggest adding a suppression for the specific issue, preferably just for older versions of MSVC. This would allow you to leave /warnaserror intact. But I'm not too familiar with the MSVC toolchain so can't advise how to do this.

Changing the signature of gs1_encoder_getDLuri would be helpful though :)

Agreed. Pushed.