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

Support for std::string_view #1443

Open
michal-kierzynka opened this issue Dec 1, 2023 · 10 comments
Open

Support for std::string_view #1443

michal-kierzynka opened this issue Dec 1, 2023 · 10 comments

Comments

@michal-kierzynka
Copy link

Hi,

I'm new to javacpp. I'm wondering if you could easily add support to std::string_view, in a similar way like you've added support for c10::string_view in #PR1392

Or maybe, is there an easier way to use std::string_view type in javacpp?

@saudet
Copy link
Member

saudet commented Dec 2, 2023

We can usually just map those to String and/or BytePointer, but that's going to depend on your code. In any case, contributions are welcome!

@michal-kierzynka
Copy link
Author

Thanks @saudet

So given the following toy case as below:

#include <string_view>


class StringClass {
 public:
    std::size_t calculateStringLength(std::string_view input) const {
        return input.size();
    }
};

Is the below code a correct way to map std::string_view to java String?

import org.bytedeco.javacpp.*;
import org.bytedeco.javacpp.annotation.*;
import org.bytedeco.javacpp.tools.*;


@Properties(
    value = @Platform(
        include = {"main.cpp"}
    ),
    target = "StringView"
)
public class StringViewConfig implements InfoMapper {
    public void map(InfoMap infoMap) {
        infoMap.put(new Info("std::string_view").pointerTypes("@StdString String"));
    }
}

@saudet
Copy link
Member

saudet commented Dec 4, 2023 via email

@HGuillemet
Copy link
Collaborator

If i remember well, this works for functions taking a string_view because the StringAdapter can be converted to a string_view, using char * as an intermediate implicit type, and recomputing the size using the null-termination.
But it won't work for functions returning a string_view, because we cannot instantiate a StringAdapter from a string_view.
For that we need either a specific adapter like was done for PyTorch, or we could add the missing constructor to StringAdapter.

@michal-kierzynka
Copy link
Author

Thanks for your answers. In my case, methods are taking std::string_view but returning std::string so no extensions needed.

@michal-kierzynka
Copy link
Author

I have, however, problem with std::vector<std::string_view> and I cannot figure out the type conversion. Example C++:

#include <iostream>
#include <string_view>
#include <vector>

class StringClass {
 public:
    void method(std::vector<std::string_view> const &input){
        for(auto &element : input) {
            std::cout << element << std::endl;
        }
    }
};

And corresponding Java file as in the post above (with infoMap.put(new Info("std::string_view").pointerTypes("@StdString String"));) produces the following error:

error: cannot convert ‘std::__cxx11::basic_string<char>’ to ‘const std::vector<std::basic_string_view<char> >&’
  937 |         ptr->method((std::basic_string< char >&)adapter0);
      |                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                     |
      |                     std::__cxx11::basic_string<char>

I have tried several other configurations but they all failed. Could you please point me in the correct direction?

@saudet
Copy link
Member

saudet commented Dec 6, 2023 via email

@saudet
Copy link
Member

saudet commented Dec 7, 2023

@michal-kierzynka
Copy link
Author

I have tried this before, unfortunately with the following error:

lib/java/jniLpTest.cpp: In function ‘_jstring* Java_LpAlignment_00024StringViewVector_00024Iterator_get(JNIEnv*, jobject)’:
lib/java/jniLpTest.cpp:2094:57: error: no matching function for call to ‘StringAdapter<char>::StringAdapter(std::basic_string_view<char>&)’
 2094 |         StringAdapter< char > radapter(ptr->operator *());
      |                                                         ^

@saudet
Copy link
Member

saudet commented Dec 10, 2023

Since your API probably doesn't need that get() method, you could patch that manually, just to get things working for now

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

3 participants