reserved identifier violation

mypaint / libmypaint

libmypaint, a.k.a. "brushlib", is a library for making brushstrokes which is used by MyPaint and other projects.

http://mypaint.org

Other

310 stars 86 forks source link

reserved identifier violation #45

Closed elfring closed 8 years ago

elfring commented 8 years ago

I would like to point out that identifiers like "_MyPaintSurface" and "_OperationQueue" do not fit to the expected naming convention of the C++ language standard. Would you like to adjust your selection for unique names?

odysseywestra commented 8 years ago

@elfring if you want, you can fork it and fix the identifiers and send us a pull request with the fix. That would be a great help.

elfring commented 8 years ago

Will a renaming of such identifiers affect also the application binary interface for this software?

achadwick commented 8 years ago

libmypaint is written in C, and loosely conforms to the C99 standard. Why are you asking for this?

achadwick commented 8 years ago

@elfring We'll have to consider API consistency more carefully than we do in future. ABI consistency is less important IMO.

A bit more context would help us understand the issue you are having. What problems is are this naming causing you? Otherwise, for now, I'm not sure if this is an issue we need to fix, frankly.

elfring commented 8 years ago

Why are you asking for this?

I imagine that more software release planning would be needed if you care for API/ABI stability around affected identifiers.

What problems is this naming causing you?

How do you think about to avoid that this software depends on undefined behaviour?

achadwick commented 8 years ago

Hit me with practical suggestions about how we can improve things, tell me what's actually going wrong, or (better still) make PRs to fix dud behaviour please.

We can also consider automated tools as suggested in #46.

achadwick commented 8 years ago

Still no practical suggestions or error messages from the reporter. Our codebase seems to be merely doing this to avoid confusing struct Foo and Foo types though, which is silly.

Picking this up for the release for another look. There are structures which probably should go private just for the sake of cleaning up the interface.

@elfring Tell me what problems this is causing you, or give me a concrete way of testing this. Compiler error messages, test cases, anything. Better still, send a PR.

elfring commented 8 years ago

Would you really like to "test" undefined behaviour? ;-)

achadwick commented 8 years ago

@elfring Yes please. Suggestions for linters, compiler flags, or similar checks.

elfring commented 8 years ago

Would you like to try an ordinary source code search out again?
How do you think about to achieve improvements in the affected software application domain?

achadwick commented 8 years ago

As if I haven't already, you mean?

Thanks for the link to the ubsan information on the RedHat blog. Looks as if it could be useful, but I note one of the comments on the post:

Most GNOME projects are maintained or contributed to by Red Hat and the use of reserved identifiers is pervasive among those projects. By pervasive, I mean close to 100% of GNOME projects do it and many of the examples in the documentation do too. It’s almost like some kind of style guide there.

Dunno if our use of GLib or GI will be a problem there.

elfring commented 8 years ago

Thanks for your source code improvement.