ch11ng / xelb

X protocol Emacs Lisp Binding
216 stars 21 forks source link

Compile warnings #20

Closed tarsius closed 6 years ago

tarsius commented 6 years ago

Compiling xelb currently results in the following warnings:

In toplevel form:
xcb-types.el:83:1:Warning: value returned from (fboundp (quote
    eieio-class-slots)) is unused
xcb-types.el:460:3:Warning: Unused lexical argument ‘fn’
xcb-types.el:460:3:Warning: Unused lexical argument ‘slot-name’
xcb-types.el:460:3:Warning: Unused lexical argument ‘class’
xcb-types.el:460:3:Warning: Unused lexical argument ‘object’

I recommend you give my auto-compile a try as it greatly increases the odds that you catch such warnings before even pushing.

ch11ng commented 6 years ago

Thanks! I usually do compile my codes before making them public, and had come across the warnings you reported. The problem is those warnings are difficult to eliminate. Do you have any ideas?

tarsius commented 6 years ago

The ones about unused argument are easy - just prefix the symbol with an underscore like _so. That means "I know the variable isn't used and that is okay".

I don't understand the use of two eval-and-compile around the definition of eieio-class-slots. Removing the inner instance seems to fix this, but I don't know whether has other consequences.

tarsius commented 6 years ago

While I am here; could you please tag releases? That would make your packages available from Melpa-Stable.

ch11ng commented 6 years ago

The ones about unused argument are easy - just prefix the symbol with an underscore like _so. That means "I know the variable isn't used and that is okay".

That did the trick.

I don't understand the use of two eval-and-compile around the definition of eieio-class-slots. Removing the inner instance seems to fix this, but I don't know whether has other consequences.

Removing the inner one would bring up other warnings. So I ended up hard-coding the obsolete functions.

While I am here; could you please tag releases? That would make your packages available from Melpa-Stable.

GNU ELPA is already hosting this package and I don't if this would pose a risk of conflict with Melpa-Stable (for instance, a same version with different commit, considering GNU ELPA checks for the library header).

tarsius commented 6 years ago

I think most packages that are available from GNU Elpa but are maintained in a separate repository also use tags. As long as you make sure that you tag the exact commit in which ooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo

tarsius commented 6 years ago

... kids ...

tarsius commented 6 years ago

... in which you bump the library header, there should be no problem.

ch11ng commented 6 years ago

I'm not quite sure how GNU ELPA decides which commit to pick when there's a version change in the library header. It seems the packaging server checks that daily and perhaps it would pick the most up-to-date commit?

tarsius commented 6 years ago

perhaps it would pick the most up-to-date commit?

That's not what happens, from GNU Elpa's README (emphasis mine):

This cron job only creates a new package when the "version" (as specified in the "Version:" header) of a package is modified. This means that you can safely work on the next version here without worrying about the unstable code making it to GNU ELPA, and simply update the "version" when you want to release the new code.

ch11ng commented 6 years ago

That makes sense. I've tagged a release.

tarsius commented 6 years ago

That makes sense. I've tagged a release.

Thanks!

I don't understand the use of two eval-and-compile around the definition of eieio-class-slots. Removing the inner instance seems to fix this, but I don't know whether has other consequences.

Removing the inner one would bring up other warnings. So I ended up hard-coding the obsolete functions.

I don't know what is going on but just noticed that the consequences of the double eval-and-compile are worse for me than previously stated. When using by borg package manager it causes a hard error that prevents it from completing its job. (It's pretty weird because it doesn't error immediately but instead waits until some later (but predictable) point to terminate emacs with 255.) Maybe that is something that could happen in other contexts too and I could and should do something to prevent that sort of issue, but I don't have the time to investigate that right now.

It seems that we both don't really understand how eval-and-compile works and that what you are doing is not an intended use-case. So I think it would be better to drop this and instead live with the compile warnings (which you might actually be able to suppress using with-no-warnings instead).

ch11ng commented 6 years ago

So the behavior of nested eval-and-compile seems to be undefined. I originally thought the inner one would not alter the meaning of code and could be used as a resort to the warnings of undefined functions. Luckily we have dropped that use.

tarsius commented 6 years ago

Yes :smile:

tarsius commented 6 years ago

Thanks again!