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

Fix parsing of functions parameters in template arguments #693

Merged
merged 3 commits into from Jul 8, 2023

Conversation

HGuillemet
Copy link
Contributor

This PR proposes to call parameters from templateArguments when a function is found in order to correctly pass its prototype.

This solves two issues: the parsing of argument identifiers when present, and the resolution of namespaces.

Example:

namespace ns {
  struct S {
    int x;
  };

  std::function<void(int a)> a;
  std::function<S(S)> b;
}

Before, parsed as:

std::function<void(inta)>
std::function<ns::S(S)>

with the PR:

std::function<void(int)>
std::function<ns::S(ns::S)>

Some presets need to be adjusted to parse correctly with this PR (fixing of std::function parameters):

  • 4 mappings in current pytorch
  • 2 in opencv
  • 1 in onnx

No changes in other presets.
Not tested: libfreenect2, chilitags,hyperscan, tensorflow, ale, ngraph, qt, skia, videoinput, tritonserver, depthai

@HGuillemet
Copy link
Contributor Author

That logic should probably be in type() instead. But then the returned Type should be able to represent a function type with its arguments (or a function pointer), which requires modifying the Type class.

@saudet
Copy link
Member

saudet commented Jul 5, 2023 via email

@HGuillemet
Copy link
Contributor Author

Do you want a PR on javacpp-presets for the corresponding fixes ?

@saudet
Copy link
Member

saudet commented Jul 8, 2023

No need to, they'll get fixed when they get updated.

@saudet saudet merged commit 7abed1c into bytedeco:master Jul 8, 2023
11 checks passed
@HGuillemet HGuillemet deleted the fix_template_parsing_functions branch July 8, 2023 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants