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

Parser exception when expanding macros of macros #674

Closed
HGuillemet opened this issue Apr 19, 2023 · 19 comments
Closed

Parser exception when expanding macros of macros #674

HGuillemet opened this issue Apr 19, 2023 · 19 comments

Comments

@HGuillemet
Copy link
Contributor

HGuillemet commented Apr 19, 2023

#define X(methodname) \
  void methodname(    \
      int i);

class C {
#define Y X(f)

public:
  Y
};

Parsing this header throws:

Info: Parsing m.h
Exception in thread "main" org.bytedeco.javacpp.tools.ParserException: m.h:6:"#define X(methodname)
  void methodname(
      int i);": Could not parse declaration at ')'
	at org.bytedeco.javacpp.tools.Parser.declarations(Parser.java:4220)

I think this is related to \n in the first macro that are expanded in the second as true \n, making this loop end too soon.

How to fix this ?

@HGuillemet HGuillemet changed the title Expansion of macros of macros Parser exception when expanding macros of macros Apr 19, 2023
@saudet saudet added the bug label Apr 21, 2023
saudet added a commit that referenced this issue Apr 21, 2023
@saudet
Copy link
Member

saudet commented Apr 21, 2023

Fixed in commit c5a9b10

@HGuillemet
Copy link
Contributor Author

Thanks. No more error.
But in the example provided, the method is not generated either.
Is that deliberate ?

@saudet
Copy link
Member

saudet commented Apr 21, 2023

@HGuillemet
Copy link
Contributor Author

To ignore the macro ? But what if I want the function to be defined ?
In the example above, with the patch, it is not generated.
But it works if I add parentheses to the macro:

class C {
#define Y() X(f)

public:
  Y();
};

@saudet
Copy link
Member

saudet commented Apr 22, 2023

It works either way for me.

@HGuillemet
Copy link
Contributor Author

Strange.
Also can you try to parse the pytorch presets ? I have an error parsing typeid.h that I don't have without the patch.

@saudet
Copy link
Member

saudet commented Apr 22, 2023 via email

@HGuillemet
Copy link
Contributor Author

The pytorch presest in javacpp-presets master did parse with javacpp master before your commit, and not after. What do you mean by updating ?

@saudet
Copy link
Member

saudet commented Apr 23, 2023

Ah, I thought that the presets for 1.13.1 were broken anyway, so I didn't test them.

PyTorch 2.0 has been released for a while now, so if you don't intend on updating the presets, I'll do it I guess? It sounds like you're again trying to do everything at the same time instead of focusing only on a couple of things, and I'm afraid it's going to take forever to sort it out.

@HGuillemet
Copy link
Contributor Author

Ah, I thought that the presets for 1.13.1 were broken anyway, so I didn't test them.

They compile before this commit.

PyTorch 2.0 has been released for a while now, so if you don't intend on updating the presets, I'll do it I guess?

That's while working on them that I encountered this issue and the last reported ones.

It sounds like you're again trying to do everything at the same time instead of focusing only on a couple of things, and I'm afraid it's going to take forever to sort it out.

If you have a crowd of active users of these presets reclaiming an update, I can try to just bump 1.13.0 to 2.0.0 and adjust some missing header, skipping whatever triggers a new bug, but I doubt it's worth.

saudet added a commit that referenced this issue Apr 25, 2023
@saudet
Copy link
Member

saudet commented Apr 25, 2023

Ok, I've fixed the regression in commit be4df66 and also updated the presets for PyTorch in commit bytedeco/javacpp-presets@0c698a7, and everything builds successfully so please rebase your changes on that!

@HGuillemet
Copy link
Contributor Author

Thanks, this will give me more time for the new version of the presets.

Ok, I've fixed the regression in commit be4df66

No more error, with this last commit.
Although for me the example above still doesn't map the function. Is it the wanted behavior ?

and also updated the presets for PyTorch in commit bytedeco/javacpp-presets@0c698a7, and everything builds successfully so please rebase your changes on that!

Thanks, this will give me more time for the new version of the presets.

@saudet
Copy link
Member

saudet commented Apr 27, 2023

Like I said, this works for me as long as I do this:
https://github.com/bytedeco/javacpp/wiki/Mapping-Recipes#ignoring-attributes-and-macros
If you can't do that though that's a different issue, so please provide more details.

@HGuillemet
Copy link
Contributor Author

It now parses without error even without info to ignore the macros.
But the macro is not expanded correctly.

More details:

#define X(methodname) void methodname(int i)

class C {
#define Y() X(y)
#define Z X(z)

public:
  Y();
  Z;
};

gcc -E output:

class C {
public:
  void y(int i);
  void z(int i);
};

Parser output:

public class C extends Pointer {
    static { Loader.load(); }
    /** Default native constructor. */
    public C() { super((Pointer)null); allocate(); }
    /** Native array allocator. Access with {@link Pointer#position(long)}. */
    public C(long size) { super((Pointer)null); allocateArray(size); }
    /** Pointer cast constructor. Invokes {@link Pointer#Pointer(Pointer)}. */
    public C(Pointer p) { super(p); }
    private native void allocate();
    private native void allocateArray(long size);
    @Override public C position(long position) {
        return (C)super.position(position);
    }
    @Override public C getPointer(long i) {
        return new C((Pointer)this).offsetAddress(i);
    }

  public native void y(int i);
}

z is missing.

@saudet
Copy link
Member

saudet commented Apr 28, 2023

And what does the InfoMapper look like?

@HGuillemet
Copy link
Contributor Author

Empty

@saudet
Copy link
Member

saudet commented Apr 29, 2023

Like I keep telling you, what's the reason why you can't do it like this?
https://github.com/bytedeco/javacpp/wiki/Mapping-Recipes#ignoring-attributes-and-macros
Is it just because you absolutely just want to get this working by default without any Info? If so, that's not something I'll spend time on myself, because this workaround works just fine:
https://github.com/bytedeco/javacpp/wiki/Mapping-Recipes#ignoring-attributes-and-macros
You'll need to explain the reason why you can't do that. Doing that doesn't "ignore" the macro. This works just fine for me, so unless there's a good reason you can't do that, I'm not going to spend any more time on this!

@HGuillemet
Copy link
Contributor Author

I, probably stupidly, though that the recipe titled "Ignoring attributes and macros", would ignore the macro, and not the reverse.

And that if you pointed it to me, it was to get rid of the parsing error, that you fixed now.

But it does make produce the correct output, I don't know with which magic. So closing this issue.

And I remind you that I don't want anything or try to make you work for useless things. I report bugs that you might be interested to know about, in order to help you improve your product. I'll avoid that in the future, and only report blocking issues for which I have a PR ready to be merged.

@saudet
Copy link
Member

saudet commented Apr 30, 2023

JavaCPP isn't a product, it's an open-source project, and it's not going to go anywhere if I remain the only one working on it. I'm glad that you're able to spend time improving it, but there hasn't been anyone else that contributes consistently, so yeah I'm not going to spend time fixing things that aren't broken. It's just not something the Java community cares about, and that tells a lot about the Java community, unfortunately.

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

2 participants