On Tue, Jul 24, 2012 at 6:21 PM, Markus Trippelsdorf
<mar...@trippelsdorf.de> wrote:
> On 2012.07.23 at 19:01 +0200, Steven Bosscher wrote:
>> Hello,
>>
>> This large patch makes GATHER_STATISTICS always take a value, 0
>> (disabled) or 1 (enabled), and turns all related #ifdef code into
>> conditional branches.
>>
>> This slightly increases the data section of cc1, but only marginally.
>> There is no impact on compile time, because all tests are simply
>> optimized away if GATHER_STATISTICS is 0.
>>
>> The benefit is that this improves the coverage of this code, and less
>> #ifdef'ed code (which is IMHO a Good Thing).
>>
>>...
>>
>> Index: alloc-pool.c
>> ===================================================================
>> --- alloc-pool.c (revision 189784)
>> +++ alloc-pool.c (working copy)
>> @@ -62,8 +62,6 @@ typedef struct allocation_object_def
>> static ALLOC_POOL_ID_TYPE last_id;
>> #endif
>>
>> -#ifdef GATHER_STATISTICS
>> -
>> /* Store information about each particular alloc_pool. Note that this
>> will underestimate the amount the amount of storage used by a small
>> amount:
>> 1) The overhead in a pool is not accounted for.
>> @@ -123,7 +121,6 @@ alloc_pool_descriptor (const char *name)
>> (*slot)->name = name;
>> return *slot;
>> }
>> -#endif
>>
>> /* Create a pool of things of size SIZE, with NUM in each block we
>> allocate. */
>> @@ -133,9 +130,6 @@ create_alloc_pool (const char *name, siz
>> {
>> alloc_pool pool;
>> size_t header_size;
>> -#ifdef GATHER_STATISTICS
>> - struct alloc_pool_descriptor *desc;
>> -#endif
>>
>> gcc_checking_assert (name);
>>
>> @@ -146,10 +140,11 @@ create_alloc_pool (const char *name, siz
>> /* Now align the size to a multiple of 4. */
>> size = align_eight (size);
>>
>> -#ifdef ENABLE_CHECKING
>> - /* Add the aligned size of ID. */
>> - size += offsetof (allocation_object, u.data);
>> -#endif
>> + if (ENABLE_CHECKING)
>> + {
>> + /* Add the aligned size of ID. */
>> + size += offsetof (allocation_object, u.data);
>> + }
>
> The change above causes a build failure with --enable-checking=release:
Yes. It seems I was a bit too zealous there. Consider it fixed. Sorry
for the breakage.
Proves my point though: #ifdef is bad :-)
Ciao!
Steven
