> On 07/01/2013 04:01 PM, Tom Breton (Tehom) wrote:
>>> On 07/01/2013 03:07 PM, Tom Breton (Tehom) wrote:
>>>> Please tell me if there are any problems.
>
> This feels odd to me:
>
> // #define DEBUG_SEQUENCE_MANAGER 1
> #if !defined DEBUG_SEQUENCE_MANAGER
> #define NDEBUG 1
> #endif
>
> NDEBUG is used to change the behavior of code based on whether or not
> the build is a debug build. In rg, it is used about 27 times for this
> purpose. Doing this changes the meaning of NDEBUG and would surprise
> someone who wanted to use it in this file. NDEBUG is sort of a reserved
> #define that probably shouldn't be modified.
Agreed. The thing is, the old way made the compiler emit spurious warnings.
I admit, I sat on the fence about that for a long time. Last time that I
went over the code and squelched spurious warnings, I left that warning
unfixed for essentially the reason you give.
The thing that persuaded me to bite the bullet this time and finally fix
them was seeing that NDEBUG was already used exactly that way in at least
one file, BasicCommand.cpp. Once I knew I wasn't breaking new ground, it
seemed preferable to fix the spurious warnings.
It's good that we are discussing this. The Rosegarden system of debug
printing could use some attention.
> The removal of
>
> #ifdef DEBUG_SEQUENCE_MANAGER
>
> ...is a bit unsettling to me in general as that's just the way things
> are usually done all throughout rg. Consistency is helpful, form is
> liberating, war is peace, all that good stuff.
The commenting-out I don't consider anomalous. All thru RG, we turn debug
printing on and off by (un)commenting DEBUG_THISFILENAME.
What's anomalous is the placement. It's usually placed after the
includes, but that wouldn't work here.
> That said, the proliferation of verbose
>
> #ifdef DEBUG_SEQUENCE_MANAGER
>
> ...is definitely an eye-sore. Maybe if some sort of disable (kill?)
> switch were embedded in SEQMAN_DEBUG (and friends) to get rid of
> obnoxious logging? Something that is reserved for file scope disabling.
> Like:
>
> #define STOP_THE_FLIPPING_DEBUG_OUTPUT_IN_THIS_FILE_ONCE_AND_FOR_ALL 1
>
> That would be nice.
I did consider coining a new name. Like:
(SequenceManager.cpp)
#define RG_NO_DEBUG_PRINT
...
#include "misc/Debug.h"
(Debug.h)
#if !defined NDEBUG && !defined RG_NO_DEBUG_PRINT
...etc
I didn't think that was the most conservative change. It is doable though.
It would be tricky for those files that want to have more than one switch
for debug printing, eg Composition.cpp
Some technical points I want to mention:
* These spurious warnings happened because some arguments like "file" and
"line" were used just if DEBUG_SEQUENCE_MANAGER was defined. AFAICT,
it's either spurious warnings, something like this, something uglier, or
a non-trivial rewrite.
* misc/Debug.h already looks at NDEBUG and defines alternate structures
accordingly. One is RGNoDebug, the other is just macros over QDebug.
The two already co-exist, eg when included in BasicCommand.
Tom Breton (Tehom)
------------------------------------------------------------------------------
This SF.net email is sponsored by Windows:
Build for Windows Store.
http://p.sf.net/sfu/windows-dev2dev
_______________________________________________
Rosegarden-devel mailing list
[email protected] - use the link below to unsubscribe
https://lists.sourceforge.net/lists/listinfo/rosegarden-devel
