Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

-Wall -Wextra has tons of warnings #8

Open
rcurtin opened this issue Dec 4, 2014 · 5 comments · Fixed by #9
Open

-Wall -Wextra has tons of warnings #8

rcurtin opened this issue Dec 4, 2014 · 5 comments · Fixed by #9

Comments

@rcurtin
Copy link
Contributor

rcurtin commented Dec 4, 2014

Suppose that I am using CHDL in something else, and I am interested in guaranteeing that the code doesn't have warnings.

This can be easily reproduced by setting CXXFLAGS="-Wall -Wextra" and typing 'make'. Here is an excerpt of the output:

g++ -Wall -Wextra -fPIC -std=c++11 -O2    -c -o hierarchy.o hierarchy.cpp
In file included from regimpl.h:5:0,
                 from hierarchy.cpp:14:
tickable.h:20:18: warning: unused parameter âcdâ [-Wunused-parameter]
tickable.h:21:18: warning: unused parameter âcdâ [-Wunused-parameter]
tickable.h:22:18: warning: unused parameter âcdâ [-Wunused-parameter]
tickable.h:23:18: warning: unused parameter âcdâ [-Wunused-parameter]
In file included from hierarchy.cpp:14:0:
regimpl.h:20:10: warning: unused parameter âcdâ [-Wunused-parameter]
hierarchy.cpp: In constructor âhierarchy::hierarchy(std::string, chdl::hpath_t)â:
hierarchy.cpp:55:10: warning: âhierarchy::nameâ will be initialized after [-Wreorder]
hierarchy.cpp:54:11: warning:   âchdl::hpath_t hierarchy::pathâ [-Wreorder]
hierarchy.cpp:21:3: warning:   when initialized here [-Wreorder]
hierarchy.cpp: In member function âvoid hierarchy::print(std::ostream&, int, unsigned int)â:
hierarchy.cpp:30:33: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]

This bug is important because users expect their code to compile cleanly without warnings, especially upstream projects that are subject to distribution requirements for -Wall and -Wextra.

@ZoogieZork
Copy link

FWIW, I do agree about the -Wreorder warnings.

@rcurtin
Copy link
Contributor Author

rcurtin commented Dec 4, 2014

A couple other warnings I found include -Wunused-function (probably does not matter), -Wdelete-non-virtual-dtor which is probably worth looking into, a few -Wsign-compare which are a bit pedantic but probably worth your time, and a -Wreturn-type for a function that could end without returning anything. Maybe they matter, maybe they don't, but hey, I helped open source, right? Or something.

@cdkersey
Copy link
Owner

cdkersey commented Dec 8, 2014

Oops. Didn't mean to close it. Not going to fix this, especially not for -Wextra, but not going to close this either. I'll let it smolder and see what kinds of fixes for warnings we accumulate. Some of these are doubleplus annoying because they appear in the default warning sets on strange compilers/platforms.

@cdkersey cdkersey reopened this Dec 8, 2014
@rcurtin
Copy link
Contributor Author

rcurtin commented Dec 20, 2014

From #9, this comment wasn't addressed:

I fixed some more -Wunused because why not. There are a few statements of the form

if(type* r = dynamic_cast<type*>(something));

with r unused. Are these equivalent to if(dynamic_cast<type*>(something))? I didn't test.

Depending on your assessment I'll happily make the gruntwork changes.

@cdkersey
Copy link
Owner

Are these equivalent to if(dynamic_cast(something))?

Yes. Feel free to remove the unused variable declarations from those predicates. The only reason I can fathom they're there is that they were used for debugging during development and left in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants