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

generated code not compiling due to missing cast to an enum #155

Closed
joelmartinez opened this issue Nov 2, 2023 · 8 comments · Fixed by #157
Closed

generated code not compiling due to missing cast to an enum #155

joelmartinez opened this issue Nov 2, 2023 · 8 comments · Fixed by #157

Comments

@joelmartinez
Copy link
Contributor

Hi ... I'm working on generating bindings with c2cs, and am getting the following error during the generate command:

2023-02-11 01:30:39 fail: C2CS.Features.WriteCodeCSharp.WriteCodeCSharpTool[0] - CSharpCompileDiagnostic - C# code compilation failure: /private/var/folders/vs/yh5mmtb55495fzn2fpbcmn5c0000gn/T/c2cs-Edr3GW/LVGL.gen.cs(11432,73): error CS0266: Cannot implicitly convert type 'int' to 'LVGLSharp.LVGL._lv_buttonmatrix_ctrl_t'. An explicit conversion exists (are you missing a cast?) [/private/var/folders/vs/yh5mmtb55495fzn2fpbcmn5c0000gn/T/c2cs-Edr3GW/Project.csproj].

That points to this line:

[CNode(Kind = "MacroObject")]
    public static _lv_buttonmatrix_ctrl_t LV_BTNMATRIX_CTRL_CHECKABLE = 128;

So the 128 is just missing an explicit cast to _lv_buttonmatrix_ctrl_t

  [CNode(Kind = "Enum")]
    public enum _lv_buttonmatrix_ctrl_t : int
    {

And if it helps, here's the corresponding enum definition in the ast file:

  "enums": {
    "_lv_buttonmatrix_ctrl_t": {
      "typeInteger": {
        "name": "unsigned int",
        "kind": "primitive",
        "sizeOf": 4,
        "alignOf": 4
      },
      "values": [
        {
          "name": "_LV_BUTTONMATRIX_WIDTH",
          "value": 15
        },
        {
          "name": "LV_BUTTONMATRIX_CTRL_HIDDEN",
          "value": 16
        },
        {
          "name": "LV_BUTTONMATRIX_CTRL_NO_REPEAT",
          "value": 32
        },
        {
          "name": "LV_BUTTONMATRIX_CTRL_DISABLED",
          "value": 64
        },
        {
          "name": "LV_BUTTONMATRIX_CTRL_CHECKABLE",
          "value": 128
        },
        {
          "name": "LV_BUTTONMATRIX_CTRL_CHECKED",
          "value": 256
        },
        {
          "name": "LV_BUTTONMATRIX_CTRL_CLICK_TRIG",
          "value": 512
        },
        {
          "name": "LV_BUTTONMATRIX_CTRL_POPOVER",
          "value": 1024
        },
        {
          "name": "LV_BUTTONMATRIX_CTRL_RECOLOR",
          "value": 2048
        },
        {
          "name": "_LV_BUTTONMATRIX_CTRL_RESERVED_1",
          "value": 4096
        },
        {
          "name": "_LV_BUTTONMATRIX_CTRL_RESERVED_2",
          "value": 8192
        },
        {
          "name": "LV_BUTTONMATRIX_CTRL_CUSTOM_1",
          "value": 16384
        },
        {
          "name": "LV_BUTTONMATRIX_CTRL_CUSTOM_2",
          "value": 32768
        }
      ],
      "location": {
        "fileName": "lv_buttonmatrix.h",
        "filePath": "/widgets/buttonmatrix/lv_buttonmatrix.h",
        "line": 34,
        "column": 6
      },
      "comment": "/** Type to store button control bits (disabled, hidden etc.)\n * The first 3 bits are used to store the width*/"
    },

Any hints or ideas as to what might be going on and how to mitigate would be greatly appreciated!

@lithiumtoast
Copy link
Member

Hey @joelmartinez,

This looking similar to #150.

The code that would need to change is here: https://github.com/bottlenoselabs/c2cs/blob/main/src/cs/production/C2CS.Tool/Features/WriteCodeCSharp/Domain/CodeGenerator/Handlers/MacroCodeGenerator.cs#L41.

@joelmartinez
Copy link
Contributor Author

@lithiumtoast ahh yes indeed, that does look like a) the same issue, and b) probably where that needs to change. Would this be a simple enough change?

            code = $@"
{attributesString}
public static {node.Type} {name} = ({node.Type}){node.Value};
";

If so ... do you want me to open a PR with this? Do you think it's easy enough to onboard by cloning the repo and making the change? would you recommend adding some test for this somewhere?

joelmartinez added a commit to joelmartinez/c2cs that referenced this issue Nov 5, 2023
@joelmartinez
Copy link
Contributor Author

@lithiumtoast ok, so I created a pull request with this change. I ran the unit tests and they all passed. There were a few updates I didn't include, for example some changes to the generated metadata of the generated files, e.g.

-//  This code was generated by the following tool on 2023-07-23 05:51:56 GMT-04:00:
-//      https://github.com/bottlenoselabs/c2cs (v2.11.1.99)
+//  This code was generated by the following tool on 2023-11-05 09:24:44 GMT-05:00:
+//      https://github.com/bottlenoselabs/c2cs (v15.0.0.0)

Let me know if you want me to include those in the PR as well

@lithiumtoast
Copy link
Member

Hey @joelmartinez,

I see you figured it out with your PR #157.

Would you recommend adding some test for this somewhere?

When I split CAstFfi from C2CS, looks like I didn't build out a lot of tests for the C# side. I have the basics going for enums: https://github.com/bottlenoselabs/c2cs/blob/main/src/cs/tests/C2CS.Tests/TestCSharpCode.cs#L14.

If you are feeling up to it, you could add a test for a macro object using a theory attribute for macro objects similar to enums.
The JSON test files are automatically re-generated by flipping this boolean to true in the constructor: https://github.com/bottlenoselabs/c2cs/blob/main/src/cs/tests/C2CS.Tests/TestCSharpCode.cs#L44. Looks like I forgot to turn it off and committed; it should be false when merging into main to use existing test files so testing is actually comparing against existing values.

lithiumtoast pushed a commit that referenced this issue Nov 13, 2023
@lithiumtoast
Copy link
Member

lithiumtoast commented Nov 15, 2023

I merged it in thanks, @joelmartinez

@lithiumtoast
Copy link
Member

@joelmartinez It's up on NuGet as version 6.1.4 : https://www.nuget.org/packages/bottlenoselabs.C2CS.Tool

@joelmartinez
Copy link
Contributor Author

Amazing, thank you @lithiumtoast! Sorry, I meant to post/respond that I had started to take a look at adding the unit test, but unfortunately I was quite busy and didn't get a chance to really dig in. Appreciate your help in this 🙏🏼

@lithiumtoast
Copy link
Member

@joelmartinez I did it as part of #158. There was a couple things that needed fixing to make adding more unit tests easier anyways.

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

Successfully merging a pull request may close this issue.

2 participants