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

ML-421 Remove Typedef from MakeR8Set in PBblas #12

Merged
merged 1 commit into from May 22, 2018

Conversation

RogerDev
Copy link

Remove typedef and re-test

Signed-off-by: Roger Dev Roger.Dev@LexisNexisRisk.com

@richardkchapman
Copy link
Member

@johnholt Please review

} pbb_cell;
#endif // pbb_cell #ifndef pbb_cell

struct __attribute__ ((__packed__)) pbb_cell{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not an expert at C++ but I am under the impression that the struct declaration should be wrapped with an if not defined (#ifndef FOO and #define FOO). If this structure is used (and it could be in my opinion) by another BEGINC++ attribute, then you could get a multiple defined name. @richardkchapman should correct me if multiple definitions of the same struct are OK.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have thought simply removing the #body on line 35 (meaning that the struct definition would be local to the generated function) would be the simplest and cleanest way to avoid that potential issue.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds OK to me, but I don't have exact knowledge of the visibility rules for the modern version of the C++ language.

Remove typedef and re-test
Remove #body tag per code review

Signed-off-by: Roger Dev <Roger.Dev@LexisNexisRisk.com>
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.

None yet

4 participants