Re: [Mingw-w64-public] How to fix error with macros in d3d12?

[Zebediah Figura]

(+ wine-devel)

On 6/11/22 13:26, LIU Hao wrote:
> 在 2022-06-12 01:59, Zebediah Figura 写道:
> > > The combination of `extern` with `__gnu_inline__` is necessary to suppress generation of a function
> > > body if the attached function could not be inlined when compiling as C99.
> >
> > Why do we need that?
>
> Otherwise there would be errors about multiple definitions.
> > > I don't think there is
> > > anything we can do with these headers. Perhaps WIDL should not generate `static` for `FORCEINLINE`
> > > functions; or, if that is not an option, `static __inline__` might be a better alternative.
> >
> > Is it legal to use 'static __forceinline' or 'extern __forceinline' with MSVC? If so I imagine we
> > should find a solution that allows them.
>
> Yes, unfortunately. I don't think there is a solution. `extern inline` and `inline` are not
> equivalent in C.

Right, so, apparently MSVC treats both "inline" and "__forceinline" as 
magic bullets—they can both appear with or without 'static' and 
'extern'; they'll always generate a function body but can also be 
defined in multiple source files (in fact, if the definition differs and 
the functions aren't static, an arbitrary one is picked and no warning 
is even printed.) And for good measure you can always take the address 
too. [So as far as I can tell there's no difference between "inline" and 
"extern inline" as far as MSVC is concerned.]

I guess we can't do anything about that without modifying the language. 
But of course in lieu of that we should probably fix our headers. I see 
at least three options:

(1) Use "FORCEINLINE" instead of "static FORCEINLINE" for these 
functions. MSVC never uses FORCEINLINE with static or extern, so this 
would be more consistent. The downside is we'd need to provide import 
library definitions for all of these functions, though, which seems less 
than ideal. (Unless combined with option 3 below.)

