cyclonedds-cxx icon indicating copy to clipboard operation
cyclonedds-cxx copied to clipboard

unused variable with bounded sequences

Open ghorn opened this issue 3 years ago • 3 comments

When I run the C++ idlc plugin on IDLs containing bounded sequences, I get unused variable warnings when compiling the results with Clang:

error: unused variable 'a_1' [-Werror,-Wunused-variable]
      for (const auto & a_1:instance) {  //array depth 1

When I look at a little more context, I see that there is a nearby related unused variable that is already suppressed:

template<typename T, std::enable_if_t<std::is_base_of<cdr_stream, T>::value, bool> = true >
bool MyType(T& streamer, const MyType& instance) {
    (void)instance;
        if (!streamer.start_consecutive(true, false))
            return false;
        for (const auto & a_1:instance) {  //array depth 1
        if (!streamer.start_consecutive(false, true))
            return false;
        {

Notice that the unused variable instance is suppressed. That makes me think that a_1 this is expected to be unused, and that the issue is that there is a missing (void)a_1 as opposed to a real bug.

ghorn avatar Sep 14 '22 05:09 ghorn

Hello @ghorn , sorry for the late response.

Could you give an example of the IDL that gives these warnings?

reicheratwork avatar Nov 24 '22 15:11 reicheratwork

I don't have my development computer with me, so this is from memory, but try this out:

struct Foo {
  sequence<float, 22> bar;
};

Compile with clang and -Wunused-variable.

ghorn avatar Nov 24 '22 19:11 ghorn

Some explanations on the code generator:

  • the approach taken in the first place was to make the code generator as simple as possible while still generating functional code
  • because of this approach, there is just the line (void) instance; added to the beginning of each streaming function to suppress compiler warnings if instance is not used in the function, which might be the case with an empty struct
  • also because of this approach for some of the streaming functions, when iterating over a range of entities (like in an array, or a sequence), but when you don't need to know the contents of the entities, only their type, (like in the max streaming functions), just a default constructed entity is used in stead of the "real" contents of the array/sequence, allowing more optimization
  • so while there are warnings, these can mostly be ignored, and in the future we of course will attempt to generate warningless code

Also, somehow the code in your second post does not generate the example you show in your first post. Looking at the generated code in your first post, I can make the following (quite general) conclusions:

  • the part bool MyType on the second line is probably missing a "read/write/move/max" part, so you probably missed that while anonimising the code after copying it
  • there is instance directly inside the range based for loop, this only occurs if the function you showed in your first post is a typedef streaming subroutine, and there are no typedefs in the code in your second post
  • the range-based for loop has as the iterated entity's name a_N indicating that you're iterating over an array, and there are no arrays in your second post's example

Also, I don't know which commit of CycloneDDS-CXX caused this issue on your side. Maybe getting the newest version of the code will give you some improvements?

reicheratwork avatar Nov 25 '22 09:11 reicheratwork