STR #1859

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 | Post Text | Post File | Prev | Next ]

STR #1859

Application:FLTK Library
Status:5 - New
Priority:3 - Moderate, e.g. unable to compile the software
Scope:3 - Applies to all machines and operating systems
Subsystem:FLUID
Summary:Fluid callbacks in declaration blocks not working correctly
Version:1.4-feature
Created By:djl
Assigned To:Unassigned
Fix Version:Unassigned
Update Notification:

Receive EMails Don't Receive EMails

Trouble Report Files:

Post File ]
Name/Time/Date Filename/Size top right image
 
#1 djl
08:17 Jan 17, 2008
test.fl
1k
 
 
#2 djl
05:34 Jan 23, 2008
Fl_Widget_Type.diff
2k
 
 
#3 wavexx
07:38 Feb 21, 2008
t1859.fl
1k
 
 
#4 wavexx
12:02 Feb 24, 2008
nesteddecls.patch
3k
 
 
#5 djl
03:13 Feb 25, 2008
test_v2.fl
1k
 
 
#6 djl
03:15 Feb 25, 2008
test.cxx
4k
 
bottom left image   bottom right image

Trouble Report Comments:

Post Text ]
Name/Time/Date Text top right image
 
#1 djl
08:17 Jan 17, 2008
I think fluid does not correctly write the code for the callbacks - if a widget class is inside a (public) #if conditional block, then the callback functions should be also.
Fluid file attached produces correct header file but cxx file has callback function declarations that should be inside the #if...#endif blocks.
The resulting code will not compile because the functions (inside #if 0) were not in the compiled class definition in the header file.
 
 
#2 djl
05:34 Jan 23, 2008
I have posted a patch for Fl_Widget_Type which fixes this issue. I am not at all sure I have done it it the most elegant way, but it seems to work fine.
The only side-effect is that you get a few empty #if...#endif blocks in the header file which doesn't worry me.
 
 
#3 wavexx
09:38 Feb 20, 2008
After double checking, I don't think the proposed solution is ideal, as it repeats the declaration. This may not always work, esp if the block is dependent of some earlier declarations.  
 
#4 djl
02:28 Feb 21, 2008
I am sure it is not perfect, as it is really a hack but it works fine for me. The asumption I have made is that a declaration block in fluid is meant only to have #if....#endif in it and nothing else. Perhaps it is not really a declaration block, but a conditional compilation block ? Normal fluid declarations (not declaration blocks) would be used for things such as #include...
Anyway, if I have this correct, since my code recurses back up the fluid widget tree looking for declaration blocks I don't see how it would fail to find one (up to a depth of 16) - and if it did, the behaviour would be to just write the callback outside of that declaration block. This is the behaviour it does anyway without the patch.
It is true that the #if... #if... #if ... #endif #endif #endif tree will be repeated for each widget callback and it would be neater to have the callbacks lumped together in one #if tree but it looked to me like that would require a significant change to fluid and I didn't want to mess anything up. This patch is unlikely to affect anything else and any compiler will not mind repeated #if..#endifs.
I may have missed something but I can't see where it would fail.
 
 
#5 wavexx
07:37 Feb 21, 2008
Because definitions may not always be repeatable.
I'm attaching the test case I'm using.
 
 
#6 djl
09:34 Feb 21, 2008
Comparing the cxx files line by line, the only difference I see in the code produced from that test case is that in the patched version, the callbacks functions are enclosed in #ifdef's as I believe thay should be.
Is your point that the widgetclass2 callback functions should not be able to see REDIFINITION_CHECK ? If so I agree, the patch does not help this particular situation, but nor does it make it any worse than it was...
 
 
#7 wavexx
00:31 Feb 23, 2008
Yes, that was the point of the test. Callbacks do not see definitions contained inside the block itself. It striked to me as "obvious" if you consider the usual idiom "ifndef define" block, combined with nested items. I admit I never did that, but the code might silently fail. Static variables also suffer from the same problem.

I agree your patch fixes the problem most of the times. If no other solution is proposed within the rc1 release schedule, it should be integrated.

I tried to work on a better fix however, though it involves more changes. I hope to get something done by tomorrow.
 
 
#8 wavexx
12:01 Feb 24, 2008
Attaching a new fix. The new code delays static data code (both declarations and functions) until the declaration block itself is emitted.

This plays much better since:

- the declaration itself is not repeated
- the code is reentrant without arbitrary limits
- the declaration itself is visible for the inner code

Also, the inclusion of FL/Fl_* headers is repeated for each block, since some hidden code could shadow the proper inclusion (and vice versa: unneeded headers are properly nested).

I've tested it a bit, it seems to work correctly.
 
 
#9 djl
03:12 Feb 25, 2008
That looks much neater than my patch - it seems to work well for me apart from images. If you have the same image used in two boxes - the first one inside a #if 0, the second one inside a #if 1, the image data array is not included. See attached file.  
 
#10 matt
05:50 Feb 25, 2008
I looked at both patches and I am not too happy with either. Both patches change FLUID's behavior, risking incompatibilities to previously written code. As there are elements of distributions depending on FLUID's behavior, I would rather not do this change for FLTK 1.1.8.

On the down side, superflous static data is created and with it a compile time warning, but no severe error. IMHO we can live with this for now.

I will forward this to FLTK 1.2 (which is secret code for "any future version of FLTK based on 1.1.8 ;-). Maybe we will be able to fix the image issue then as well.

BTW: the second patch creates duplicate menu arrays... .
 
 
#11 AlbrechtS
04:10 Sep 01, 2016
Changed "Subsystem" to FLUID.  
bottom left image   bottom right image

Return to Bugs & Features | Post Text | Post File ]

 
 

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