(2) Use "static inline" for COM wrappers. This of course drops the 
__always_inline__ attribute. I'm inclined to assert that's a non-issue, 
but then again I never really saw the point of __always_inline__ anyway. 
[I guess it can be used to force the compiler to inline even when 
-finline isn't enough, but surely that would never be the case here...]

(2b) Or use "static inline __attribute((__always_inline__))", and define 
a new macro to encapsulate that.

(3) Define __forceinline as "static inline" rather than "extern inline". 
I have to assume there was a reason this wasn't done in the first place, 
but there's no documentation in the file and I couldn't easily find 
discussion about it. As far as I can tell this actually matches MSVC 
almost exactly, except that multiple definitions will be generated if 
something takes the function address. That is arguably a break in 
behaviour which is worse than just failing to link, though.

I think my preferred solution is 2 (or 2b I guess). I'm planning to send 
the attached patch to winehq (since we need to sync widl from there), 
but I'd like to hear some feedback from the mingw-w64 project first.

ἔρρωσθε,
Zeb

[Mingw-w64-public] How to fix error with macros in d3d12?

[Jacek Caban]

On 7/25/22 00:10, Zebediah Figura wrote:
> (+ wine-devel)
>
> On 6/11/22 13:26, LIU Hao wrote:
> > 在 2022-06-12 01:59, Zebediah Figura 写道:
> > > > The combination of `extern` with `__gnu_inline__` is necessary to 
> > > > suppress generation of a function
> > > > body if the attached function could not be inlined when compiling 
> > > > as C99.
> > >
> > > Why do we need that?
> >
> > Otherwise there would be errors about multiple definitions.
> > > > I don't think there is
> > > > anything we can do with these headers. Perhaps WIDL should not 
> > > > generate `static` for `FORCEINLINE`
> > > > functions; or, if that is not an option, `static __inline__` might 
> > > > be a better alternative.
> > >
> > > Is it legal to use 'static __forceinline' or 'extern __forceinline' 
> > > with MSVC? If so I imagine we
> > > should find a solution that allows them.
> >
> > Yes, unfortunately. I don't think there is a solution. `extern 
> > inline` and `inline` are not
> > equivalent in C.
>
> Right, so, apparently MSVC treats both "inline" and "__forceinline" as 
> magic bullets—they can both appear with or without 'static' and 
> 'extern'; they'll always generate a function body but can also be 
> defined in multiple source files (in fact, if the definition differs 
> and the functions aren't static, an arbitrary one is picked and no 
> warning is even printed.) And for good measure you can always take the 
> address too. [So as far as I can tell there's no difference between 
> "inline" and "extern inline" as far as MSVC is concerned.]
>
> I guess we can't do anything about that without modifying the 
> language. But of course in lieu of that we should probably fix our 
> headers. I see at least three options:
>
> (1) Use "FORCEINLINE" instead of "static FORCEINLINE" for these 
> functions. MSVC never uses FORCEINLINE with static or extern, so this 
> would be more consistent. The downside is we'd need to provide import 
> library definitions for all of these functions, though, which seems 
> less than ideal. (Unless combined with option 3 below.)
>
> (2) Use "static inline" for COM wrappers. This of course drops the 
> __always_inline__ attribute. I'm inclined to assert that's a 
> non-issue, but then again I never really saw the point of 
> __always_inline__ anyway. [I guess it can be used to force the 
> compiler to inline even when -finline isn't enough, but surely that 
> would never be the case here...]
>
> (2b) Or use "static inline __attribute((__always_inline__))", and 
> define a new macro to encapsulate that.
>
> (3) Define __forceinline as "static inline" rather than "extern 
> inline". I have to assume there was a reason this wasn't done in the 
> first place, but there's no documentation in the file and I couldn't 
> easily find discussion about it. As far as I can tell this actually 
> matches MSVC almost exactly, except that multiple definitions will be 
> generated if something takes the function address. That is arguably a 
> break in behaviour which is worse than just failing to link, though.

I recall attempts to improve it in mingw-w64, but I think they always 
hit one or another compatibility limitations (see commit 5255bcfd21 for 
an example). My long term preferred solution would be:

(4) Have proper compiler support for __forceinline (probably in form of 
__attribute__((__forceinline__))
> I think my preferred solution is 2 (or 2b I guess). I'm planning to 
> send the attached patch to winehq (since we need to sync widl from 
> there), but I'd like to hear some feedback from the mingw-w64 project 
> first.

FWIW, the patch seems fine to me.

Thanks,

Jacek

[Mingw-w64-public] How to fix error with macros in d3d12?

[Zebediah Figura]

On 7/25/22 10:25, Jacek Caban wrote:
> On 7/25/22 00:10, Zebediah Figura wrote:
> > (+ wine-devel)
> >
> > On 6/11/22 13:26, LIU Hao wrote:
> > > 在 2022-06-12 01:59, Zebediah Figura 写道:
> > > > > The combination of `extern` with `__gnu_inline__` is necessary to 
> > > > > suppress generation of a function
> > > > > body if the attached function could not be inlined when compiling 
> > > > > as C99.
> > > >
> > > > Why do we need that?
> > >
> > > Otherwise there would be errors about multiple definitions.
> > > > > I don't think there is
> > > > > anything we can do with these headers. Perhaps WIDL should not 
> > > > > generate `static` for `FORCEINLINE`
> > > > > functions; or, if that is not an option, `static __inline__` might 
> > > > > be a better alternative.
> > > >
> > > > Is it legal to use 'static __forceinline' or 'extern __forceinline' 
> > > > with MSVC? If so I imagine we
> > > > should find a solution that allows them.
> > >
> > > Yes, unfortunately. I don't think there is a solution. `extern 
> > > inline` and `inline` are not
> > > equivalent in C.
> >
> > Right, so, apparently MSVC treats both "inline" and "__forceinline" as 
> > magic bullets—they can both appear with or without 'static' and 
> > 'extern'; they'll always generate a function body but can also be 
> > defined in multiple source files (in fact, if the definition differs 
> > and the functions aren't static, an arbitrary one is picked and no 
> > warning is even printed.) And for good measure you can always take the 
> > address too. [So as far as I can tell there's no difference between 
> > "inline" and "extern inline" as far as MSVC is concerned.]
> >
> > I guess we can't do anything about that without modifying the 
> > language. But of course in lieu of that we should probably fix our 
> > headers. I see at least three options:
> >
> > (1) Use "FORCEINLINE" instead of "static FORCEINLINE" for these 
> > functions. MSVC never uses FORCEINLINE with static or extern, so this 
> > would be more consistent. The downside is we'd need to provide import 
> > library definitions for all of these functions, though, which seems 
> > less than ideal. (Unless combined with option 3 below.)
> >
> > (2) Use "static inline" for COM wrappers. This of course drops the 
> > __always_inline__ attribute. I'm inclined to assert that's a 
> > non-issue, but then again I never really saw the point of 
> > __always_inline__ anyway. [I guess it can be used to force the 
> > compiler to inline even when -finline isn't enough, but surely that 
> > would never be the case here...]
> >
> > (2b) Or use "static inline __attribute((__always_inline__))", and 
> > define a new macro to encapsulate that.
> >
> > (3) Define __forceinline as "static inline" rather than "extern 
> > inline". I have to assume there was a reason this wasn't done in the 
> > first place, but there's no documentation in the file and I couldn't 
> > easily find discussion about it. As far as I can tell this actually 
> > matches MSVC almost exactly, except that multiple definitions will be 
> > generated if something takes the function address. That is arguably a 
> > break in behaviour which is worse than just failing to link, though.
>
> I recall attempts to improve it in mingw-w64, but I think they always 
> hit one or another compatibility limitations (see commit 5255bcfd21 for 
> an example). My long term preferred solution would be:
>
> (4) Have proper compiler support for __forceinline (probably in form of 
> __attribute__((__forceinline__))

Is there anything special about __forceinline in this respect, relative 
to the MSVC semantics of regular inline? I.e. as far as I can tell, to 
sort of mix metaphors, __forceinline is basically just inline plus GNU's 
__attribute__((__always_inline__)). But there certainly may have been 
things I've missed.

Anyway, yes, I agree we want proper compiler support.
> > I think my preferred solution is 2 (or 2b I guess). I'm planning to 
> > send the attached patch to winehq (since we need to sync widl from 
> > there), but I'd like to hear some feedback from the mingw-w64 project 
> > first.
>
> FWIW, the patch seems fine to me.
>
> Thanks,
>
> Jacek

[Mingw-w64-public] How to fix error with macros in d3d12?

[Zebediah Figura]

Sorry, accidentally sent this reply to wine-devel only...

On 7/25/22 14:12, Zebediah Figura wrote:
> On 7/25/22 10:25, Jacek Caban wrote:
> > On 7/25/22 00:10, Zebediah Figura wrote:
> > > (+ wine-devel)
> > >
> > > On 6/11/22 13:26, LIU Hao wrote:
> > > > 在 2022-06-12 01:59, Zebediah Figura 写道:
> > > > > > The combination of `extern` with `__gnu_inline__` is necessary to 
> > > > > > suppress generation of a function
> > > > > > body if the attached function could not be inlined when compiling 
> > > > > > as C99.
> > > > >
> > > > > Why do we need that?
> > > >
> > > > Otherwise there would be errors about multiple definitions.
> > > > > > I don't think there is
> > > > > > anything we can do with these headers. Perhaps WIDL should not 
> > > > > > generate `static` for `FORCEINLINE`
> > > > > > functions; or, if that is not an option, `static __inline__` might 
> > > > > > be a better alternative.
> > > > >
> > > > > Is it legal to use 'static __forceinline' or 'extern __forceinline' 
> > > > > with MSVC? If so I imagine we
> > > > > should find a solution that allows them.
> > > >
> > > > Yes, unfortunately. I don't think there is a solution. `extern 
> > > > inline` and `inline` are not
> > > > equivalent in C.
> > >
> > > Right, so, apparently MSVC treats both "inline" and "__forceinline" 
> > > as magic bullets—they can both appear with or without 'static' and 
> > > 'extern'; they'll always generate a function body but can also be 
> > > defined in multiple source files (in fact, if the definition differs 
> > > and the functions aren't static, an arbitrary one is picked and no 
> > > warning is even printed.) And for good measure you can always take 
> > > the address too. [So as far as I can tell there's no difference 
> > > between "inline" and "extern inline" as far as MSVC is concerned.]
> > >
> > > I guess we can't do anything about that without modifying the 
> > > language. But of course in lieu of that we should probably fix our 
> > > headers. I see at least three options:
> > >
> > > (1) Use "FORCEINLINE" instead of "static FORCEINLINE" for these 
> > > functions. MSVC never uses FORCEINLINE with static or extern, so this 
> > > would be more consistent. The downside is we'd need to provide import 
> > > library definitions for all of these functions, though, which seems 
> > > less than ideal. (Unless combined with option 3 below.)
> > >
> > > (2) Use "static inline" for COM wrappers. This of course drops the 
> > > __always_inline__ attribute. I'm inclined to assert that's a 
> > > non-issue, but then again I never really saw the point of 
> > > __always_inline__ anyway. [I guess it can be used to force the 
> > > compiler to inline even when -finline isn't enough, but surely that 
> > > would never be the case here...]
> > >
> > > (2b) Or use "static inline __attribute((__always_inline__))", and 
> > > define a new macro to encapsulate that.
> > >
> > > (3) Define __forceinline as "static inline" rather than "extern 
> > > inline". I have to assume there was a reason this wasn't done in the 
> > > first place, but there's no documentation in the file and I couldn't 
> > > easily find discussion about it. As far as I can tell this actually 
> > > matches MSVC almost exactly, except that multiple definitions will be 
> > > generated if something takes the function address. That is arguably a 
> > > break in behaviour which is worse than just failing to link, though.
> >
> > I recall attempts to improve it in mingw-w64, but I think they always 
> > hit one or another compatibility limitations (see commit 5255bcfd21 
> > for an example). My long term preferred solution would be:
> >
> > (4) Have proper compiler support for __forceinline (probably in form 
> > of __attribute__((__forceinline__))
>
> Is there anything special about __forceinline in this respect, relative 
> to the MSVC semantics of regular inline? I.e. as far as I can tell, to 
> sort of mix metaphors, __forceinline is basically just inline plus GNU's 
> __attribute__((__always_inline__)). But there certainly may have been 
> things I've missed.
>
> Anyway, yes, I agree we want proper compiler support.
> > > I think my preferred solution is 2 (or 2b I guess). I'm planning to 
> > > send the attached patch to winehq (since we need to sync widl from 
> > > there), but I'd like to hear some feedback from the mingw-w64 project 
> > > first.
> >
> > FWIW, the patch seems fine to me.
> >
> > Thanks,
> >
> > Jacek

[Mingw-w64-public] How to fix error with macros in d3d12?

[LIU Hao]

在 2022/7/25 23:25, Jacek Caban 写道:
> > I think my preferred solution is 2 (or 2b I guess). I'm planning to send the attached patch to 
> > winehq (since we need to sync widl from there), but I'd like to hear some feedback from the 
> > mingw-w64 project first.
>
> FWIW, the patch seems fine to me.

If this function should be callable from C, then `__inline__` should be preferred, for compatibility 
with C89... or is it pure C++?

In practice, I think `static __inline__` suffices almost everywhere.