Harmony
  1. Harmony
  2. HARMONY-3310

[drlvm][doc] interpreter headers should be complemented with code comments

    Details

    • Type: Improvement Improvement
    • Status: In Progress
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: DRLVM
    • Labels:
      None
    • Estimated Complexity:
      Moderate

      Description

      header files located in drlvm\trunk\vm\interpreter\src have very scarse comments. i suggest that we improve those.

      1. 6_interpreter_headers.patch
        51 kB
        Ilya Leviev
      2. 3_interpreter_headers.patch
        20 kB
        Svetlana Konovalova
      3. 3_interpreter_headers.patch
        20 kB
        Ilya Leviev
      4. interpreter_h_comments.patch
        0.7 kB
        Ilya Leviev
      5. interpreter_imports_h_comments.patch
        3 kB
        Ilya Leviev
      6. interpreter_exports_h_comments.patch
        11 kB
        Ilya Leviev
      7. interp_defs_comments.patch
        8 kB
        Nadya Morozova

        Activity

        Hide
        Nadya Morozova added a comment -

        the patch adds some comments to a part of the interp_defs.h file. this patch does not resolve the whole issue, more comments needed.
        the patch was created with the help of Ivan Volosyuk, his permission to use the patch has been granted.

        Show
        Nadya Morozova added a comment - the patch adds some comments to a part of the interp_defs.h file. this patch does not resolve the whole issue, more comments needed. the patch was created with the help of Ivan Volosyuk, his permission to use the patch has been granted.
        Hide
        Ilya Leviev added a comment -

        add comments to vm\include\interpreter_exports.h

        Show
        Ilya Leviev added a comment - add comments to vm\include\interpreter_exports.h
        Hide
        Ilya Leviev added a comment -

        adding comments to vm\include\interpreter_imports.h

        Show
        Ilya Leviev added a comment - adding comments to vm\include\interpreter_imports.h
        Hide
        Ilya Leviev added a comment -

        adding comments to vm\include\interpreter.h

        Show
        Ilya Leviev added a comment - adding comments to vm\include\interpreter.h
        Hide
        Ilya Leviev added a comment -

        update patch for vm\include\interpreter headers.

        Show
        Ilya Leviev added a comment - update patch for vm\include\interpreter headers.
        Hide
        Svetlana Konovalova added a comment -

        Ilya,
        thanks for adding file descriptions. but still we have certain issues to resolve.
        would you be so kind to fix the following?

        1. Add missing comments for the following:

        • [interpreter_imports.h] typedef struct ManagedObject ManagedObject
        • [interpreter_exports.h] typedef struct FrameHandle FrameHandle

        2. Add missing interpreter_exports.h parameter types, for example to:

        • @param bsp - the pointer to the register
        • @param reg - the register

        3. [interpreter_exports.h]
        Ln 262 "jvmtiEnv's"
        Need to add the type of element it is. Is it an option, a variable, or what? And we shouldn't use plural form for a code name.

        4. [interpreter_exports.h]
        Ln 326 Returns <code>TRUE</code> if interpreter table.
        It defiantly should be paraphrased. The original version does not make sense.

        5. [interpreter_ exports.h]
        The definition of the group does not include the description of what Open Interfaces include. Do they include the interface functions defined by the OPEN design? Could we provide the link or something?

        If you have any questions, do not hesitate to contact me any time.
        Thanks in advance.

        Sveta

        Show
        Svetlana Konovalova added a comment - Ilya, thanks for adding file descriptions. but still we have certain issues to resolve. would you be so kind to fix the following? 1. Add missing comments for the following: [interpreter_imports.h] typedef struct ManagedObject ManagedObject [interpreter_exports.h] typedef struct FrameHandle FrameHandle 2. Add missing interpreter_exports.h parameter types, for example to: @param bsp - the pointer to the register @param reg - the register 3. [interpreter_exports.h] Ln 262 "jvmtiEnv's" Need to add the type of element it is. Is it an option, a variable, or what? And we shouldn't use plural form for a code name. 4. [interpreter_exports.h] Ln 326 Returns <code>TRUE</code> if interpreter table. It defiantly should be paraphrased. The original version does not make sense. 5. [interpreter_ exports.h] The definition of the group does not include the description of what Open Interfaces include. Do they include the interface functions defined by the OPEN design? Could we provide the link or something? If you have any questions, do not hesitate to contact me any time. Thanks in advance. Sveta
        Hide
        Nadya Morozova added a comment -

        committed the latest patch for the three headers. will wait for remaining fixes and more comments on other interpreter-related files before closing the issue.
        keep up the good work.

        Show
        Nadya Morozova added a comment - committed the latest patch for the three headers. will wait for remaining fixes and more comments on other interpreter-related files before closing the issue. keep up the good work.
        Hide
        Svetlana Konovalova added a comment -

        Ilya,
        Here are suggestions on how to improve interp_vm_helpers.h and interp_native.h files.

        interp_vm_helpers.h
        -Add detailed description
        -Add brief description

        interp_native.h
        -Add detailed description
        -Add brief description

        • Add missing Define Documentation and Function Documentation Descriptions

        If you have questions, just let me know. would be glad to help.

        Cheers,
        Sveta

        Show
        Svetlana Konovalova added a comment - Ilya, Here are suggestions on how to improve interp_vm_helpers.h and interp_native.h files. interp_vm_helpers.h -Add detailed description -Add brief description interp_native.h -Add detailed description -Add brief description Add missing Define Documentation and Function Documentation Descriptions If you have questions, just let me know. would be glad to help. Cheers, Sveta
        Hide
        Ilya Leviev added a comment -

        The main part of the 6_interpreter_headers.patch adds documentation for interpreter internal header files.
        Also it includes some corrections in comments of interpreter external headers.

        There are following areas for documentation improvement suggested by Svetlana.

        interpreter_imports.h

         Add missing Typedef description (typedef struct ManagedObject ManagedObject)

        interpreter_exports.h

         Add missing Typedef description (typedef struct FrameHandle FrameHandle)
         Add missing [in]/[out] parameter types, for example to:

        • @param bsp - the pointer to the register
        • @param reg - the register
           Ln 264 "jvmtiEnv's"
          Need to add the type of element it is. Is it an option, a variable, or what? And we shouldn't use plural form for a code name.
           Ln 324 Returns <code>TRUE</code> if interpreter table.
          It defiantly should be paraphrased. The original version does not make sense.
           The definition of the group does not include the description of what Open Interfaces include. Do they include the interface functions defined by the OPEN design? Could we provide the link or something?

        interp_defs.h

         Add missing Member Data and Define Documentation descriptions

        Many thanks to Svetlana for review and suggestions during work with interpreter documentation.

        Show
        Ilya Leviev added a comment - The main part of the 6_interpreter_headers.patch adds documentation for interpreter internal header files. Also it includes some corrections in comments of interpreter external headers. There are following areas for documentation improvement suggested by Svetlana. interpreter_imports.h  Add missing Typedef description (typedef struct ManagedObject ManagedObject) interpreter_exports.h  Add missing Typedef description (typedef struct FrameHandle FrameHandle)  Add missing [in] / [out] parameter types, for example to: @param bsp - the pointer to the register @param reg - the register  Ln 264 "jvmtiEnv's" Need to add the type of element it is. Is it an option, a variable, or what? And we shouldn't use plural form for a code name.  Ln 324 Returns <code>TRUE</code> if interpreter table. It defiantly should be paraphrased. The original version does not make sense.  The definition of the group does not include the description of what Open Interfaces include. Do they include the interface functions defined by the OPEN design? Could we provide the link or something? interp_defs.h  Add missing Member Data and Define Documentation descriptions Many thanks to Svetlana for review and suggestions during work with interpreter documentation.
        Hide
        Nadya Morozova added a comment -

        Hi,
        i've applied the 6file patch, here are a few comments:

        interpreter_exports.h: line 308 and comment above it: had a conflict during merge, is it void (stack_dump) (VM_thread); or void (stack_dump) (FILE *, VM_thread); ?
        interpreter_imports.h and interpreter.h have been chagned earlier.

        i committed the patch for internal headers at revision: 536108
        please resolve the remaining issues and we'll close the issue. thanks for the work you're doing.

        Show
        Nadya Morozova added a comment - Hi, i've applied the 6file patch, here are a few comments: interpreter_exports.h: line 308 and comment above it: had a conflict during merge, is it void ( stack_dump) (VM_thread ); or void ( stack_dump) (FILE *, VM_thread ); ? interpreter_imports.h and interpreter.h have been chagned earlier. i committed the patch for internal headers at revision: 536108 please resolve the remaining issues and we'll close the issue. thanks for the work you're doing.

          People

          • Assignee:
            Nadya Morozova
            Reporter:
            Nadya Morozova
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:

              Development