BRL-CAD's sources are compiled with a number of build flags including numerous warning flags. By default, we treat all warnings as errors and we turn on nearly every warning that the compiler is capable of producing and then we fix them. When you do this systematically and consistently, it can greatly improve code quality. Help us improve BRL-CAD's code quality even more. We already compile with the following gcc/clang warning flags:
-pedantic -Wall -Wextra -Wundef -Wfloat-equal -Wshadow -Winline -Wno-long-long -Wno-variadic-macros -Werror
This task involves enabling any new warning flag that is not already explicitly enabled or enabled by default, and then fixing the warnings that result.
Post a patch for review that enables the new flag, fixes at least one warning, and post a full build log (with "make -k" so we can get an estimate of how many issues need to be fixed).
You can enable the build flag by adding a CHECK_C_FLAG(your_flag_here) line in:
- misc/CMake/BRLCAD_CompilerFlags.cmake
Add your line after the "Wall" checks (search for Wall) similar to the numerous other flags being tested. Don't worry about setting it as a C++/CXX flag unless you're feeling adventurous. See the GCC or Clang manual page for a list of warning flags that can be enabled but some unconfirmed possibilities include:
- Wcast-qual
- Wredundant-decls
- Wunreachable-code
- Wmissing-declarations
- Wmissing-prototypes
- Wstrict-prototypes
- Wdocumentation (for Doxygen comments)
- Wfour-char-constants
- Wtraditional
- Wbad-function-cast
- Wc++-compat
- ... many many others ... really, see the GCC manpage and try enabling one
How to approach this task? Add your flag, make sure the build halts because something was detected, then run "make -k" to see just how many were detected. Then try to fix just one of them. If you are successful fixing one of them, submit your make -k log and a patch that fixes one and turns on the flag and get started on fixing the rest.
You'll be expected to fix some portion of the warnings reported, but this will be determined on a case-by-case basis depending on how complicated the warnings are and how many there are.
| File name/URL | File size | Date submitted | |
|---|---|---|---|
| flag_change.patch | 60.3 KB | January 04 2014 23:51 UTC | |
| build.log | 3.7 MB | January 05 2014 00:01 UTC |
I would like to work on this task.
This task has been assigned to Johannes Schulte. You have 144 hours to complete this task, good luck!
While I'm not sure, if just deleting the duplicated usages is the right way, we do now have at least a summary of all duplicated declerations. The least satifisfied I am with the hack for table.c. Nevetheless, I think there is a deeper issue, because of which some functions need to be already defined in raytrace.h, instead of table.c, so this will require bigger changes, not related to this task.
The work on this task is ready to be reviewed.
I also have a question, concerning this task: http://www.google-melange.com/gci/task/view/google/gci2013/5836041357361152 . At the moment, all the calculation for the bot is done inside analyze.c. Because of this, the analyze command can display the surface area for each face of the bot. Nevertheless, it doesn't take plate mode in account, so in this cases the calculations are wrong. Can I give up this functionality of listing the faces areas, and instead use the function, I implemented last year from librt?
Congratulations, this task has been completed successfully.
Don't commit this patch just yet, but it's more than enough work for a gci task so it can be closed. This will take some extensive review and care, particularly on windows where declarations serve a different role than they do on other platforms. Looks 95% okay, but there's 5% that looks like a problem too.
To answer your question, we can give up the functionality of listing areas per-face. That feature was only ever useful for trivial meshes anyways. I don't know that it was ever even used.