Re: [PATCH v2 2/6] kunit: Add macro to conditionally expose declarations to tests

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

On 27.08.2024 15:45, Lucas De Marchi wrote:
> On Tue, Aug 27, 2024 at 12:20:11AM GMT, Michal Wajdeczko wrote:
>> The DECLARE_IF_KUNIT macro will introduces identifiers only if
>> CONFIG_KUNIT is enabled. Otherwise if CONFIG_KUNIT is not enabled
>> no identifiers from the param list will be defined.
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Reviewed-by: Rae Moar <rmoar@google.com>
>> Reviewed-by: David Gow <davidgow@google.com>
>> ---
>> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> 
> up to kunit maintainers, but it doesn't seem the word "DECLARE" is
> entirely correct. What it's doing is expanding arg, and it doesn't
> matter if it's an expression, definition, declaration.

hmm, while this is true for statement & declarations (as both have
similar notation) but not sure about the expression (that's why we have
patch 3)

> 
> Looking at patch 3, I think it would be more obvious to the caller if we
> have:
> 
> IF_KUNIT_ELSE_EMPTY()
> IF_KUNIT_ELSE_ZERO()

existing macros in this file are named as xxx_IF_KUNIT so maybe we
should try to follow that pattern...

so maybe (like BUILD_BUG_ON_ZERO)

	ONLY_IF_KUNIT(body...)
	ONLY_IF_KUNIT_ZERO(expr...)

> 
>> ---
>> include/kunit/visibility.h | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/include/kunit/visibility.h b/include/kunit/visibility.h
>> index 0dfe35feeec6..1c23773f826c 100644
>> --- a/include/kunit/visibility.h
>> +++ b/include/kunit/visibility.h
>> @@ -11,6 +11,13 @@
>> #define _KUNIT_VISIBILITY_H
>>
>> #if IS_ENABLED(CONFIG_KUNIT)
>> +    /**
>> +     * DECLARE_IF_KUNIT - A macro that introduces identifiers only if
>> +     * CONFIG_KUNIT is enabled. Otherwise if CONFIG_KUNIT is not enabled
>> +     * no identifiers will be defined.
>> +     * @body: identifiers to be introduced conditionally
> 
> missing an example here with fields inside a struct

would this work?

Example:

	struct example {
		int foo;
		/* private: test only */
		DECLARE_IF_KUNIT(int bar);
	};

> 
> Lucas De Marchi
> 
>> +     */
>> +    #define DECLARE_IF_KUNIT(body...)    body
>>     /**
>>      * VISIBLE_IF_KUNIT - A macro that sets symbols to be static if
>>      * CONFIG_KUNIT is not enabled. Otherwise if CONFIG_KUNIT is enabled
>> @@ -26,6 +33,7 @@
>>     #define EXPORT_SYMBOL_IF_KUNIT(symbol) EXPORT_SYMBOL_NS(symbol, \
>>         EXPORTED_FOR_KUNIT_TESTING)
>> #else
>> +    #define DECLARE_IF_KUNIT(body...)
>>     #define VISIBLE_IF_KUNIT static
>>     #define EXPORT_SYMBOL_IF_KUNIT(symbol)
>> #endif
>> -- 
>> 2.43.0
>>