> See the following code:
> (... trimmed ...)

Hao, thanks for explanations.  Now I understand everything.

But please allow me to make some notes.

> struct foo
>   {
>     struct bar
>       {
>         char one;
>       };
>     char two;
>   };
> ...
>     // This prints 2 when compiled as C, because the member `one` of
>     // `struct bar` is a member of the enclosing struct i.e. `foo`.
>     // This prints 1 when compiled as C++.
>     printf("%d\n", (int)sizeof(struct foo));

With C compilers that prints ``2'' **sometimes** :-)  Because of
`-fms-extensions`[1].

When it's on (and for mingw it's on by default), compiling your sample
with the following command:

```
$ x86_64-w64-mingw32-gcc -x c -std=c17 -O2 -S -fms-extensions \
-o - sample.c
```

results in:

```
movl    $2, %edx  ; `sizeof(struct foo)`
call    printf
```

And with `-fms-extensions` turned off:

```
$ x86_64-w64-mingw32-gcc -x c -std=c17 -O2 -S -fno-ms-extensions \
-o - sample17.c
...
movl    $1, %edx
call    printf
```

This is why I've got inconsistent results with different C
compilers[2][3].  Therefore, the following code (the second version of
the patch):

```
struct SSVARIANT {
  /* declaration of some fields */
  struct _Time2Val {
    DBTIME2 tTime2Val;
    BYTE bScale;
  };
  ...
};
```

isn't always valid for a C compiler because of:

a) some C compilers know about `-fms-extensions`, and if the latter is
used, the `_Time2Val` becomes[1] anonymous structure, and, as a
consequence, the members of the `_Time2Val` are considered to be members
of the `SSVARIANT`[4].  This, I believe, changes layout of structure in
memory, when compare it with "original" structure (from my initial
patch).  Moreover, when several structures like the `_Time2Val` and
`_DateTimeVal` have members of the same name (the `bScale` in this
case), we end up with a compilation error[3].

b) for all other C compilers there's no much sense to declare structures
like the `_Time2Val` within the `SSVARIANT`.  Semantically it declares
all those structures at scope where the header is included (6.2.1 Scopes
of identifiers).  The original version of the patch provides some
"logical locality" of declarations of the tags like `_Time2Val`,
combining a structure declaration with field definition, but it doesn't
change semantics: the tags are available after the completion of their
corresponding declarators at scope where the header is included.

Therefore, as we've already discussed, for a C compiler we may declare
the `_Time2Val` structure and others just before the `SSVARIANT`
structure.  But for a C++ compiler we should avoid that, I believe,
because original API of msoledbsql.h file doesn't have such a type like
`::_Time2Val`.  We should preserve the
`SSVARIANT::{unnamed union}::_Time2Val` type, but there's another broken
"feature" of Microsoft Visual C++.

Errors which Liu Hao found[5] when compiling the code with g++, are the
result of:

> 10.4.1 Anonymous unions [class.union.anon]
> 1 ... Each member-declaration in the member-specification of an
> anonymous union shall either define a non-static data member or be a
> static_assert-declaration. [Note: Nested types, anonymous unions, and
> functions cannot be declared within an anonymous union. -end note]

But MSVC ignores that and compiles the following code successfully:

```
struct SSVARIANT
{
  union
  {
    struct _Time2Val
    {
    } Time2Val;
  };
};
```

GNU C++ is not compatible with such behavior.  Moreover, GNU C++ doesn't
allow `typedef`s within anonymous unions (again because of
[class.union.anon]).  Therefore, we can't declare a type like
`SSVARIANT::{unnamed union}::_Time2Val` with GNU C++.  We may declare[5]
all such types directly inside `SSVARIANT` (for C++ **only** -- see
above), creating the types like `SSVARIANT::_Time2Val`, but we change
original API of msoledbsql.h again -- there are no such types there.
Thus, I believe, it's impossible to reproduce that (original) code,
which is specific to Microsoft "extensions", in a form compatible with
the C++ standard.

We may move all those types (`_Time2Val` and others) out from the
`SSVARIANT` completely (just as we already did that for C, we may do
that for C++), and end up with `::_Time2Val`, `::_DateTimeVal`, etc. for
a C++ compiler.  Yes, it also modifies the original API, but unify the
code which declares the `SSVARIANT` between C and C++.  Though I believe
that having the types scoped to the `SSVARIANT` (`SSVARIANT::_Time2Val`,
`SSVARIANT::_DateTimeVal`, etc.) is the lesser of two evils.  And for
the latter I have only not-very-beautiful solution with a preprocessor
macro.

> The typedef there is to make the qualified name `SSVARIANT::_Time2Val`
> valid in C++, as in the old, non-working code.

I see, thanks once again for explanation!

[1]: https://gcc.gnu.org/onlinedocs/gcc/Unnamed-Fields.html
"6.63 Unnamed Structure and Union Fields"
[2]: https://sourceforge.net/p/mingw-w64/mailman/message/36600915/
[3]: https://sourceforge.net/p/mingw-w64/mailman/message/36599160/
[4]: ISO/IEC 9899:2017, 6.7.2.1 Structure and union specifiers, 13.
[5]: https://sourceforge.net/p/mingw-w64/mailman/message/36598884/


_______________________________________________
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public

Reply via email to