Hello,

I have fixed bug 28760:

https://bugs.freepascal.org/view.php?id=28760

This will cause compilation of win32wsdialogs.pp to fail, in particular in

class procedure TWin32WSOpenDialog.SetupVistaFileDialog(ADialog: IFileDialog; const AOpenDialog: TOpenDialog);

search for:

   if InitialDir  '' then
   begin
     if Succeeded(SHCreateItemFromParsingName(PWideChar(UTF8ToUTF16(InitialDir)), nil, IShellItem, DefaultFolderItem)) then
       ADialog.SetFolder(DefaultFolderItem);
   end;

Directly passing an interface where (T)REFIID is expected, will no longer be possible.

Michael.

Hello,

I have fixed bug 28760: https://bugs.freepascal.org/view.php?id=28760

as the reporter: not really.

The pointer-ness of REFIID is an artefact of the C-ABI. It is not meant to mean 'pass a pointer to a GUID-struct', but 'pass a GUID using byref'. We have constref for that.

Directly passing an interface where (T)REFIID is expected, will no longer be

possible.
It must be, if only for Delphi compat.

Delphi import is (... const riid: TIID;...), but that only works because they never seem to pass structured types on the stack, regardless of size.

Martok

See the link to the Windows MSDN.

Michael.

Yes, so?

REFIID is also the type of the first argument of QueryInterface. <https://msdn.microsoft.com/en-us/library/windows/desktop/ms682521%28v=vs.85%29.aspx>

    { IUnknown }
    function QueryInterface({$IFDEF
FPC_HAS_CONSTREF}constref{$ELSE}const{$ENDIF} IID: TGUID; out Obj): Hresult;
virtual; {$IFNDEF WINDOWS}cdecl{$ELSE}stdcall{$ENDIF};

But I see already it has been translated differently in every unit (mostly because MS made it uniform only after the headers were translated), so might as well use the new one to keep things interesting. Carry on.

Well, if they declare it as a pointer, why should I not do the same :) ?

REFIID is also the type of the first argument of QueryInterface. <https://msdn.microsoft.com/en-us/library/windows/desktop/ms682521%28v=vs.85%29.aspx>

I know.

That is a historical monstrosity :)

But I see already it has been translated differently in every unit (mostly because MS made it uniform only after the headers were translated), so might as well use the new one to keep things interesting. Carry on.

I agree the matter is a bit dubious :/

But, before doing anything, I looked around. Saw some C# and VB import statements. Noticed even in Delphi it's not 100% clear (see winrt* units), and finally decided to do it like this to make people aware of the fact that if they used a refiid argument in the past, it can hardly have worked. The current definition makes it quite clear that it is in fact a pointer to a GUID.

Michael.

I a gree we can do away with the IFDEFS and just use constref.

I think it's really not. We can transparently use an interface name where a GUID is required for a reason, and that is part of why it's easier to write COM applications in Delphi than in MS compilers.
Were your new definition applied globally and none of the APIs actually use the non-pointer type, we might as well get rid of that feature, as it'd be useless then.

The current definition makes it quite clear that it is in fact a pointer to a GUID.

In MSDN hungarian notation terms, it is very obviously not: REFIID is notationally different from LPIID (or just plain *IID in argument lists).

Well, I prefer something less ambiguous, not relying on murky ABI agreements. So I acted accordingly.

Michael.

Michael Van Canneyt wrote

Directly passing an interface where (T)REFIID is expected, will no longer be possible.

What is the fix?

"@GetTypeData(TypeInfo(IShellItem))^.Guid" ?

Ondrej

That is one possible fix, yes.

Maybe we can add a leightweight function for this to the typinfo unit.

Michael.

Michael Van Canneyt wrote

That is one possible fix, yes. Maybe we can add a leightweight function for this to the typinfo unit.

Actually it works well with a local variable. I fixed it in r55562. I hope it's fine so.

Ondrej

I don't think that it was necessary to change (T)REFIID to be a pointer and thus making COM interface usage unnecessarily more complicated in Object Pascal:
- Microsoft implicitely defines a REFIID in guiddef.h to always be an "IN" parameter, by defining it as "const IID &" for C++ code, so it can never be written to anyway, thus a "constref" would have sufficed (a "const IID &" in C++ behaves the same as a "IID *" in C or a "constref x: IID" in FPC)
- aggregates (which is what a TGUID is) are always passed as pointers for stdcall functions (basically all functions that are related to COM and thus to REFIID) and also the Win64 ABI anyway, so a mere "TIID" already behaved correctly for the Windows targets

Regards,
Sven

Being old school, I really don't like all this magic. But if you feel it necessary, you are of course free to change it back.

Michael.

Sven Barth wrote

- aggregates (which is what a TGUID is) are always passed as pointers for stdcall functions (basically all functions that are related to COM and thus to REFIID) and also the Win64 ABI anyway, so a mere "TIID" already behaved correctly for the Windows targets

They weren't always, at least in this particular revision I reported the original bug against. Ended up having the GUID itself on the stack, not a reference - but only with const, and only in some functions. No specifier would have worked too, but I think constref is more descriptive.

Am 23.07.2017 12:50 schrieb "Michael Van Canneyt via Lazarus" <

IFileDialog; const AOpenDialog: TOpenDialog);

Succeeded(SHCreateItemFromParsingName(PWideChar(UTF8ToUTF16(InitialDir)), nil, IShellItem, DefaultFolderItem)) then

ADialog.SetFolder(DefaultFolderItem); end; Directly passing an interface where (T)REFIID is expected, will no longer

be possible.

I reverted the changes by Michael and changed the parameters of those functions to constref instead.
However that doesn't mean that there won't be breaking changes: there are some interfaces that take REFIID (or TGuid parameters that are generally used with interface IIDs) and those will need to be changed as well. Most will be interface which have implementations provided by e.g. Windows, but in some cases those might be implemented by users and thus will lead to compilation failures (but at least those will be easy to spot :) ).

Regards,
Sven
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus-ide.org/pipermail/lazarus/attachments/20170729/b8680deb/attachment.html>