Klaim commented 1 month ago

Godot version

4.3-rc2 4.2.2

godot-cpp version

2bcc4bd23c4e6d7db1f7d5f68be9727ac920c70a

System information

Windows 11, AMD Ryzen 9 5900X, 96GB RAM

Issue description

In my project using godot-cpp, when I have a type using GDCLASS macro (from wrapped.hpp) I get (among other) the following warnings:

warning C4100: 'p_rval': unreferenced formal parameter
warning C4100: 'p_count': unreferenced formal parameter
warning C4100: 'data': unreferenced formal parameter
warning C4100: 'p_instance': unreferenced formal parameter
warning C4100: 'p_token': unreferenced formal parameter
warning C4100: 'p_binding': unreferenced formal parameter
warning C4100: 'p_instance': unreferenced formal parameter
warning C4100: 'p_token': unreferenced formal parameter
warning C4100: 'p_reference': unreferenced formal parameter
warning C4100: 'p_instance': unreferenced formal parameter
warning C4100: 'p_token': unreferenced formal parameter

I removed the source paths because it's just the source file where I defined a node-inheriting class and used GDCLASS as usual, it's irrelevant.

My warning flags are just /W4, which is recommended for new projects (and I dont intend to reduce that level, that warning is useful in my project's code).

Setting the include directory as "system" will not work because this is a macro injecting code in the user's sources.

This seems easy to fix if you either comment these names (which all seems part of the macro) OR by using [[maybe_unnamed]] on them. However, when trying to make a PR , I realized that file might be generated through scripts because of the weird formatting, in which case the issue should be fixed in that generator instead, but I'm not totally sure. If you tell me to just comment the names directly in that header, I'll make a quick PR, otherwise I dont feel confident in fixing this quickly and correctly myself (yet).

Notes:

I can see the same issue with the same build configurations (so same flags) when using v4.2.2 instead of v4.3-rc2 so it's not a regression
This line in tests cmake scripts deactivates the warning about unnamed parameters, essentially hiding the issue which will still appear in user's projects: https://github.com/godotengine/godot-cpp/blob/master/test/CMakeLists.txt#L51
Still in tests cmake scripts, the following lines show that the issue has been intentionally side-stepped? I didnt follow the context so not sure what's happening here. https://github.com/godotengine/godot-cpp/blob/master/test/CMakeLists.txt#L55-L58
I understand that there is an ongoing effort to reduce the warnings coming from the library headers - however here it's coming from a macro which needs to be used in user's source code, which implies that it can never be hidden without the user having to deactivate the warning themselves. Because these warnings seems easy to fix specifically in that macro, I think it's worth fixing these as a separate issue from the other header-warnings issues.

Steps to reproduce

Just adding /W4 to GODOT_COMPILE_FLAGS at cmake configuration using Visual Studio 2022 and building the test/ project will reveal the issue (I did test that locally).

In general, in any user project building using msvc:

set warning level 4 (/W4) when compiling a source file
in that source file, use GDCLASS as expected, for example (from my project):
```
// ...
```

include <godot_cpp/core/class_db.hpp>

include <godot_cpp/classes/node.hpp>

// ... class __symexport MEGAST_Driver : public godot::Node { GDCLASS(MEGAST_Driver, godot::Node) // HERE THE WARNINGS ARE GENERATED public: // ...

};



(`__symexport` does the dllimport/export dance, ignore that)

### Minimal reproduction project

N/A

dsnopek commented 1 month ago

This is a silly warning, but I think it'd be fine to remove the names from the unused arguments in the GDCLASS() macro.

However, when trying to make a PR , I realized that file might be generated through scripts because of the weird formatting, in which case the issue should be fixed in that generator instead, but I'm not totally sure. If you tell me to just comment the names directly in that header, I'll make a quick PR

The wrapped.hpp file is not created by the code generator, so this would just be a matter of making a PR that modifies that file.