Warnings shadowing variables

Question

Warnings shadowing variables

bkaradzic opened this issue 8 years ago · 13 comments

There are bunch of warnings about shadowing variables (add -Wshadow to Makefile).

Example:

ast.h: In constructor ‘glsl::astMemory::astMemory(T*)’:
ast.h:18:24: warning: declaration of ‘data’ shadows a member of ‘glsl::astMemory’ [-Wshadow]
     astMemory(T *data) : data((void*)data), dtor(&astDestroy<T>) { }
                        ^
ast.h:19:11: note: shadowed declaration is here
     void *data;

I can send PR with fix for these, but not sure what you would prefer for naming them?

Answer 1 · 2016-08-23T16:34:13.000Z

This is a silly warning

Answer 2 · 2016-08-25T05:04:57.000Z

This warning is default in VS2015 when building with warning level 4, adding it to GCC makes both compilers produce similar warnings, and helps maintain no-warnings.

Answer 3 · 2016-08-25T06:32:14.000Z

in general I'd like to cleanup glsl-parser to use m_ prefix on member variables but I haven't gotten around to that.

Answer 4 · 2016-08-25T15:46:27.000Z

Cool, that would fix it. Another cleanup suggestion is to typedef all std::vector usage, that way std::vector can be swapped with something that has compatible interface in one place. For example typedef std::vector<chars> Buffer; or something similar.

Answer 5 · 2016-08-26T07:03:31.000Z

Took your idea regarding std::vector and added a wrapper around it in util.h. If you have C++11 you can at least easily use your own by doing something like

template <typename T>
using vector = myVector<T>;

If not you should beable to replace the one instance of std::vector in the wrapper
with myVector to achieve the same thing in anything < C++11.

Answer 6 · 2016-08-26T16:05:59.000Z

Actually my idea was slightly simpler.

typedef std::vector<char> Buffer; then I can change that typedef to use different implementation of STL I provide, typedef someother_stl::vector<char> Buffer;.

Anyhow your change does the same.

Answer 7 · 2016-08-26T18:24:56.000Z

the problem with that is I have more than just one instantation of a vector with a type. The proper solution is

template <typename T>
using vector = std::vector<T>;

But that is C++11. This wrapper is the only other technique I could think of to do it. Your method would require a typedef for every possible instantation.

Answer 8 · 2016-08-26T18:31:42.000Z

Yeah, I meant you typedef them all. :)

Like:
typedef std::vector<astLayoutQualifier*> AstLayoutQualifierVector;

And then instead:
vector<astLayoutQualifier*> &qualifiers = variable->layoutQualifiers;

In the rest of the code you just write:
AstLayoutQualifierVector &qualifiers = variable->layoutQualifiers;

Answer 9 · 2016-08-26T20:06:15.000Z

That is too tedious and would require more changes than the wrapper :)

Answer 10 · 2016-08-26T21:31:14.000Z

I can do the work and send you PR... :)

Answer 11 · 2016-08-26T21:32:22.000Z

Btw, I really like this part about library:

Portable and embeddable

    Written in portable C++03.
        Only uses std::vector from the standard library
    Exception free
    Doesn't use virtual functions
    Small (~90 KB)
    Permissive (MIT)

Answer 12 · 2016-08-26T21:33:18.000Z

It's Orthodox C++ compliant: https://gist.github.com/bkaradzic/2e39896bc7d8c34e042b :)

Answer 13 · 2016-08-27T04:49:00.000Z

That is how C++ is supposed to be written. I too share very similar regards to this "modern C++" world. I have a style guide I use that defines this stuff too: https://github.com/graphitemaster/neothyne/blob/master/docs/STYLE.md