Export symbols for Windows DLL and MSVC build compatibility

[GericoVi]

Export DLL symbols for Windows and MSVC build compatibility. Related to #290

[rusty1s]

I missed this PR, I am really sorry. I am happy to merge it once tests pass. Let me know if you need any help in fixing those.

[GericoVi]

Don't worry, it's not ready yet anyway, I hadn't tested the pip build process. The new changes seem to break the setup.py installation - some details here - but it builds fine using MSVC directly.

I'm not entirely sure what the root cause is (and why the Linux build is also not working now) since the additions were meant to fix the unresolved symbols issue when building on Windows - #290. I'm guessing something in the compiler or linker arguments in the setup.py but I'm not too familiar.

[rusty1s]

Yes, this is a bit weird since it looks identical to rusty1s/pytorch_sparse#198. Hopefully @dfalbel has some idea?

[GericoVi]

I've since revisited this and the linking failure seems to only occur with the cuda_version symbol. The others seem to export properly.

I.e. commenting out these lines:

namespace scatter {
SCATTER_API int64_t cuda_version() noexcept;

namespace detail {
SCATTER_INLINE_VARIABLE int64_t _cuda_version = cuda_version();
} // namespace detail
} // namespace scatter

in scatter.h allows setup.py to run to completion and install. But I'm assuming the package won't work correctly in Python without this functionality.

Trying to investigate why this symbol doesn't export correctly. The setup seems to be the same as in the pytorch_sparse symbol export and also in the new rusty1s/pytorch_cluster#132.

[GericoVi]

I have a feeling that it has something to do with the fact that the main header scatter.h is also associated with a code file (scatter.cpp). While in torch_sparse and torch_cluster, their respective headers (sparse.h and cluster.h) only serve to declare functions (including the cuda_version) and don't have a corresponding *.cpp file - they're not even really included anywhere in the c source.

This is really the only thing I can see that's different between the implementations in those repos which worked and here where it isn't. Note, the other files build and link correctly (especially notably version.cpp has no problem linking even though it contains the cuda_version symbol).

The above commit brings the changes even more inline with the export symbols PRs in the other repos. But unsure how to fully fix - needs separation of scatter.cpp into individual files maybe? Maybe a bit out of scope for this PR, @rusty1s?

[codecov-commenter]

Codecov Report

Merging #294 (b1815bd) into master (5ec31fc) will not change coverage.
The diff coverage is n/a.

❗ Current head b1815bd differs from pull request most recent head 093ddcb. Consider uploading reports for the commit 093ddcb to get more accurate results

@@           Coverage Diff           @@
##           master     #294   +/-   ##
=======================================
  Coverage   97.54%   97.54%           
=======================================
  Files           9        9           
  Lines         204      204           
=======================================
  Hits          199      199           
  Misses          5        5           

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

[GericoVi]

So it works now? By removing the #include "scatter.h" from scatter.cpp.

I don't have enough C++/Python extension knowledge to know why that works or if it's a bad solution. So I'll leave it up to others...

[rusty1s]

Thank you! The "fix" makes sense.