Hi,

On Sun, Jul 15, 2012 at 3:31 PM, Måns Rullgård <[email protected]> wrote:
> "Ronald S. Bultje" <[email protected]> writes:
>
>> Hi,
>>
>> On Sun, Jul 8, 2012 at 1:34 PM, Luca Barbato <[email protected]> wrote:
>>> On 07/06/2012 07:55 PM, Ronald S. Bultje wrote:
>>>> From: "Ronald S. Bultje" <[email protected]>
>>>>
>>>> MSVC has incomplete variadic macro argument handling, meaning it does
>>>> support it to some extend, but it will basically handle all variadic
>>>> arguments as a single argument, including the comma that separates them,
>>>> thus making it useless for anything else than as a function argument.
>>>> Our implementation of LOCAL_ALIGNED() breaks because of this, thus here
>>>> we implement an alternative form of LOCAL_ALIGNED() that does not use
>>>> variadic macro arguments.
>>>> ---
>>>>  configure                    |    1 +
>>>>  libavcodec/aacps.c           |   10 +++++-----
>>>>  libavcodec/aacsbr.c          |    2 +-
>>>>  libavcodec/ac3enc.c          |    2 +-
>>>>  libavcodec/ac3enc_template.c |    4 ++--
>>>>  libavcodec/dsputil.h         |   26 ++++++++++++++++++--------
>>>>  libavcodec/dvdec.c           |    2 +-
>>>>  libavcodec/h264_loopfilter.c |    2 +-
>>>>  libavcodec/ra288.c           |    6 +++---
>>>>  9 files changed, 33 insertions(+), 22 deletions(-)
>>>
>>> As reported in the bug closed as WontFix[1], a second expansion solves
>>> the problem, I tested in my personal msvc tree and it seems working.
>>>
>>> Probably it could be implemented with a sed replacement as well so it
>>> won't hinder normal development.
>>>
>>> lu
>>>
>>> [1]
>>> http://connect.microsoft.com/VisualStudio/feedback/details/380090/variadic-macro-replacement
>>
>> That still requires source code modifications, which seems to hit a
>> brick wall here.
>>
>> I've looked at http://msdn.microsoft.com/en-us/library/ms177415(v=vs.80).aspx
>> and http://gcc.gnu.org/onlinedocs/cpp/Variadic-Macros.html, and it's
>> clear that something like #define LOCAL_ALIGNED_16(t, v, s, ...) has
>> theoretical issues if defined to a function (#define eprintf(format,
>> ...) fprintf(stderr, format, __VA_ARGS__) with eprintf("hi\n") becomes
>> fprintf(stderr, "hi\n", ), i.e. an empty terminal comma). However,
>> both appear to support this form when used on variadic macros (and in
>> fact also when used on a vararg functions like fprintf). So I could do
>> something like:
>
> Of course this doesn't work.  The standard requires that every
> invocation of a variadic macro supply at least one argument for the
> variable argument list.  Your example does not do this.  Some compilers
> will not even compile it.
>
>> #define LOCAL_ALIGNED_16(t, v, s, ...) LOCAL_ALIGNED_D(16, t, v, s,
>> __VA_ARGS__,)
>>
>> instead of
>>
>> #define LOCAL_ALIGNED_16(t, v, ...) LOCAL_ALIGNED_D(16, t, v, __VA_ARGS__,,)
>>
>> and to the best of my knowledge, since the called macro itself is
>> always variadic also, this should actually work - not just on these
>> two compilers, but on any compiler. Is there a specific reason we
>> don't use this instead?
>
> Yes, the code is the way it is because that is how the standard requires
> it.  Your suggestion will break with some strictly conforming compilers.

I mentioned this very fact in my email. However, your quote is only
true for functions, not macros. Here, the variadic macro calls a
macro, not a function, so your claim is incorrect. A macro with an
empty argument is perfectly valid; in fact, the current code relies on
two empty arguments.

Instead of saying "rejected", which you undoubtedly will do, can you
please think constructively on how we can fix this?

Ronald
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to