Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Go YDK fails to build unique types for cascading typedef statements in Yang model #675

Closed
ygorelik opened this issue Feb 1, 2018 · 4 comments

Comments

@ygorelik
Copy link
Collaborator

ygorelik commented Feb 1, 2018

Steps to Reproduce

Include the following statements (taken from NXOS) into Yang model and then build go bundle:

typedef comp_InstType {
    type enumeration {
        enum unknown {
            value 0;   // Unknown
        }
        enum phys {
            value 1;     // Baremetal Host
        }
        enum virt {
            value 2;     // Virtual Machine
        }
        enum hv {
            value 3;   // Hypervisor Host
        }
    }
    default "unknown";
}

typedef comp_NicInstType {
    type comp_InstType;
}

In this model the "comp_NicInstType" type "cascades" or "repeats" definition of "comp_InstType"

The Go API builder creates duplicate statements like this one, which causes compiler syntax error

// CompInstType
type CompInstType int
const (
CompInstType_unknown CompInstType = 0
CompInstType_phys CompInstType = 1
CompInstType_virt CompInstType = 2
CompInstType_hv CompInstType = 3
)

The type CompNicInstType does not appear in the Go code.

Additional notes

The issue does not appear in C++ and Python bundle builders.

Logs

TBD

System Information

Ubuntu 14.1

@ghost ghost added bug bundle labels Feb 1, 2018
@ghost ghost added the golang label Feb 16, 2018
@ylil93
Copy link
Contributor

ylil93 commented Mar 7, 2018

Also:
duplicate type and method definitions under cisco_ios_xe/native
589301:type Native_Interface_Vlan_CiscoIOSXEInterfacesSwitchportConf struct {
...
606925:type Native_Interface_Vlan_CiscoIOSXEInterfacesSwitchportConf struct {

ylil93 added a commit to ylil93/ydk-gen that referenced this issue Apr 2, 2018
- update generator to differentiate typedefs with the same base type
- added sanity test case
- generated documentation is wrong (api model/builder issue)
ylil93 added a commit to ylil93/ydk-gen that referenced this issue Apr 3, 2018
- update generator to differentiate typedefs with the same base type
- added sanity test case
- generated documentation is wrong (api model/builder issue)
@ylil93
Copy link
Contributor

ylil93 commented Apr 5, 2018

The reason why the go docstring comment is wrong:

picture1

In the class constructor printer, we’re printing both the comment and the leaf at the same time by iterating through the class’s owned_elements, which is a list containing Property objects representing the leafs. The Property object’s property_type is used to create the comment.
The Property objects CompInsttype and CompNicinsttype are distinct, but both have property_type of CompInsttype_, meaning the issue occurs within the api builder, where the Property objects are created.

Notable lines in code:
class_constructor_printer.py: _print_leaf_inits(): 80
class_constructor_printer.py: _get_attribute_comment(): 137
class_constructor_printer.py: _get_meta_info_data(): 157
meta_data_util.py: get_meta_info_data(): 276
meta_data_util.py: get_class_crossref_tag(): 617-620

picture2

In the api builder, we fetch the type statement using the types extractor and pair the type statement with its corresponding Enum object (specifically by setting enum_type_stmt.parent.i_enum). This Enum object is then used to define the Property object and its property_type field. Notably, the types extractor returns the same enum_type_stmt for the statements representing CompInsttype and CompNicinsttype.

Notable lines in code:
_api_model_builder.py: _add_enums_and_bits(): 81
_api_model_builder.py: _add_enums_and_bits(): 90
_api_model_builder.py: _resolve_expanded_cross_references(): 120
_api_model_builder.py: _resolve_expanded_cross_references(): 132

Useful breakpoints:

if stmt.arg == 'comp_InstType': set_trace()
if stmt.parent.arg == 'comp_InstType': set_trace()
if stmt.parent.i_enum.name == 'CompInsttype_': set_trace()
if parent_element.name == 'sanity': set_trace()
if prop.property_type.name == 'CompInsttype_': set_trace()

ylil93 added a commit to ylil93/ydk-gen that referenced this issue Apr 5, 2018
- update generator to differentiate typedefs with the same base type
- added sanity test case
- generated documentation is wrong (api model/builder issue)
ghost pushed a commit that referenced this issue Apr 5, 2018
@ylil93
Copy link
Contributor

ylil93 commented Apr 5, 2018

I've committed fixes that prevent the code from failing for now, but the issue with the api builder causes the documentation to be incorrect in both go and cpp.

@ylil93 ylil93 self-assigned this Apr 5, 2018
@ylil93 ylil93 removed their assignment Apr 27, 2018
@ghost ghost added this to the 0.8.3 milestone Mar 5, 2019
@ygorelik
Copy link
Collaborator Author

The #891 was resolved in YDK-0.8.2. There are no more code duplication and XE bundle in Go is compiling fine. Hence closing the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants