Fix shadow variable for enum generation

[JohanBertrand]

Currently, when compiled with -wShadow, the auto generated enums are generating some warnings.

For example:

XEnumAc.hpp: warning: declaration of 'e' shadows a member of 'X::E_X' [-Wshadow]
   71 |             const T e //!< The raw enum value
      |             ~~~~~~~~^
note: shadowed declaration is here
  164 |         T e;
      |           ^
XEnumAc.hpp: warning: declaration of 'e' shadows a member of 'X::E_X'[-Wshadow]
  108 |         bool operator==(T e) const
      |                         ~~^
note: shadowed declaration is here
  164 |         T e;
      |           ^
XEnumAc.hpp: warning: declaration of 'e' shadows a member of 'X::E_X' [-Wshadow]
  114 |         bool operator!=(T e) const
      |                         ~~^
note: shadowed declaration is here
  164 |         T e;
      |           ^

Since the enum value are used in fprime using my_enum.e, I think that it is not recommended to change the member name e to m_e.

However, we could have a prefix/suffix on the function arguments which are shadowing the member value.

For example:

      //! Constructor (user-provided value)
      E(
          const T e //!< The raw enum value
      )
      {
        this->e = e;
      }

      /* ... */

      //! Equality operator
      bool operator==(T e) const
      {
        return this->e == e;
      }

      //! Inequality operator
      bool operator!=(T e) const
      {
        return !(*this == e);
      }

With the a_ prefix (for "argument") would become:

      //! Constructor (user-provided value)
      E(
          const T a_e //!< The raw enum value
      )
      {
        this->e = a_e;
      }

      /* ... */

      //! Equality operator
      bool operator==(T a_e) const
      {
        return this->e == a_e;
      }

      //! Inequality operator
      bool operator!=(T a_e) const
      {
        return !(*this == a_e);
      }

I am open to suggestion if there is a better option/a better prefix or suffix.

[bocchino]

I think the easiest fix is to rename e to e1 everywhere it appears as the name of a function parameter.

[JohanBertrand]

Alright, I made the changes and created a pull request.

However, I can see that the same problem can happen with the files generated by the StructCppWriter file, where the different members are having the same name as the function argument.

Adding a suffix here might would not work, because a member variable called m1 would possibly shadow a member called m11. This case is happening on the unit tests.

I guess it could be fixed with the m_ prefix for those member variables, since they are protected, but I'm not sure of the impact of such changes on FPrime.

[bocchino]

However, I can see that the same problem can happen with the files generated by the StructCppWriter file

I suspect this issue may be all over F Prime, because we don't normally compile with -Wshadow. Also, we do use the idiom of this->x (member variable) and x (function parameter), which -Wshadow doesn't like. If we want the F Prime code base to be -Wshadow-compliant, then we should add -Wshadow to the build rules and fix all the resulting warnings.

I think this is a larger style issue with F Prime code: do we want to be -Wshadow compliant?

@LeStarch FYI

[bocchino]

If we do want to be -Wshadow compliant, then adding a prefix like m_ to the member variable names seems reasonable.

[bocchino]

Right now it looks like we comply with -Wall and -Wextra. There are many other warning flags we could add, -Wshadow being one of them. Probably the right way to introduce a change like this is to propose to the F Prime team to include -Wshadow in the list of warning flags that we comply with. Is there something special about -Wshadow vs. the other flags we could add?

[bocchino]

Addressed in #327.