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 emit error #306

Closed
wants to merge 1 commit into from
Closed

Go emit error #306

wants to merge 1 commit into from

Conversation

hoxell
Copy link
Contributor

@hoxell hoxell commented Jan 10, 2020

Resolving issue #294 and re-enabling go in CMakeLists.txt

Cause:
lcm_find_struct() returns null for primitive types.

Solution:
Only invoke if non-primitive type.

* Add check if:
  - fingerprint suffix is requested, and
  - member is primitive type

If not, don't bother calling lcm_find_struct() when creating array
@hoxell
Copy link
Contributor Author

hoxell commented Jan 10, 2020

Go tests failing due to <lcm/lcm.h> not found

@gustafj
Copy link
Member

gustafj commented Feb 11, 2020

I will not merge this as I'd rather revert the broken commit/PR #288 (that should not have been merged, my bad) that introduced the issue, see #310.
And as the tests are failing.

And why do you ignore the fingerprint for primitive types? Seems wrong to me.
I will gladly merge a new PR with other updates that are part of this PR.

Thanks for the work!

@hoxell
Copy link
Contributor Author

hoxell commented Feb 11, 2020

I'm on my phone atm, but if I've managed to get a correct overview while browsing the code, the fingerprint calculated in emit_go_array_loops() is only used for appending to the type name.

As per the comment above go_typename() (i.e. "Fingerprint suffix is added if fingerprint is != 0"), I think the rationale was to avoid adding the suffix to primitive types. However, that is already achieved in go_typename(), so the check for a primitive type is redundant and confusing.
Like in emit_go_lcm_struct_definition(), it effectively sets the fingerprint of primitive types to 0 only for the call to go_typename(), but does not set the actual fingerprint of the type to 0, which would be wrong.

Anyway, I'd go with a different, more robust solution if I were to implement it again, so I'll close this PR.

Thanks :)

@hoxell hoxell closed this Feb 11, 2020
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 this pull request may close these issues.

2 participants