You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@milagro.apache.org by GitBox <gi...@apache.org> on 2020/09/08 13:56:27 UTC
[GitHub] [incubator-milagro-crypto-c] gimer opened a new issue #85: Enable MSVC compilation
gimer opened a new issue #85:
URL: https://github.com/apache/incubator-milagro-crypto-c/issues/85
Hello,
We've prepared bunch of changes that enable milagro compilation in MS Visual Studio.
The changes can be divided into 3 groups:
1. trivial name<->type clash
2. variable array stack allocation (this is C99, which VS does not fully support, solutions below)
3. static and dynamic symbol export (most intrusive part)
Re 2. In case of C code adding `const` to array length vars doesn't change anything. There are 2 various solutions applied:
* where possible, variable holding length is turned into define (i.e. `int n -> #define NUM_SHARES`)
* where it's not possible [_alloca](https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/alloca?view=vs-2019) is used (i.e. where length is passed via function arg)
Re 3. Static libs should compile with modifications from 1 and 2, dynamic libs, require few more changes.
Cmake's [CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS](https://cmake.org/cmake/help/v3.18/prop_tgt/WINDOWS_EXPORT_ALL_SYMBOLS.html) is used to export most of the symbols, this however does not work for **data** exported via `amcl_curve_*` libraries
To fix that:
* definitions are prefixed with `SYMBOL_EXPORT` defined as emtpy for non-msvc in `amcl.h.in`
* declarations are prefixed with `EXTERNAL_SYMBOL_DECLARATION`,
* in case of non-vs, this is == `extern`
* in vs case this is either `declspec(dllexport)` in case of `amcl_curve_*` libraries and `declspec(dllimport)` for all other libraries including those header files.
Finally my question is following: would you prefer this change split into some smaller chunks, or is ok to issue it as a single pull request?
FYI: whole change is here: https://github.com/apache/incubator-milagro-crypto-c/compare/develop...nemtech:msvc-compilation
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@milagro.apache.org
For additional commands, e-mail: issues-help@milagro.apache.org
[GitHub] [incubator-milagro-crypto-c] gimer commented on issue #85: Enable MSVC compilation
Posted by GitBox <gi...@apache.org>.
gimer commented on issue #85:
URL: https://github.com/apache/incubator-milagro-crypto-c/issues/85#issuecomment-689097354
I closed pull request, there's still something wrong, that I'm trying to figure out, will get back with results
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@milagro.apache.org
For additional commands, e-mail: issues-help@milagro.apache.org
[GitHub] [incubator-milagro-crypto-c] gimer edited a comment on issue #85: Enable MSVC compilation
Posted by GitBox <gi...@apache.org>.
gimer edited a comment on issue #85:
URL: https://github.com/apache/incubator-milagro-crypto-c/issues/85#issuecomment-688924815
a) correct me if I'm wrong, but there are no building instructions for ms vs
b)
> VS2019 supports C99 and i suppose C11
I wish that was the case, msdn:
> MSVC is compatible with the ISO C99 standard, but not strictly compliant. In most cases, portable C code will compile and run as expected. Visual C++ doesn't support most of the changes in ISO C11.
[VLA](https://en.wikipedia.org/wiki/Variable-length_array) support in VS in case of C is lacking,
godbolt as a proof: https://godbolt.org/z/oz49Ys
**edit** P.S. you might notice "proof" function signature is based on `recover_coefficients` from src/bls.c.in
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@milagro.apache.org
For additional commands, e-mail: issues-help@milagro.apache.org
[GitHub] [incubator-milagro-crypto-c] gimer commented on issue #85: Enable MSVC compilation
Posted by GitBox <gi...@apache.org>.
gimer commented on issue #85:
URL: https://github.com/apache/incubator-milagro-crypto-c/issues/85#issuecomment-688924815
a) correct me if I'm wrong, but there are no building instructions for ms vs
b)
> VS2019 supports C99 and i suppose C11
I wish that was the case, msdn:
> MSVC is compatible with the ISO C99 standard, but not strictly compliant. In most cases, portable C code will compile and run as expected. Visual C++ doesn't support most of the changes in ISO C11.
[VLA](https://en.wikipedia.org/wiki/Variable-length_array) support in VS in case of C is lacking,
godbolt as a proof: https://godbolt.org/z/oz49Ys
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@milagro.apache.org
For additional commands, e-mail: issues-help@milagro.apache.org
[GitHub] [incubator-milagro-crypto-c] giorgiozoppi removed a comment on issue #85: Enable MSVC compilation
Posted by GitBox <gi...@apache.org>.
giorgiozoppi removed a comment on issue #85:
URL: https://github.com/apache/incubator-milagro-crypto-c/issues/85#issuecomment-688913239
VS2019 supports C99 and i suppose C11
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@milagro.apache.org
For additional commands, e-mail: issues-help@milagro.apache.org
[GitHub] [incubator-milagro-crypto-c] gimer commented on issue #85: Enable MSVC compilation
Posted by GitBox <gi...@apache.org>.
gimer commented on issue #85:
URL: https://github.com/apache/incubator-milagro-crypto-c/issues/85#issuecomment-689094520
Eh, there's still a bug in the patch.
I've assumed every curve has it's own `rom_fields` file (that is _usually_ the case except for `NUMS512E` vs `NUMS512W` which both reuse `rom_field_512PM.c` - I'm not sure if this is a bug or intended behavior
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@milagro.apache.org
For additional commands, e-mail: issues-help@milagro.apache.org
[GitHub] [incubator-milagro-crypto-c] kealan commented on issue #85: Enable MSVC compilation
Posted by GitBox <gi...@apache.org>.
kealan commented on issue #85:
URL: https://github.com/apache/incubator-milagro-crypto-c/issues/85#issuecomment-688982540
It looks good to me. A single pull request would be ok.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@milagro.apache.org
For additional commands, e-mail: issues-help@milagro.apache.org
[GitHub] [incubator-milagro-crypto-c] giorgiozoppi commented on issue #85: Enable MSVC compilation
Posted by GitBox <gi...@apache.org>.
giorgiozoppi commented on issue #85:
URL: https://github.com/apache/incubator-milagro-crypto-c/issues/85#issuecomment-688938557
Ok i trust Matt work. He is really good person and professional :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@milagro.apache.org
For additional commands, e-mail: issues-help@milagro.apache.org
[GitHub] [incubator-milagro-crypto-c] gimer edited a comment on issue #85: Enable MSVC compilation
Posted by GitBox <gi...@apache.org>.
gimer edited a comment on issue #85:
URL: https://github.com/apache/incubator-milagro-crypto-c/issues/85#issuecomment-688924815
a) correct me if I'm wrong, but there are no building instructions for ms vs
b) answering @giorgiozoppi
> VS2019 supports C99 and i suppose C11
I wish that was the case, msdn:
> MSVC is compatible with the ISO C99 standard, but not strictly compliant. In most cases, portable C code will compile and run as expected. Visual C++ doesn't support most of the changes in ISO C11.
[VLA](https://en.wikipedia.org/wiki/Variable-length_array) support in VS in case of C is lacking,
godbolt as a proof: https://godbolt.org/z/oz49Ys
**edit** P.S. you might notice "proof" function signature is based on `recover_coefficients` from src/bls.c.in
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@milagro.apache.org
For additional commands, e-mail: issues-help@milagro.apache.org
[GitHub] [incubator-milagro-crypto-c] gimer removed a comment on issue #85: Enable MSVC compilation
Posted by GitBox <gi...@apache.org>.
gimer removed a comment on issue #85:
URL: https://github.com/apache/incubator-milagro-crypto-c/issues/85#issuecomment-689094520
Eh, there's still a bug in the patch.
I've assumed every curve has it's own `rom_fields` file (that is _usually_ the case except for `NUMS512E` vs `NUMS512W` which both reuse `rom_field_512PM.c` - I'm not sure if this is a bug or intended behavior
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@milagro.apache.org
For additional commands, e-mail: issues-help@milagro.apache.org
[GitHub] [incubator-milagro-crypto-c] giorgiozoppi commented on issue #85: Enable MSVC compilation
Posted by GitBox <gi...@apache.org>.
giorgiozoppi commented on issue #85:
URL: https://github.com/apache/incubator-milagro-crypto-c/issues/85#issuecomment-692265281
@gimer we had an interesting chat at C++Con with msvc team members, they are stating that in th next version they will support C11 features but not optional features like valarrays. They have changed the preprocessor for C++20 and they take those opportunity to support C11 and C17.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@milagro.apache.org
For additional commands, e-mail: issues-help@milagro.apache.org
[GitHub] [incubator-milagro-crypto-c] giorgiozoppi commented on issue #85: Enable MSVC compilation
Posted by GitBox <gi...@apache.org>.
giorgiozoppi commented on issue #85:
URL: https://github.com/apache/incubator-milagro-crypto-c/issues/85#issuecomment-688913239
VS2019 supports C99 and i suppose C11
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@milagro.apache.org
For additional commands, e-mail: issues-help@milagro.apache.org
[GitHub] [incubator-milagro-crypto-c] gimer edited a comment on issue #85: Enable MSVC compilation
Posted by GitBox <gi...@apache.org>.
gimer edited a comment on issue #85:
URL: https://github.com/apache/incubator-milagro-crypto-c/issues/85#issuecomment-689097354
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@milagro.apache.org
For additional commands, e-mail: issues-help@milagro.apache.org