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

ZCBOR code generator generates names not compatible with C++ #54092

Closed
kborowski3 opened this issue Jan 25, 2023 · 13 comments
Closed

ZCBOR code generator generates names not compatible with C++ #54092

kborowski3 opened this issue Jan 25, 2023 · 13 comments
Assignees
Labels
area: C++ bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug

Comments

@kborowski3
Copy link

Describe the bug
There is a problem with compiling ZBOR code generated from .cddl files when we use it in the application written in C++.
To be precise the problem occurs if we want to generate Union type. The generated code can be compiled in C but it is not working for C++ due to generated variables names conflicts.

To Reproduce
Simple example showing the problem was created and it is available on my repository -> https://github.com/kborowski3/zcbor_union_test

This is a simple application, which just includes the generated zcbor code. ZCBOR code is generated using zcbor_union_test.cddl which contains a simple union definition.

zcbor_union_test.cddl file content

Foo = int / bstr

where Foo is either an int or bstr.

The code is generated using ZCBOR code generator:

python3 ${ZCBOR_PATH}/zcbor/zcbor.py code -c zcbor_union_test.cddl --encode -t Foo --oc src/zcbor_union_test.c --oh src/zcbor_union_test.h

The problem is in the zcbor_union_test.h file. The content of the file was pasted below.

/*
 * Generated using zcbor version 0.6.99
 * https://github.com/NordicSemiconductor/zcbor
 * Generated with a --default-max-qty of 3
 */

#ifndef ZCBOR_UNION_TEST_TYPES_H__
#define ZCBOR_UNION_TEST_TYPES_H__

#include <stdint.h>
#include <stdbool.h>
#include <stddef.h>
#include <string.h>
#include "zcbor_encode.h"

/** Which value for --default-max-qty this file was created with.
 *
 *  The define is used in the other generated file to do a build-time
 *  compatibility check.
 *
 *  See `zcbor --help` for more information about --default-max-qty
 */
#define DEFAULT_MAX_QTY 3

struct Foo_ {
	union {
		int32_t _Foo_int;
		struct zcbor_string _Foo_bstr;
	};
	enum {
		_Foo_int,
		_Foo_bstr,
	} _Foo_choice;
};


#endif /* ZCBOR_UNION_TEST_TYPES_H__ */

As you can see there is a problem with the enum's and union's fields names. Both union and enum has a member _Foo_int and _Foo_bstr, which cannot be compiled in C++ code.

Steps to reproduce the behavior:

  1. git clone https://github.com/kborowski3/zcbor_union_test.git
  2. cd zcbor_union_test
  3. west build -p always -b nucleo_wb55rg ./
  4. See error

Expected behavior
The application compiles successfully.

Logs and console output

/zcbor_union_test/src/zcbor_union_test_types.h:31:17: error: '_Foo_int' conflicts with a previous declaration
   31 |                 _Foo_int,
      |                 ^~~~~~~~
/zcbor_union_test/src/zcbor_union_test_types.h:27:25: note: previous declaration 'int32_t Foo_::<unnamed union>::_Foo_int'
   27 |                 int32_t _Foo_int;
      |                         ^~~~~~~~
/zcbor_union_test/src/zcbor_union_test_types.h:32:17: error: '_Foo_bstr' conflicts with a previous declaration
   32 |                 _Foo_bstr,
      |                 ^~~~~~~~~
/zcbor_union_test/src/zcbor_union_test_types.h:28:37: note: previous declaration 'zcbor_string Foo_::<unnamed union>::_Foo_bstr'
   28 |                 struct zcbor_string _Foo_bstr;
      |       

Environment

  • Zephyr SDK version 0.15.1
  • ZCBOR version 0.6.99
@kborowski3 kborowski3 added the bug The issue is a bug, or the PR is fixing a bug label Jan 25, 2023
@nordicjm
Copy link
Collaborator

This should really have been posted to https://github.com/NordicSemiconductor/zcbor

@kborowski3
Copy link
Author

kborowski3 commented Jan 25, 2023

Actually I can see that there are some C++ improvements commits but they are not available on the zephyrproject-rtos/zcbor forked repository. Can you bump there the newest version?

@laurenmurphyx64 laurenmurphyx64 added priority: low Low impact/importance bug area: C++ labels Jan 26, 2023
@github-actions
Copy link

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

@github-actions
Copy link

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

@github-actions github-actions bot added the Stale label Jun 16, 2023
@jgl-meta
Copy link
Collaborator

@oyvindronningstad this has been marked as stale 3 times. Is this an issue? If it isn't can you close this?

@jgl-meta jgl-meta removed the Stale label Jun 20, 2023
@oyvindronningstad
Copy link
Collaborator

It is an issue. It is tracked by NordicSemiconductor/zcbor#314.

@github-actions
Copy link

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

@github-actions github-actions bot added the Stale label Aug 22, 2023
@r2r0
Copy link
Member

r2r0 commented Aug 22, 2023

@oyvindronningstad Any chance to upstream zcbor fix to zephyr?

@github-actions github-actions bot removed the Stale label Aug 23, 2023
@github-actions
Copy link

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

@github-actions github-actions bot added the Stale label Oct 22, 2023
@r2r0
Copy link
Member

r2r0 commented Oct 23, 2023

@oyvindronningstad Ping

Copy link

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

@github-actions github-actions bot added the Stale label Dec 23, 2023
@r2r0
Copy link
Member

r2r0 commented Jan 3, 2024

@oyvindronningstad If upstreaming is not possibel or planned then please close this issue.

@github-actions github-actions bot removed the Stale label Jan 4, 2024
@oyvindronningstad
Copy link
Collaborator

@r2r0 sorry for the delay. This is coming with zcbor 0.8.0 in #67418

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: C++ bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug
Projects
None yet
Development

No branches or pull requests

7 participants