Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
WIP: Interface allowing ELM to drive ATS #157
WIP: Interface allowing ELM to drive ATS #157
Changes from 8 commits
20dae3c
eada89b
312aa7b
bba9dee
2704dd9
704c25c
5e59a11
a68f7b7
b68126c
fdffc7b
c94ba70
b5b3325
9ffa11e
f186146
fe63ea8
f2a69e5
a48e52e
bb24059
5f87eff
f105cbc
7f5683f
90c62dc
e575f10
f6252d3
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it have to be static?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean, I assume so, or you wouldn't have added this, just yuck.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this void*?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, namespaces are such a good idea. Too bad C didn't think of them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kind of. I used an opaque pointer method that's pretty common when creating portable C++ libraries. It's defined in the accompanying header file:
ats/src/executables/elm_ats_api/elm_ats_api.h
Lines 5 to 16 in a68f7b7
To the C compiler and whatever linker the external program invokes, this is a typedef to a pointer to a forward declared dummy struct. It's like a void* with a bit of type safety. To the C++ compiler/linker, it's resolved as a ELM_ATSDriver*.
I made the decision to hide the pointer behind the typedef because calling code should never dereference the ATS object. I think some people hate this, but it seems like the right call here.