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

Windows: basic compatibilty issues #23

Open
guestieng opened this issue Jun 13, 2019 · 5 comments
Open

Windows: basic compatibilty issues #23

guestieng opened this issue Jun 13, 2019 · 5 comments

Comments

@guestieng
Copy link

In case there is some interest regarding the usability of CppADCodeGen within Windows (e.g. VS 2015) I'd like to share a few basic compatibilty issues.
In fact, these are rather minor but have quite an impact since they appear in several places throughout all the source code.
Therefore, in the following I list representative examples and suggestions for their principle replacements which should work for both, Windows and Linux (g++):

  1. suggested change of, e.g., std::max(size_t(123), 124) to std::max<size_t>(size_t(123), 124)
    and std::min(size_t(123), 124) to std::min<size_t>(size_t(123), 124), i.e. std::max/std::min should carry a template argument specifying the data type which is relevant for the two passed numbers.

  2. suggested change of, e.g., std::numeric_limits<int>::max() to (std::numeric_limits<int>::max)()
    and std::numeric_limits<int>::min() to (std::numeric_limits<int>::min)()

  3. there exist some problems with the operator <<, it's overloading and the type size_t/__int64 in Windows. This, at least, applies only to recent changes (commit from Jun 13, 2019) and, so far, can be easily fixed in /lang/lang_stream_stack.hpp by adding:

friend inline LangStreamStack<Base>& operator<<(LangStreamStack<Base>& lss, size_t i) {
	return (lss << std::to_string(i));
}
@joaoleal
Copy link
Owner

CppadCodegen does not explicitly support Windows mainly because I don't really use it.
Nevertheless, it would be great to have it working.

Regarding your suggestions:

  1. Simple change
  2. Simple change
  3. The third cannot be implemented just by adding size_t since in Linux (64bit) it is a long unsigned int.
    I'll add new overloading for long long int.

This, however, will not be enough to fully support windows.
For instance, the functions in system.hpp will have to be implemented for windows just as they are for Linux/Mac (linux_system.hpp).

@guestieng
Copy link
Author

Yes, that's true, there are a few more changes to make in order to have it working in windows. But particularly with the first two basic issues (see above) respected it's at least a little less cumbersome.
I've got one solution which, I think, is sufficiently stable with VS 2015 and Mingw-w64 (gcc) , and which I can prepare to share once I have a gap. However, so far one little drawback is that this needs a little helper library like dlfcn-win, if one wants to keep it easy and avoid touching all the existing code which handles the created dynamic libraries.

@joaoleal
Copy link
Owner

I've implemented the 3 items.
Can you please check if you can compile it with VS 2015?

What kind of features would you need in the dynamic library handling classes?
If we implement the system.hpp for windows you might get it.

@guestieng
Copy link
Author

Alright, thanks.
These 3 changes are working.
However, to make it compile completely more changes are needed.
As said I can prepare these for sharing once I have another gap.
But it's too cumbersome to post all this here.
What would suit you instead, perhaps a pull request?

@joaoleal
Copy link
Owner

Yes, a pull request would be great.
Travis also supports windows now, we can add it to the platforms being tested.

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

No branches or pull requests

2 participants