| [ Return to Bugs & Features | Roadmap 1.3 | SVN ⇄ GIT ]
STR #2728
Application: | FLTK Library |
Status: | 1 - Closed w/Resolution |
Priority: | 1 - Request for Enhancement, e.g. asking for a feature |
Scope: | 3 - Applies to all machines and operating systems |
Subsystem: | Test Framework |
Summary: | Add test program to test for shadowed variables (-Wshadow) |
Version: | 1.3-feature |
Created By: | greg.ercolano |
Assigned To: | AlbrechtS |
Fix Version: | 1.4.0 |
Fix Commit: | 3fc8875756c3ce02b1b45bd0383a0c307c442dde |
Update Notification: | |
Trouble Report Files:
Trouble Report Comments:
|
#1 | greg.ercolano 09:13 Oct 04, 2011 |
| To test for problems with variable shadowing (for apps that compile against FLTK with e.g. -Wshadow), we could make one test program that #include's all fltk widgets, instancing them once, and have just that test program compile with -Wshadow (or equivalent depending on the compiler)
This way future builds will show any shadow errors in our .H's as part of the build. | |
|
#2 | greg.ercolano 23:44 Nov 12, 2011 |
| Attached a test/Makefile (shadow-variables-test-program.patch) patch to generate the shadow_variables.cxx automatically, based on all the include'able FL/*.H files, with some exceptions.
By automatically generating the shadow variable test program, devs don't have to modify the test program every time a new #include file is added. However, this patch uses a one-liner perl script to generate the file, also creating perhaps an unwanted dependence on perl.
Will follow up with an attachment of the 'generated' shadow-variables.cxx file, just to show what it should look like. I suppose the alternative to the above automated approach is to check in this .cxx file, and have devs add all new .H files to it.
The goal here is to have a build of FLTK fail if there are any shadowing variables in any of our .H files. | |
|
#3 | greg.ercolano 13:05 May 11, 2013 |
| Attaching a patch (Makefile.patch) for test/Makefile that implements the shadow variable test.
The Makefile target generates the program's source code on the fly based on the current headers in FL, and compiles it with the -wshadow flag (if the local $CXX compiler is g++. If not, the test is skipped). So if there are any shadow conflicts, the build stops with the error.
I don't want to check this in yet, as we currently still have shadow variable errors; see recent follow up in STR #2714.
Suggest fixing that before checking this in. | |
|
#4 | greg.ercolano 13:21 May 11, 2013 |
| Attaching small revision: Makefile-v2.patch.
Added '-' to ensure compiler doesn't stop the build process. Also, small mods to the Makefile comments. | |
|
#5 | AlbrechtS 14:23 May 11, 2013 |
| Index: test/Makefile =================================================================== --- test/Makefile (revision 9864) +++ test/Makefile (working copy) @@ -80,6 +80,7 @@ resize.cxx \ rotated_text.cxx \ scroll.cxx \ + shadow_varaibles.cxx \ ------------------^^
should read .. shadow_variables.cxx | |
|
#6 | greg.ercolano 16:21 May 11, 2013 |
| Thanks Albrecht; fixed in Makefile-v3.patch.
Also in v3, converted the perl commands to awk, because perl is not standard in unix, whereas awk is.
Tested on Mac and linux, but not yet with windows mingw/msys suite.. | |
|
#7 | greg.ercolano 16:36 Oct 27, 2014 |
| The fact this RFE has not been implemented yet should not hold up a 1.3.3 release. | |
|
#8 | AlbrechtS 08:42 Apr 18, 2024 |
| Fixed in Git repository.
Fixed in commit bb45198413ef8efe236afbd665a4623239ac0da0 and updated in 3fc8875756c3ce02b1b45bd0383a0c307c442dde.
I decided to add my own test/shadow_variables.cxx as a new test program only in CMake builds. This should be sufficient to detect warnings early. I deliberately decided not to add this to configure/make builds because that will be dropped in 1.5 anyway.
The header file 'include_all.h' is generated by CMake dynamically during the CMake configure phase. There's no shell involved (no perl, no awk etc.).
Side note: I detected two minor issues with this new feature and fixed them in commit 2c21e520f4859904739deb7d058b084f314ed47d *before* I added this new feature.
Thanks to Greg for the inspiration. | |
[ Return to Bugs & Features ]
|
| |