On 20.07.2017 15:32, Henri Verbeet wrote:
On 20 July 2017 at 15:00, Jacek Caban jacek@codeweavers.com wrote:
This commit takes an approach similar to the earlier commit fabfa59aea5c5b3201142382038beb3e76cb7567: For ABI compatibility the return pointer is listed as an explicit parameter in the vtbl entry, while an inline helper is provided for source code compatibility.
While it fixes the problem for C++ callers, it breaks building classes implementing those interfaces. Implementation will try to override inline function instead of virtual one.
I does mean C++ implementations would need to explicitly list the return pointer as well, yes. I'm not aware of a way around that, aside from using a fixed compiler.
Agreed.
Given that the workaround has bad side effects, I'd say that we should at least try harder to not use it. For example it could be #ifdefed to not be used by compilers doing the right thing.
That's fine with me in principle, but see below. It also implies writing detection code for the bug.
Yes, proper detection code would be a problem. However I would expect something like following (could be emitted in header prologue) to be accurate for recent compilers and allow overriding settings with compiler arguments:
#ifndef WIDL_EXPLICIT_AGGREGATED_RETURN #if defined(__GNUC__) && !defined(__clang__) #define WIDL_EXPLICIT_AGGREGATED_RETURN 1 #else #define WIDL_EXPLICIT_AGGREGATED_RETURN 0 #endif #endif
That said, I don't really like it. I'd be happier to see a better solution, but if we have none, it's better than nothing.
With that, using our headers would require modifications in compiled code compatible with MSVC, so there would be a point in having a nice to use macro that interface implementations could use to decide if they should apply needed changes.
But if you have to make (minor) modifications to implementations, is that really any better than just explicitly listing the return pointer in the implementation? (Regardless of how easy/hard/ugly such a macro would need to be.)
You can't unconditionally do that if you want to keep sources compatible with MSVC. What I meant is that such code could use WIDL_EXPLICIT_AGGREGATED_RETURN for required #ifdefs (just like we can use it to generate declarations both with and without the workaround).
Jacek