Merge pull request #1211 from aarongreig/aaron/allowHandlesInStructs

Update scripts and loader template to allow object handles in structs.

Author: aarongreig

scripts/core/CONTRIB.rst

@@ -225,6 +225,29 @@ arguments:
 *   Pointer to handle arguments, such as out parameters, are prefixed with
     ``ph`` i.e. ``phQueue``.
 
+Limitations
+-----------
+
+There are some limitations on the patterns our spec generator can handle. These
+limitations are due to convenience of implementation rather than design: if
+they are preventing you from implementing a feature please open an issue and we
+will be happy to try and accommodate your use case. Otherwise beware of the
+following:
+
+* A function parameter or struct member which is a struct type that has any of
+  the following members in its type definition must not have the ``[range]``
+  tag:
+
+  * An object handle with the ``[range]`` tag
+
+  * A struct type with the ``[range]`` tag that has an object handle member
+
+* A struct member which is a pointer to a struct type must not have the
+  ``[optional]`` tag if that struct (or any of its members, recursively) has
+  an object handle member in its definition.
+
+* A struct member which is an object handle must not have the ``[out]`` tag.
+
 Forks and Pull Requests
 =======================
 

scripts/parse_specs.py

@@ -83,7 +83,7 @@ def _get_etor_value(value, prev):
 """
     validate documents meet some basic (easily detectable) requirements of code generation
 """
-def _validate_doc(f, d, tags, line_num):
+def _validate_doc(f, d, tags, line_num, meta):
     is_iso = lambda x : re.match(r"[_a-zA-Z][_a-zA-Z0-9]{0,30}", x)
 
     def __validate_ordinal(d):
@@ -265,7 +265,35 @@ def __validate_base(d):
             elif type_traits.is_properties(d['name']) and not d.get('base', "").endswith("base_properties_t"):
                 raise Exception("'base' must be '%s_base_properties_t': %s"%(namespace, d['name']))
 
-    def __validate_members(d, tags):
+    def __validate_struct_range_members(name, members, meta):
+        def has_handle(members, meta):
+            for m in members:
+                if type_traits.is_handle(m):
+                    return True
+                if type_traits.is_struct(m, meta):
+                    return has_handle(
+                        type_traits.get_struct_members(m['type']), meta)
+            return False
+
+        for m in members:
+            if param_traits.is_range(m) and type_traits.is_handle(m['type']):
+                raise Exception(
+                    f"struct range {name} must not contain range of object handles {m['name']}"
+                )
+            if type_traits.is_struct(m['type'], meta):
+                member_members = type_traits.get_struct_members(
+                    m['type'], meta)
+                # We can't handle a range of structs with handles within a range of structs
+                if param_traits.is_range(m) and has_handle(
+                        member_members, meta):
+                    raise Exception(
+                        f"struct range {m['name']} is already within struct range {name}, and must not contain an object handle"
+                    )
+                # We keep passing the original name so we can report it in
+                # exception messages.
+                __validate_struct_range_members(name, member_members, meta)
+
+    def __validate_members(d, tags, meta):
         if 'members' not in d:
             raise Exception("'%s' requires the following sequence of mappings: {`members`}"%d['type'])
 
@@ -286,21 +314,28 @@ def __validate_members(d, tags):
             if not annotation:
                 raise Exception(prefix+"'desc' must start with {'[in]', '[out]', '[in,out]'}")
 
-            if type_traits.is_handle(item['type']):
-                raise Exception(prefix+"'type' must not be '*_handle_t': %s"%item['type'])
-
             if item['type'].endswith("flag_t"):
                 raise Exception(prefix+"'type' must not be '*_flag_t': %s"%item['type'])
 
             if d['type'] == 'union'and item.get('tag') is None:
                 raise Exception(prefix + f"union member {item['name']} must include a 'tag' annotation")
 
