FLTK logo

STR #3354

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 | SVN ⇄ GIT ]

STR #3354

Application:FLTK Library
Status:1 - Closed w/Resolution
Priority:2 - Low, e.g. a documentation error or undocumented side-effect
Scope:2 - Specific to an operating system
Subsystem:Core Library
Summary:v1.4: Include file discrepancy to v1.3
Version:1.4-feature
Created By:chris
Assigned To:AlbrechtS
Fix Version:1.4.0
Fix Commit:44bb080c0ff81b16d48dccd8d15809f058cc68ea
Update Notification:

Receive EMails Don't Receive EMails

Trouble Report Files:


Name/Time/Date Filename/Size  
 
#1 chris
05:37 Nov 20, 2016
remove_friend_fl_from_fl_window.diff
4k
 
     

Trouble Report Comments:


Name/Time/Date Text  
 
#1 chris
10:39 Nov 18, 2016
Compiling the following program:

#include <FL/Fl_Double_Window.H>
int main() {
  return Fl::run();
}

gives no error in FLTK-1.4,

wheras with FLTK-1.3 you get "Fl has not been declared", which is correct.
 
 
#2 AlbrechtS
11:21 Nov 18, 2016
Thanks for the report.

If it works in FLTK 1.4 it's not a regression. Does the program compile, link, and run? I think yes. I just tested with FLTK's test/hello.cxx by removing the include statement for FL/Fl.H and it worked. So, is this a problem? I don't think so.

The only point I can think of is that it might be documented that you always need to

  #include <FL/Fl.H>

and here it works without that explicit include file.

That said, include files and include requirements are tricky. IMHO we should only include _other_ headers in a header file if they are really needed. Requirements and dependencies may have changed in FLTK 1.4 so that we now include FL/Fl.H in Fl_[Double_]Window.H.

The downside of this is: if we _remove_ included "other" headers in FLTK 1.4 (because they were included in 1.3, but not used, or because they are no longer needed in 1.4), then we have some kind of regression, because user programs that relied on this inclusion didn't include that "other" header in their code.

Back to the example:

- FL/Fl_Double_Window.H includes FL/Fl_Window.H
- FL/Fl_Window.H includes FL/Fl.H

The same is likely true for all other Fl_*_Window header files, so that we can say that you only need to #include one Fl_*_Window header file, maybe as the first FLTK header, and don't need to include FL/Fl.H in 1.4 ...

Or am I missing anything?
 
 
#3 chris
12:10 Nov 18, 2016
It means that FLTK 1.4 is not entirely source compatible with FLTK 1.3, because a source compilable with FLTK 1.4 could not compile under 1.3 (that was, what actually happened to me earlier).

If this inclusion of Fl.H in Fl_Window.H it shows that that new dependencies have been introduced. In this case there is friend class declaration what it is needed for.

Sometimes it pays to take care of such indicators in order to get more clean code in the end.
 
 
#4 chris
05:36 Nov 20, 2016
Uploaded a patch that resolves this issue and makes the interface cleaner.

Notes to the patch:

- It uses the same strategy, that was already present for 'use_high_res_GL'

- It resolves the issue, but could be improved even more,
  by now moving the static Fl_Window::show_iconic_ to a
  private variable of Fl_Window.

- It 'uncovered' that Fl_Gl_Window.cxx did not include Fl.H
 
 
#5 AlbrechtS
15:57 Feb 02, 2023
Working on this now...

First of all: I changed the docs that it is not (always) required to #include <FL/Fl.H> since FLTK 1.4.0 (as opposed to 1.3.x).
Git commit: 32b6c04bcf4f9977b38a15e84739e8395f57e97b

The fact that programs that don't include this header work with FLTK 1.4.0 is not a regression, and that's sufficient because we only need to be backwards compatible, i.e. programs written for (and working with) FLTK 1.3.x should be compilable and work with FLTK 1.4.0.

That said, I'm looking for some FLTK headers that #include other unnecessary header files. I'm also evaluating the provided patch but I don't think that it will be applied as it is. As I said, still evaluating...
 
 
#6 AlbrechtS
07:59 Nov 17, 2023
Fixed in Git repository.

Although the given patch was not strictly necessary I applied it in a slightly modified way. The result is the same: the 'friend' declaration was removed.

@chris: thanks for the patch which I used partially.

The issue with include files has been explained and documented and will not be changed.
 
     

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'.