FLTK logo

STR #2728

FLTK matrix user chat room
(using Element browser app)   FLTK gitter user chat room   GitHub FLTK Project   FLTK News RSS Feed  
  FLTK Apps      FLTK Library      Forums      Links     Login 
 Home  |  Articles & FAQs  |  Bugs & Features  |  Documentation  |  Download  |  Screenshots  ]
 

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:

Receive EMails Don't Receive EMails

Trouble Report Files:


Name/Time/Date Filename/Size  
 
#1 greg.ercolano
23:38 Nov 12, 2011
shadow-variables-test-program.patch
1k
 
 
#2 greg.ercolano
23:45 Nov 12, 2011
shadow_variables.cxx
4k
 
 
#3 greg.ercolano
13:06 May 11, 2013
Makefile.patch
2k
 
 
#4 greg.ercolano
13:21 May 11, 2013
Makefile-v2.patch
2k
 
 
#5 greg.ercolano
16:21 May 11, 2013
Makefile-v3.patch
2k
 
     

Trouble Report Comments:


Name/Time/Date Text  
 
#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 ]

 
 

Comments are owned by the poster. All other content is copyright 1998-2024 by Bill Spitzak and others. This project is hosted by The FLTK Team. Please report site problems to 'erco@seriss.com'.