Qpid Proton
  1. Qpid Proton
  2. PROTON-67

Porting Issue -- Initialization with braces is not supported by Visual Studio.

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.4
    • Component/s: proton-c
    • Labels:
    • Environment:
      Windows using Visual Studio 2010

      Description

      In the Windows port, I used ifdef(s) for the initializations to keep the current code for Linux, and added code to compile in Visual Studio. If there is no objection, maybe we could replace the current initialization code with the Visual Studio code and remove the #ifdef(s).
      Here's some examples:

      Eample1

      ssize_t pn_data_encode(pn_data_t *data, char *bytes, size_t size)
      {
      #ifndef _WINDOWS
      pn_atoms_t latoms =

      {.size=data->size + data->extras, .start=atoms}

      ;
      #else
      pn_atoms_t latoms;
      latoms.size = data->size + data->extras;
      latoms.start = atoms;
      #endif

      --------------------
      Example 2

      #ifndef _WINDOWS
      return (pn_bytes_t)

      {size, start}

      ;
      #else
      pn_bytes_t pnBytes;
      pnBytes.size = size;
      pnBytes.start= start;
      return pnBytes;
      #endif
      --------------------
      Example 3

      pn_do_transfer()

      #ifndef _WINDOWS
      delivery = pn_delivery(link, pn_dtag(tag.start, tag.size));
      #else
      pn_delivery_tag_t delivt;
      delivt.bytes = tag.start;
      delivt.size = tag.size;
      delivery = pn_delivery(link, delivt);
      #endif

        Activity

        Hide
        Cliff Jansen added a comment -

        I'm not sure there is any way to manage this other than as you suggest.

        The downside is that the code becomes less compact and readable. Somehow, even if fancy macros could restore the compactness, I am skeptical such macros would really make for clearer code. So I am in favour of this approach in the absence of a better suggestion.

        Note that your changes have nothing to do with "WINDOWS". It is all about "_cplusplus". The same for PROTON-57. In your future examples, it would be helpful if you kept that clear.

        As a corollary, since these proposed changes are about languages and not platforms, the implication is that it is less difficult for developers to code to both C99 and C++ without having multiple development machines or rely on external CI notifications. Linux proton developers should be able develop in gcc (C99) and sanity check with g++. Likewise, Visual Studio proton developers should be able to develop in their IDE and sanity check with a third party C99 toolchain. End users can just use their favourite compiler.

        Just as there are C++ coding guidelines for qpid cpp

        https://cwiki.apache.org/qpid/qpid-c-documentation.html

        there could be similar info for proton C: the dual compilation environment, gotchas, and guidelines. I would volunteer to draft these pages as part of implementing PROTON-57 and PROTON-67.

        Show
        Cliff Jansen added a comment - I'm not sure there is any way to manage this other than as you suggest. The downside is that the code becomes less compact and readable. Somehow, even if fancy macros could restore the compactness, I am skeptical such macros would really make for clearer code. So I am in favour of this approach in the absence of a better suggestion. Note that your changes have nothing to do with " WINDOWS". It is all about " _cplusplus". The same for PROTON-57 . In your future examples, it would be helpful if you kept that clear. As a corollary, since these proposed changes are about languages and not platforms, the implication is that it is less difficult for developers to code to both C99 and C++ without having multiple development machines or rely on external CI notifications. Linux proton developers should be able develop in gcc (C99) and sanity check with g++. Likewise, Visual Studio proton developers should be able to develop in their IDE and sanity check with a third party C99 toolchain. End users can just use their favourite compiler. Just as there are C++ coding guidelines for qpid cpp https://cwiki.apache.org/qpid/qpid-c-documentation.html there could be similar info for proton C: the dual compilation environment, gotchas, and guidelines. I would volunteer to draft these pages as part of implementing PROTON-57 and PROTON-67 .
        Hide
        Andrew Stitcher added a comment -

        Is it worth pointing out that regular brace initialisation of structures has been in C for a very long time and is certainly supported by Visual Studio c and c++.

        So it's only necessary to change
        pn_atoms_t latoms =

        {.size=data->size + data->extras, .start=atoms}

        ;
        into
        pn_atoms_t latoms =

        {data->size + data->extras, atoms}

        ;

        Of course for many other structs it won't be so simple and will require setting default values for uninitialised members in the middle of initialised values. Also the code will no longer be as clear or safe.

        Show
        Andrew Stitcher added a comment - Is it worth pointing out that regular brace initialisation of structures has been in C for a very long time and is certainly supported by Visual Studio c and c++. So it's only necessary to change pn_atoms_t latoms = {.size=data->size + data->extras, .start=atoms} ; into pn_atoms_t latoms = {data->size + data->extras, atoms} ; Of course for many other structs it won't be so simple and will require setting default values for uninitialised members in the middle of initialised values. Also the code will no longer be as clear or safe.
        Hide
        Andrew Stitcher added a comment -

        I also second Cliff's point about the #ifdef test being incorrect: Id say that the correct test would probably be something like:

        #if defined(_STDC_VERSION) && __STDC_VERSION_ >= 199901L
        ...[existing code]
        #elif __cplusplus
        ...[replacement code]
        #else
        #error Unsupported compiiler
        #endif

        We could protect ourselves immediately from unsupported environments by using just the STDC_VERSION test with the else and error.

        [There is a useful stackoverflow article about detecting C language version here - http://stackoverflow.com/questions/2115867/is-there-a-define-for-c99]

        Show
        Andrew Stitcher added a comment - I also second Cliff's point about the #ifdef test being incorrect: Id say that the correct test would probably be something like: #if defined(_ STDC_VERSION) && __STDC_VERSION _ >= 199901L ... [existing code] #elif __cplusplus ... [replacement code] #else #error Unsupported compiiler #endif We could protect ourselves immediately from unsupported environments by using just the STDC_VERSION test with the else and error. [There is a useful stackoverflow article about detecting C language version here - http://stackoverflow.com/questions/2115867/is-there-a-define-for-c99]
        Hide
        Mary hinton added a comment -

        You're right. I should have said Visual Studio does not support "Designated initializers"
        Thanks,
        Mary

        Show
        Mary hinton added a comment - You're right. I should have said Visual Studio does not support "Designated initializers" Thanks, Mary
        Hide
        Mary hinton added a comment -

        Hi Cliff and Andrew,
        Whatever you want to use for the #ifdef is fine, as long as it distinguishes the Visual Studio port. I'll change the #ifdef's accordingly.

        I did find this about different compilers:

        "Notice that not all compliant compilers provides the correct pre-defined macros. For example, Microsoft Visual C++ does not define _STDC, or Sun Workshop 4.2 supports C94 without setting __STDC_VERSION_ to the proper value. Extra checks for such compilers must be added.

        Notice that some compilers, such as the HP aC+, use the value 199707L to indicate the C98 standard. This value was used in an earlier proposal of the C+98 standard."

        Show
        Mary hinton added a comment - Hi Cliff and Andrew, Whatever you want to use for the #ifdef is fine, as long as it distinguishes the Visual Studio port. I'll change the #ifdef's accordingly. I did find this about different compilers: "Notice that not all compliant compilers provides the correct pre-defined macros. For example, Microsoft Visual C++ does not define _ STDC , or Sun Workshop 4.2 supports C94 without setting __STDC_VERSION _ to the proper value. Extra checks for such compilers must be added. Notice that some compilers, such as the HP aC+ , use the value 199707L to indicate the C 98 standard. This value was used in an earlier proposal of the C +98 standard."
        Hide
        Cliff Jansen added a comment -

        fixed in proton-159

        Show
        Cliff Jansen added a comment - fixed in proton-159

          People

          • Assignee:
            Cliff Jansen
            Reporter:
            Mary hinton
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development