Re: [Audacity-devel] Checked for safe uses of Experimental.h

[Paul L. <pau...@gm...>]

On Mon, Aug 31, 2015 at 6:27 PM, James Crook <cr...@in...> wrote:

> That suggests we should use
>
> #if EXPERIMENTAL_featurename
> rather than
> #ifdef EXPERIMENTAL_featurename
>
> and set them to 0 or 1 in experimental.h
>

I thought of that too, but when I try it, unfortunately, I don't break
compilation.  Apparently an undefined symbol is like a symbol defined 0.

PRL


> --James.
>
>
>
> On 31/08/2015 23:01, Paul Licameli wrote:
>
> It bothers me that if you forget to #include "Experimental.h", some code
> that tests the experimental flags might compile quietly but do the wrong
> thing.  Some EXPERIMENTAL symbol used in many places might appear defined
> in one and not another.
>
> It's not so wrong for the release build if the flag is turned off, but
> your private experimental build may be wrong of you turn it on.
>
> It is wronger when the symbol is turned on in release.
>
> I identified every place in a .h or .cpp file where an EXPERIMENTAL flag
> is tested but #include "Experimental.h" did not precede it in the same
> file.  Not all of these are necessarily broken, if some earlier #include
> saves you, but some might be (and for headers, broken when included in some
> places but not others).
>
> At every such place I added
> #ifndef __EXPERIMENTAL__
> #error
> #endif
>
> and then I saw what failed to compile in release on Windows.
>
> Failures were in:
>
> AboutDialog.cpp
> Equalization.cpp
> ErrorDialog.cpp
> ExpandingToolBar.cpp
> HelpSystem.cpp
> KeyView.cpp
> LoadVamp.cpp
> SplashDialog.cpp
>
> These failures were because of four header files:
>
> AllThemeResources.h
> CommandManager.h
> EffectManager.h
> Equalization.h
>
> Three of those four are harmless cases, because they test flags that are
> not turned on in release:  EXPERIMENTAL_THEMING, EXPERIMENTAL_EFFECTS_RACK,
> EXPERIMENTAL_EQ_SSE_THREADED
>
> But CommandManager.h tests EXPERIMENTAL_KEY_VIEW which is on.
>
> However, even that one turned out to be harmless, because it changed the
> argument list of one overload of CommandManager::GetAllCommandData, which
> KeyView.cpp did not call.
>
> Conclusion:  If I put #include "Experimental.h" in these places where it
> was missing, I am not making a quiet change in the Release build.
>
> So I committed those changes, along with other #include lines that are
> redundant because the #error directives were not hit, but those #include
> lines are a good idea anyway.
>
> PRL
>
>
>
>
> ------------------------------------------------------------------------------
>
>
>
> _______________________________________________
> audacity-devel mailing lis...@li...https://lists.sourceforge.net/lists/listinfo/audacity-devel
>
>
>
>
> ------------------------------------------------------------------------------
>
> _______________________________________________
> audacity-devel mailing list
> aud...@li...
> https://lists.sourceforge.net/lists/listinfo/audacity-devel
>
>