[ Return to Bugs & Features | Post Text | Post File | Prev | Next ]

STR #1859

Trouble Report Files:

	Name/Time/Date	Filename/Size
	#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

Trouble Report Comments:

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

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