[PATCH] extend __interface support

Please find attached a patch that extends the support for the
Microsoft-specific __interface keyword, currently supported as an alias
for struct. I do not have commit access, and if this were to be accepted
I would need someone to commit it for me.

This patch adds the semantics of making all member functions defined in
the interface into pure virtuals, as MSVC does
(__interface | Microsoft Docs).

It is implemented as a keyword which translates to struct with a new
MS-specific attribute "ms_interface", which causes member functions to
be set to pure virtual (it is not an error to declare either on top of
the automatic setting; additional pure/virtual specifiers are ignored as
in MSVC).

The MicrosoftExtension.cpp test is also updated to do an override of a
base interface method not explicitly declared virtual, which would fail
without this change.

Thank you for your comments/consideration.

Clang.__interface.patch (5.82 KB)

Do clang's diagnostics with your patch match MSVC's diagnostics for
examples like the following?

__interface X;
struct X;
__interface Y;
class Y;

It looks like your patch has a few lines which run over 80 columns.

-Eli

Is this implicitly an answer to my previous message? That is, are you
saying that __interface cannot be used in elaborated type specifiers?
Please do test this; the right way to model this feature depends on
how it fits in the language.

John.

Do clang’s diagnostics with your patch match MSVC’s diagnostics for
examples like the following?

__interface X;
struct X;
__interface Y;
class Y;

MSVC does not seem to emit any diagnostics in these cases, although maybe we should (maybe a warning).

Thanks. In that case, I definitely don’t think we want to model this with
macros and attributes — MSVC is clearly treating __interface as a fourth
kind of class-key, and that’s the appropriate way to model it.

John.

Do clang's diagnostics with your patch match MSVC's diagnostics for
examples like the following?

__interface X;
struct X;
__interface Y;
class Y;

MSVC 10 (2010) emits (with /Wall; nothing without):

  int.cpp(4) : warning C4099: 'Y' : type name first seen using 'struct'
  now seen using 'class'
          int.cpp(3) : see declaration of 'Y'

Clang with patch emits (with -Wall; nothing without):

  int.cpp:4:1: warning: class 'Y' was previously declared as a struct
        [-Wmismatched-tags]
  class Y;
  ^
  int.cpp:3:13: note: previous use is here
  __interface Y;
              ^

This looks like a good match.

It looks like your patch has a few lines which run over 80 columns.

I'll fix any lines > 80 columns after resolving other issues and before
resubmitting.

Is this implicitly an answer to my previous message? That is, are you
saying that __interface cannot be used in elaborated type specifiers?
Please do test this; the right way to model this feature depends on
how it fits in the language.

I answered the questions ("can you write...") in a reply to your message
in the previous thread.

You said in a previous message to João Matos:

Thanks. In that case, I definitely don't think we want to model this with
macros and attributes * MSVC is clearly treating __interface as a fourth
kind of class-key, and that's the appropriate way to model it.

I'm happy to rework the patch; but if you have the time could you
explain the difference? By "fourth kind of class-key" (I understand
class-key from the C++ spec) do you mean that it should have its own
decl spec type TST_interface and TagTypeKind TTK_Interface rather than
the ms_interface attribute?

Note that MS diagnostics refer to __interface as 'struct', e.g.

  int.cpp(4) : warning C4099: 'Y' : type name first seen using 'struct'
  now seen using 'class'

Does that matter at all?

No wonder I wasn’t getting warnings, forgot to enable “-Wall” in my tests.

I can see some advantages in modelling this as a new tag type specifier. Just from a quick glance at the code, there are some places in the parser where we’re assigning the expected name type specifier (struct / class / enum) and with an attribute these cases might be ignored. With a new TST value, this will trigger compiler warnings (and hopefully it will get fixed :).

Is this implicitly an answer to my previous message?

David pointed out that I am apparently losing messages, which is worrisome.
I'll grab your last response via the web archives.

> Is this implicitly an answer to my previous message? That is, are you
> saying that __interface cannot be used in elaborated type specifiers?
> Please do test this; the right way to model this feature depends on
> how it fits in the language.

I answered the questions ("can you write...") in a reply to your message
in the previous thread.

You said in a previous message to João Matos:

> Thanks. In that case, I definitely don't think we want to model this with
> macros and attributes * MSVC is clearly treating __interface as a fourth
> kind of class-key, and that's the appropriate way to model it.

I'm happy to rework the patch; but if you have the time could you
explain the difference? By "fourth kind of class-key" (I understand
class-key from the C++ spec) do you mean that it should have its own
decl spec type TST_interface and TagTypeKind TTK_Interface rather than
the ms_interface attribute?

Yeah, that's what I was suggesting. That lets arbitrary clients remember
the exact intent a little easier.

Note that MS diagnostics refer to __interface as 'struct', e.g.

  int.cpp(4) : warning C4099: 'Y' : type name first seen using 'struct'
  now seen using 'class'

Does that matter at all?

That's interesting. Maybe MSVC just defines this as a macro? Have you tried
running it through the preprocessor? Although it's more likely that they're
just being lazy about their diagnostics.

John.

> class-key from the C++ spec) do you mean that it should have its own
> decl spec type TST_interface and TagTypeKind TTK_Interface rather than
> the ms_interface attribute?

Yeah, that's what I was suggesting. That lets arbitrary clients remember
the exact intent a little easier.

I'll work up a patch that works that way and post when ready.

> Note that MS diagnostics refer to __interface as 'struct', e.g.
>
> int.cpp(4) : warning C4099: 'Y' : type name first seen using 'struct'
> now seen using 'class'
>
> Does that matter at all?

That's interesting. Maybe MSVC just defines this as a macro? Have you tried
running it through the preprocessor? Although it's more likely that they're
just being lazy about their diagnostics.

No, it's not an actual macro - and can't be, because of the way it
forces member function decls to be pure virtual.

(BTW, I dropped you from the 'To' line this time and sent direct to the
list address only, which doesn't seem to cause messages to be dropped -
let me know.)

class-key from the C++ spec) do you mean that it should have its own
decl spec type TST_interface and TagTypeKind TTK_Interface rather than
the ms_interface attribute?

Yeah, that's what I was suggesting. That lets arbitrary clients remember
the exact intent a little easier.

I'll work up a patch that works that way and post when ready.

Thanks.

Note that MS diagnostics refer to __interface as 'struct', e.g.

  int.cpp(4) : warning C4099: 'Y' : type name first seen using 'struct'
  now seen using 'class'

Does that matter at all?

That's interesting. Maybe MSVC just defines this as a macro? Have you tried
running it through the preprocessor? Although it's more likely that they're
just being lazy about their diagnostics.

No, it's not an actual macro - and can't be, because of the way it
forces member function decls to be pure virtual.

It could have been a macro the same way you had it implemented, i.e. with
some extra keyword or attribute after 'struct'. But it did seem unlikely.

(BTW, I dropped you from the 'To' line this time and sent direct to the
list address only, which doesn't seem to cause messages to be dropped -
let me know.)

Well, it worked, thanks. :slight_smile:

John.