Fix namespaces and template arguments parsing.

[HGuillemet]

At various places, the parser uses simple character searchs like indexOf('<') to find templates arguments, or namespace components. This PR replaces this with more robust searchs that should work in all cases like nested templates, operator<, namespace-qualified template arguments etc...

Also add a couple of minor improvements.

More details below.

[saudet]

Please test this on all presets to make sure it doesn't break anything.

[HGuillemet]

I checked all presets, the PR introduces no changes in the result of parsing but for:

• pytorch (added missing @NoDealloactor due to nested template argument not correctly parsed)
• gym (nothing parsed ?)
• chilitags, scipy, tensorflow, tvm, qt, skia, systems, ngraph : could not compile them on my machine without significant efforts

Can we use the CI to check this last 8 presets ?

[saudet]

Yes, but what we want to check is that the output of the Parser doesn't change in unexpected ways. We can easily build everything in Docker as per https://github.com/bytedeco/javacpp-presets/wiki/Build-Environments#linux-x86-and-x86_64

[HGuillemet]

I could compile TensorFlow using Docker. It succeeds with master and fails with this PR, but I believe the parser does the right thing now and that the problem is with the preset config file:

When parsing graph_def_builder.h, the PR, for the <tensorflow::gtl::ArraySlice<int>> instance of WithAttr gives:

 public native @ByVal @Name("WithAttr<tensorflow::gtl::ArraySlice<int> >") Options WithAttr(@StringPiece BytePointer attr_name, @ArraySlice IntPointer value);

While master produces:

    public native @ByVal Options WithAttr(@StringPiece String attr_name, @ArraySlice IntPointer value);

The @Name annotation is missing, because here the parser fails to remove the nested template arguments.

The JNI from the PR doesn't compile at this line:

        rptr = new tensorflow::GraphDefBuilder::Options(ptr->WithAttr<tensorflow::gtl::ArraySlice<int> >((tensorflow::StringPiece&)adapter0, (tensorflow::gtl::ArraySlice< int >&)adapter1));

with error:

error: cannot bind rvalue reference of type 'absl::Span<const int>&&' to lvalue of type 'tensorflow::gtl::ArraySlice<int>' {aka 'absl::Span<const int>'}

This seems to be related to the definition of the ArraySlice adapter.

[HGuillemet]

This line seems to be missing from ngraph preset. Without it the parsed Java doesn't compile with the PR:

               .put(new Info("ngraph::element::from<ngraph::float16>").javaNames("fromNGraphFloat16"))

skia doesn't pass the cppbuild, even in docker, error:

bin/gn: /lib64/libc.so.6: version `GLIBC_2.18' not found (required by bin/gn)

Nothing is parsed in Scipy ?

The parsing result doesn't differ for the remaining presets: chilitags, tvm, qt, systems.

[saudet]

Ok, thanks for checking all this. We don't need to worry about TF 1.x and nGraph, those are obsolete and won't have any new releases, so I probably won't be making any for those either.

Yes, SciPy doesn't have a native API, that's normal.

I guess something changed with the build for Skia, but anyway we're not parsing its C++ API so no need to worry about it.

[HGuillemet]

Good to merge ?

[saudet]

Well, instead of having templateThis, templateThat, maybe we could have all that in a Template class just like you did with DocTag?

[HGuillemet]

We can move static functions templateStrip and noTemplate only out of Parser to a Templates class, but this wouldn't gather all processing done in Parser about templates.
What about creating some ParserUtils class instead, to put those static functions and some more, like namespaceSplit and a couple of others to come in next PR ?

[saudet]

There's no point in creating a class like ParserUtils... The point is, no other methods use that templatePattern you want to use. So, let's do something about that, like if you want to use more patterns and put them in static variables.

[saudet]

Thanks! Looks good. Shall we merge?

[HGuillemet]

Ok for me, you may want to add the missing line in ngraph and have a look at the error for tensorflow, so that all presets can be built. Or are those built already skipped ?

[saudet]

They won't get built unless they get updated, and they won't get updated.