+            if type_traits.is_struct(item['type'],
+                                     meta) and param_traits.is_range(item):
+                member_members = type_traits.get_struct_members(
+                    item['type'], meta)
+                __validate_struct_range_members(item['name'], member_members,
+                                                meta)
+
+            if type_traits.is_handle(item['type']) and param_traits.is_output(item):
+                raise Exception(prefix + f"struct member {item['name']} is an object handle, so it must not be have the [out] tag")
+
             ver = __validate_version(item, prefix=prefix, base_version=d_ver)
             if ver < max_ver:
                 raise Exception(prefix+"'version' must be increasing: %s"%item['version'])
             max_ver = ver
 
-    def __validate_params(d, tags):
+    def __validate_params(d, tags, meta):
         if 'params' not in d:
             raise Exception("'function' requires the following sequence of mappings: {`params`}")
 
@@ -347,6 +382,11 @@ def __validate_params(d, tags):
                 if not has_queue:
                     raise Exception(prefix+"bounds must only be used on entry points which take a `hQueue` parameter")
 
+            if type_traits.is_struct(item['type'],
+                                     meta) and param_traits.is_range(item):
+                members = type_traits.get_struct_members(item['type'], meta)
+                __validate_struct_range_members(item['name'], members, meta)
+
             ver = __validate_version(item, prefix=prefix, base_version=d_ver)
             if ver < max_ver:
                 raise Exception(prefix+"'version' must be increasing: %s"%item['version'])
@@ -421,7 +461,7 @@ def __validate_union_tag(d):
                 __validate_union_tag(d)
             __validate_type(d, 'name', tags)
             __validate_base(d)
-            __validate_members(d, tags)
+            __validate_members(d, tags, meta)
             __validate_details(d)
             __validate_ordinal(d)
             __validate_version(d)
@@ -435,7 +475,7 @@ def __validate_union_tag(d):
             else:
                 __validate_name(d, 'name', tags, case='camel')
 
-            __validate_params(d, tags)
+            __validate_params(d, tags, meta)
             __validate_details(d)
             __validate_ordinal(d)
             __validate_version(d)
@@ -893,7 +933,7 @@ def parse(section, version, tags, meta, ref):
 
         for i, d in enumerate(docs):
             d = _preprocess(d)
-            if not _validate_doc(f, d, tags, line_nums[i]):
+            if not _validate_doc(f, d, tags, line_nums[i], meta):
                 continue
 
             d = _filter_version(d, float(version))

scripts/templates/helper.py

@@ -231,15 +231,15 @@ def is_array(cls, name):
             return True if re.match(cls.RE_ARRAY, name) else False
         except:
             return False
-        
+
     @classmethod
     def get_array_length(cls, name):
         if not cls.is_array(name):
             raise Exception("Cannot find array length of non-array type.")
 
         match = re.match(cls.RE_ARRAY, name)
         return match.groups()[1]
-    
+
     @classmethod
     def get_array_element_type(cls, name):
         if not cls.is_array(name):
@@ -248,6 +248,14 @@ def get_array_element_type(cls, name):
         match = re.match(cls.RE_ARRAY, name)
         return match.groups()[0]
 
+    @staticmethod
+    def get_struct_members(type_name, meta):
+        struct_type = _remove_const_ptr(type_name)
+        if not struct_type in meta['struct']:
+            raise Exception(
+                f"Cannot return members of non-struct type {struct_type}")
+        return meta['struct'][struct_type]['members']
+
 """
     Extracts traits from a value name
 """
@@ -427,7 +435,7 @@ def is_bounds(cls, item):
             return True if re.match(cls.RE_BOUNDS, item['desc']) else False
         except:
             return False
-    
+
     @classmethod
     def tagged_member(cls, item):
         try:
@@ -1266,6 +1274,120 @@ def get_loader_prologue(namespace, tags, obj, meta):
 
     return prologue
 
+
+"""
+Private:
+    Takes a list of struct members and recursively searches for class handles.
+    Returns a list of class handles with access chains to reach them (e.g.
+    "struct_a->struct_b.handle"). Also handles ranges of class handles and
+    ranges of structs with class handle members, although the latter only works
+    to one level of recursion i.e. a range of structs with a range of structs
+    with a handle member will not work.
+"""
+def get_struct_handle_members(namespace,
+                              tags,
+                              meta,
+                              members,
+                              parent='',
+                              is_struct_range=False):
+    handle_members = []
+    for m in members:
+        if type_traits.is_class_handle(m['type'], meta):
+            m_tname = _remove_const_ptr(subt(namespace, tags, m['type']))
+            m_objname = re.sub(r"(\w+)_handle_t", r"\1_object_t", m_tname)
+            # We can deal with a range of handles, but not if it's in a range of structs
+            if param_traits.is_range(m) and not is_struct_range:
+                handle_members.append({
+                    'parent': parent,
+                    'name': m['name'],
+                    'obj_name': m_objname,
+                    'type': m_tname,
+                    'range_start': param_traits.range_start(m),
+                    'range_end': param_traits.range_end(m)
+                })
+            else:
+                handle_members.append({
+                    'parent': parent,
+                    'name': m['name'],
+                    'obj_name': m_objname,
+                    'optional': param_traits.is_optional(m)
+                })
+        elif type_traits.is_struct(m['type'], meta):
+            member_struct_members = type_traits.get_struct_members(
+                m['type'], meta)
+            if param_traits.is_range(m):
+                # If we've hit a range of structs we need to start a new recursion looking
+                # for handle members. We do not support range within range, so skip that
+                if is_struct_range:
+                    continue
+                range_handle_members = get_struct_handle_members(
+                    namespace, tags, meta, member_struct_members, '', True)
+                if range_handle_members:
+                    handle_members.append({
+                        'parent': parent,
+                        'name': m['name'],
+                        'type': subt(namespace, tags, _remove_const_ptr(m['type'])),
+                        'range_start': param_traits.range_start(m),
+                        'range_end': param_traits.range_end(m),
+                        'handle_members': range_handle_members
+                    })
+            else:
+                # If it's just a struct we can keep recursing in search of handles
+                m_is_pointer = type_traits.is_pointer(m['type'])
+                new_parent_deref = '->' if m_is_pointer else '.'
+                new_parent = m['name'] + new_parent_deref
+                handle_members += get_struct_handle_members(
+                    namespace, tags, meta, member_struct_members, new_parent,
+                    is_struct_range)
+
+    return handle_members
+
+
+"""
+Public:
+    Strips a string of all dereferences.
+
+    This is useful in layer templates for creating unique variable names. For
+    instance if we need to copy pMyStruct->member.hObject out of a function
+    parameter into a local variable we use this to get the unique (or at least
+    distinct from any other parameter we might copy) variable name
+    pMyStructmemberhObject.
+"""
+def strip_deref(string_to_strip):
+    string_to_strip = string_to_strip.replace('.', '')
+    return string_to_strip.replace('->', '')
+
+
+"""
+Public:
+    Takes a function object and recurses through its struct parameters to return
+    a list of structs that have handle object members the loader will need to
+    convert.
+"""
+def get_object_handle_structs_to_convert(namespace, tags, obj, meta):
+    structs = []
+    params = _filter_param_list(obj['params'], ["[in]"])
+
+    for item in params:
+        if type_traits.is_struct(item['type'], meta):
+            members = type_traits.get_struct_members(item['type'], meta)
+            handle_members = get_struct_handle_members(namespace, tags, meta,
+                                                       members)
+            if handle_members:
+                name = subt(namespace, tags, item['name'])
+                tname = _remove_const_ptr(subt(namespace, tags, item['type']))
+                struct = {
+                    'name': name,
+                    'type': tname,
+                    'optional': param_traits.is_optional(item),
+                    'members': handle_members
+                }
+
+                structs.append(struct)
+
+    return structs
+
+
 """
 Public:
     returns an enum object with the given name

scripts/templates/ldrddi.cpp.mako

@@ -177,6 +177,87 @@ namespace ur_loader
                 pPropSizeRet = &sizeret;
         %endif
 
+        ## Here we deal with handles buried inside struct type parameters. First
+        ## we create a local copy of the struct, then we convert all the handles
+        ## in that local copy and set the parameter to point to it before forwarding
+        ## it to the final API call.
+        <% handle_structs = th.get_object_handle_structs_to_convert(n, tags, obj, meta) %>
+        %if handle_structs:
+        // Deal with any struct parameters that have handle members we need to convert.
+        %for struct in handle_structs:
+            %if struct['optional']:
+            ${struct['type']} ${struct['name']}Local = {};
+            if(${struct['name']})
+                ${struct['name']}Local = *${struct['name']};
+            %else:
+            auto ${struct['name']}Local = *${struct['name']};
+            %endif
+        %endfor
+
+        %for struct in handle_structs:
+        %for member in struct['members']:
+            ## If this member has a handle_members field that means it's a range of
+            ## structs which each contain a handle to convert.
+            %if 'handle_members' in member:
+                ## we use the parent info stripped of derefs for a unique variable name
+                <%
+                parent_no_deref = th.strip_deref(member['parent'])
+                range_vector_name = struct['name'] + parent_no_deref + member['name']
+                ## we need to check if range bounds are literals or variables: variables
+                ## need the full reference chain prepended to them
+                range_start = member['range_start']
+                if not re.match(r"[0-9]+$", range_start):
+                    range_start = struct['name'] + "->" + member['parent'] + range_start
+                range_end = member['range_end']
+                if not re.match(r"[0-9]+$", range_end):
+                    range_end = struct['name'] + "->" + member['parent'] + range_end %>
+                std::vector<${member['type']}> ${range_vector_name};
+                for(uint32_t i = ${range_start}; i < ${range_end}; i++) {
+                    ${member['type']} NewRangeStruct = ${struct['name']}Local.${member['parent']}${member['name']}[i];
+                    %for handle_member in member['handle_members']:
+                    %if handle_member['optional']:
+                    if(NewRangeStruct.${handle_member['parent']}${handle_member['name']})
+                    %endif
+                    NewRangeStruct.${handle_member['parent']}${handle_member['name']} =
+                        reinterpret_cast<${handle_member['obj_name']}*>(
+                            NewRangeStruct.${handle_member['parent']}${handle_member['name']})
+                            ->handle;
+                    %endfor
+
+                    ${range_vector_name}.push_back(NewRangeStruct);
+                }
+                ${struct['name']}Local.${member['parent']}${member['name']} = ${range_vector_name}.data();
+            ## If the member has range_start then its a range of handles
+            %elif 'range_start' in member:
+                ## we use the parent info stripped of derefs for a unique variable name
+                <%
+                parent_no_deref = th.strip_deref(member['parent'])
+                range_vector_name = struct['name'] + parent_no_deref + member['name'] %>
+                std::vector<${member['type']}> ${range_vector_name};
+                for(uint32_t i = 0;i < ${struct['name']}->${member['parent']}${member['range_end']};i++) {
+                    ${range_vector_name}.push_back(reinterpret_cast<${member['obj_name']}*>(${struct['name']}->${member['parent']}${member['name']}[i])->handle);
+                }
+                ${struct['name']}Local.${member['parent']}${member['name']} = ${range_vector_name}.data();
+            %else:
+                %if member['optional']:
+                if(${struct['name']}Local.${member['parent']}${member['name']})
+                %endif
+                ${struct['name']}Local.${member['parent']}${member['name']} =
+                    reinterpret_cast<${member['obj_name']}*>(
+                        ${struct['name']}Local.${member['parent']}${member['name']})->handle;
+            %endif
+        %endfor
+        %endfor
+
+        // Now that we've converted all the members update the param pointers
+        %for struct in handle_structs:
+            %if struct['optional']:
+            if(${struct['name']})
+            %endif
+            ${struct['name']} = &${struct['name']}Local;
+        %endfor
+        %endif
+
         // forward to device-platform
         %if add_local:
         result = ${th.make_pfn_name(n, tags, obj)}( ${", ".join(th.make_param_lines(n, tags, obj, format=["name", "local"], replacements=param_replacements))} );