Export symbols in Windows DLL #213
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| /// passed as a parameter, and if the parent is not passed, | ||
| /// then global scope will be assumed. | ||
| TCppScope_t GetScope(const std::string &name, TCppScope_t parent = 0); | ||
| DllExport TCppScope_t GetScope(const std::string &name, TCppScope_t parent = 0); |
Contributor
There was a problem hiding this comment.
warning: use nullptr [modernize-use-nullptr]
Suggested change
| DllExport TCppScope_t GetScope(const std::string &name, TCppScope_t parent = 0); | |
| DllExport TCppScope_t GetScope(const std::string &name, TCppScope_t parent = nullptr); |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #213 +/- ##
=======================================
Coverage 78.74% 78.74%
=======================================
Files 8 8
Lines 3091 3091
=======================================
Hits 2434 2434
Misses 657 657
|
vgvassilev
reviewed
Mar 20, 2024
Comment on lines
19
to
23
| #ifdef _WIN32 | ||
| #define DllExport __declspec(dllexport) | ||
| #else | ||
| #define DllExport __attribute__((visibility("default"))) | ||
| #endif |
Contributor
There was a problem hiding this comment.
Suggested change
| #ifdef _WIN32 | |
| #define DllExport __declspec(dllexport) | |
| #else | |
| #define DllExport __attribute__((visibility("default"))) | |
| #endif | |
| #ifdef _WIN32 | |
| #define CPPINTEROP_API __declspec(dllexport) | |
| #else | |
| #define CPPINTEROP_API __attribute__((visibility("default"))) | |
| #endif |
It's probably better name.
vgvassilev
reviewed
Mar 20, 2024
| /// can be used to iterate through the index value to get each specific | ||
| /// base class. | ||
| TCppScope_t GetBaseClass(TCppType_t klass, TCppIndex_t ibase); | ||
| DllExport DllExport TCppScope_t GetBaseClass(TCppType_t klass, TCppIndex_t ibase); |
Contributor
There was a problem hiding this comment.
Suggested change
| DllExport DllExport TCppScope_t GetBaseClass(TCppType_t klass, TCppIndex_t ibase); | |
| DllExport TCppScope_t GetBaseClass(TCppType_t klass, TCppIndex_t ibase); |
vgvassilev
reviewed
Mar 20, 2024
|
|
||
| /// Ends recording the standard stream and returns the result as a string. | ||
| std::string EndStdStreamCapture(); | ||
| DllExport std::string EndStdStreamCapture(); |
Contributor
There was a problem hiding this comment.
Suggested change
| DllExport std::string EndStdStreamCapture(); | |
| DllExport std::string EndStdStreamCapture(); | |
| #undef CPPINTEROP_API |
vgvassilev
reviewed
Mar 20, 2024
lib/Interpreter/CppInterOp.cpp
Outdated
| // End of JitCall Helper Functions | ||
|
|
||
| JitCall MakeFunctionCallable(TCppConstFunction_t func) { | ||
| DllExport JitCall MakeFunctionCallable(TCppConstFunction_t func) { |
Contributor
There was a problem hiding this comment.
We do not need to export things outside of the public API that's defined in the header file.
5 tasks
| /// passed as a parameter, and if the parent is not passed, | ||
| /// then global scope will be assumed. | ||
| TCppScope_t GetScope(const std::string &name, TCppScope_t parent = 0); | ||
| CPPINTEROP_API TCppScope_t GetScope(const std::string &name, TCppScope_t parent = 0); |
Contributor
There was a problem hiding this comment.
warning: use nullptr [modernize-use-nullptr]
Suggested change
| CPPINTEROP_API TCppScope_t GetScope(const std::string &name, TCppScope_t parent = 0); | |
| CPPINTEROP_API TCppScope_t GetScope(const std::string &name, TCppScope_t parent = nullptr); |
lib/Interpreter/CppInterOp.cpp
Outdated
| // End of JitCall Helper Functions | ||
|
|
||
| JitCall MakeFunctionCallable(TCppConstFunction_t func) { | ||
| DllExport JitCall MakeFunctionCallable(TCppConstFunction_t func) { |
Contributor
There was a problem hiding this comment.
warning: variable 'JitCall' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
DllExport JitCall MakeFunctionCallable(TCppConstFunction_t func) {
^
vgvassilev
reviewed
Mar 20, 2024
vgvassilev
force-pushed
the
FixWindowsExportSymbols
branch
from
9ddad94 to
310d7c3
Compare
| /// then global scope will be assumed. | ||
| TCppScope_t GetScope(const std::string &name, TCppScope_t parent = 0); | ||
| CPPINTEROP_API TCppScope_t GetScope(const std::string& name, | ||
| TCppScope_t parent = 0); |
Contributor
There was a problem hiding this comment.
warning: use nullptr [modernize-use-nullptr]
Suggested change
| TCppScope_t parent = 0); | |
| TCppScope_t parent = nullptr); |
| bool ExistsFunctionTemplate(const std::string& name, | ||
| TCppScope_t parent = 0); | ||
| CPPINTEROP_API bool ExistsFunctionTemplate(const std::string& name, | ||
| TCppScope_t parent = 0); |
Contributor
There was a problem hiding this comment.
warning: use nullptr [modernize-use-nullptr]
Suggested change
| TCppScope_t parent = 0); | |
| TCppScope_t parent = nullptr); |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
No description provided.