Fix byte-compilation warnings (again)

[wasamasa]

It looks as if there've been introduced new ones after the huge cleanup:


Compiling file /home/wasa/.emacs.d/elpa/csharp-mode-0.9.0/csharp-mode-pkg.el at Thu Jun  2 10:33:34 2016
Entering directory `/home/wasa/.emacs.d/elpa/csharp-mode-0.9.0/'

Compiling file /home/wasa/.emacs.d/elpa/csharp-mode-0.9.0/csharp-mode.el at Thu Jun  2 10:33:34 2016

In csharp-log:
csharp-mode.el:429:17:Warning: reference to free variable `csharp-log-level'

In csharp-mode:
csharp-mode.el:2942:9:Warning: (lambda nil ...) quoted with ' rather than with
    #'
csharp-mode.el:2942:9:Warning: (lambda nil ...) quoted with ' rather than with
    #'

In end of data:
csharp-mode.el:2948:1:Warning: the following functions might not be defined at runtime:
    c-skip-comments-and-strings, c-font-lock-invalid-string,
    c-font-lock-declarators, c-fontify-recorded-types-and-refs,
    c-define-abbrev-table, c-make-inherited-keymap,
    c-initialize-cc-mode, c-populate-syntax-table, c-common-init
csharp-mode.el:2948:1:Warning: the following functions are not known to be defined: csharp-log,
    csharp-is-square-parentasis-block-p,
    csharp--on-defun-open-curly-p, imenu--generic-function

• csharp-log-level is introduced after the function using it. Perhaps it's better to put customizables right at the top, like in many other packages. Alternatively, put the relevant customizables before the section using them.
• Nothing we can do about cc-mode being terrible.
• Should be fixable by eval-and-compile or something of that kind
• csharp-is-square-parentasis-block-p is a really terrible typo and the function definition should precede the c-defconst
• csharp--on-defun-open-curly-p is no more, probably removed along with the other curly predicates. Maybe it's time to actually try out these commands and rewrite them if terrible/improvable
• Require imenu when using imenu functionality

[josteink]

I noticed myself some fontification tests failing, and looking into why, it seems I've accidentally removed some functions which are definitely still referenced. Wonder how I managed to miss that.

I say step one for this issue should be restoring the missing defuns which are still referenced. I've already done one bit there in this commit:

9616fd0

[josteink]

Right now the following compilation warnings remain:

$ make clean && make csharp-mode.elc
...
In end of data:
csharp-mode.el:2963:1:Warning: the following functions might not be defined at
    runtime: c-skip-comments-and-strings, c-font-lock-invalid-string,
    c-font-lock-declarators, c-fontify-recorded-types-and-refs,
    c-define-abbrev-table, c-make-inherited-keymap, c-initialize-cc-mode,
    c-populate-syntax-table, c-common-init
csharp-mode.el:2963:1:Warning: the following functions are not known to be
    defined: csharp-log, csharp-is-square-parentasis-block-p
Wrote /home/jostein/build/csharp-mode/csharp-mode.elc

They are defined though, and earlier in the file than any of it's immediate callers, so it's looks like we have some sort of issue with evaluation order (due to a million eval-and-compile statements).

Running the test-suite all is fine too:

$ make clean && make all
...
csharp-mode.el:2963:1:Warning: the following functions are not known to be
    defined: csharp-log, csharp-is-square-parentasis-block-p
...
Ran 21 tests, 21 results as expected (2016-06-02 15:30:02+0200)

Wrapping these the two functions csharp-log and csharp-is-square-parentasis-block-p in similar eval-and-compile statements, makes the byte-compilation complete cleanly, but now the tests fail!

$ make clean && make all
...
Test fontification-of-method-names condition:
    (void-function csharp-is-square-parentasis-block-p)
   FAILED   9/21  fontification-of-method-names
...
Ran 21 tests, 20 results as expected, 1 unexpected (2016-06-02 15:31:52+0200)

1 unexpected results:
   FAILED  fontification-of-method-names

If we try to run the tests without pre-compiling, they work too:

$ make clean && make test
rm -f ../csharp-mode-0.9.0.tar
rm -rf ./.tmp/csharp-mode-0.9.0
rm -rf csharp-mode.elc csharp-mode-tests.elc
"/usr/bin/emacs" -Q -batch -L . -l csharp-mode-tests.el -f ert-run-tests-batch-and-exit
Running 21 tests (2016-06-02 15:32:44+0200)
   passed   1/21  activating-mode-doesnt-cause-failure
...
   passed  21/21  indentation-rules-should-be-as-specified-in-test-doc

Ran 21 tests, 21 results as expected (2016-06-02 15:32:45+0200)

Basically the compilation-process is eating our defuns.

One hack around that is to define the functions and then redundantly define them in eval-and-compile statements too. But there has to be a better way?? What wisdom can the great wasamasa provide? :)

[wasamasa]

Hm, so reordering doesn't do anything about it. Time to learn what these special forms do and why they're needed in the first place...

[josteink]

My latest push fixes everything we're responsible for, but we're still getting the nasty stuff from cc-mode. Basically it boils down wrapping dependant code (the one causing warnings) with (eval-when-compile).

What I tried first was (eval-and-compile), and that's quite a different beast, so yeah. My bad.

I've tried various approaches to silence the remaining cc-mode warnings, and getting the dependencies loaded before the big (c-lang-defconst), but not really getting anywhere.

Right now we have the following output:

$ make clean && make csharp-mode.elc
rm -f ../csharp-mode-0.9.0.tar
rm -rf ./.tmp/csharp-mode-0.9.0
rm -rf csharp-mode.elc csharp-mode-tests.elc
"/usr/bin/emacs" -Q -batch -L . -f batch-byte-compile csharp-mode.el

In csharp-mode:
csharp-mode.el:3000:9:Warning: (lambda nil ...) quoted with ' rather than with
    #'
csharp-mode.el:3000:9:Warning: (lambda nil ...) quoted with ' rather than with
    #'

In end of data:
csharp-mode.el:3006:1:Warning: the following functions might not be defined at
    runtime: c-skip-comments-and-strings, c-font-lock-invalid-string,
    c-font-lock-declarators, c-fontify-recorded-types-and-refs,
    c-define-abbrev-table, c-make-inherited-keymap, c-initialize-cc-mode,
    c-populate-syntax-table, c-common-init
Wrote /home/jostein/build/csharp-mode/csharp-mode.elc

Is that good enough to close this issue, or do we dig even further?

[josteink]

I spoke too early and forgot to do make clean && make all... Super-ugly hacks submitted to make build green again.

Not sure where we go from here.

[josteink]

I did a git bisect on this thing, because I know we had it fixed properly at some point. Below is the output, and I guess I should feel guilty :)

➜  csharp-mode git:(bisect) ✗ git bisect start
➜  csharp-mode git:(bisect) ✗ make clean && make all 2>&1 | egrep -a "csharp-log|square-p"
rm -f ../csharp-mode-0.9.0.tar
rm -rf ./.tmp/csharp-mode-0.9.0
rm -rf csharp-mode.elc csharp-mode-tests.elc
    (void-function csharp-log)
➜  csharp-mode git:(bisect) ✗ git bisect bad
➜  csharp-mode git:(bisect) ✗ git checkout HEAD~20
Note: checking out 'HEAD~20'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by performing another checkout.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -b with the checkout command again. Example:

  git checkout -b <new-branch-name>

HEAD is now at c4a2dfd... Fixup tests.
➜  csharp-mode git:(c4a2dfd) ✗ make clean && make all 2>&1 | egrep -a "csharp-log|square-p"
rm -f ../csharp-mode-0.8.13.tar
rm -rf ./.tmp/csharp-mode-0.8.13
rm -rf csharp-mode.elc csharp-mode-tests.elc
    defined: csharp-is-square-parentasis-block-p, imenu--subalist-p
➜  csharp-mode git:(c4a2dfd) ✗ git bisect bad
➜  csharp-mode git:(c4a2dfd) ✗ git checkout HEAD~30                                        
Previous HEAD position was c4a2dfd... Fixup tests.
HEAD is now at 367ecc2... Clean up test-code.
➜  csharp-mode git:(367ecc2) ✗ make clean && make all 2>&1 | egrep -a "csharp-log|square-p"
rm -f ../csharp-mode-0.8.11.tar
rm -rf ./.tmp/csharp-mode-0.8.11
rm -rf csharp-mode.elc csharp-mode-tests.elc
    defined: csharp-is-square-parentasis-block-p, imenu--subalist-p
➜  csharp-mode git:(367ecc2) ✗ git bisect bad                                              
➜  csharp-mode git:(367ecc2) ✗ make clean && make all 2>&1 | egrep -a "csharp-log|square-p"
➜  csharp-mode git:(367ecc2) ✗ git checkout HEAD~40                                        
Previous HEAD position was 367ecc2... Clean up test-code.
HEAD is now at 421d3de... remove aspx code
➜  csharp-mode git:(421d3de) ✗ make clean && make all 2>&1 | egrep -a "csharp-log|square-p"
rm -f ../csharp-mode-0.8.8.tar
rm -rf ./.tmp/csharp-mode-0.8.8
rm -rf LCS
➜  csharp-mode git:(421d3de) ✗ git bisect good
Bisecting: 37 revisions left to test after this (roughly 5 steps)
[5ecae8b91a741bab6ea19d8cc9d1235c01ca5f04] Add support for devenv compilation-output.
➜  csharp-mode git:(5ecae8b) ✗ make clean && make all 2>&1 | egrep -a "csharp-log|square-p"
rm -f ../csharp-mode-0.8.10.tar
rm -rf ./.tmp/csharp-mode-0.8.10
rm -rf LCS
    defined: csharp-is-square-parentasis-block-p, imenu--subalist-p
➜  csharp-mode git:(5ecae8b) ✗ git bisect bad
Bisecting: 17 revisions left to test after this (roughly 4 steps)
[c9e1aa9f748b7f99349aa6f66f37a61ff7d00f97] Merge pull request #29 from josteink/compilation-regexps
➜  csharp-mode git:(c9e1aa9) ✗ make clean && make all 2>&1 | egrep -a "csharp-log|square-p"
rm -f ../csharp-mode-0.8.8.tar
rm -rf ./.tmp/csharp-mode-0.8.8
rm -rf LCS
    defined: csharp-is-square-parentasis-block-p, imenu--subalist-p
➜  csharp-mode git:(c9e1aa9) ✗ git bisect bad                                              
Bisecting: 9 revisions left to test after this (roughly 3 steps)
[e73ad4e49d7de288bff3b307ca9b32601308ecb4] Merge pull request #30 from binki/compilation-regexps
➜  csharp-mode git:(e73ad4e) ✗ make clean && make all 2>&1 | egrep -a "csharp-log|square-p"
rm -f ../csharp-mode-0.8.8.tar
rm -rf ./.tmp/csharp-mode-0.8.8
rm -rf LCS
    defined: csharp-is-square-parentasis-block-p, imenu--subalist-p
➜  csharp-mode git:(e73ad4e) ✗ git bisect bad                                              
Bisecting: 4 revisions left to test after this (roughly 2 steps)
[0a2a3b0943c2463ca38b673c6c42d2e3e9bc2d5a] Fix Emacs-lockup during fontification.
➜  csharp-mode git:(0a2a3b0) ✗ make clean && make all 2>&1 | egrep -a "csharp-log|square-p"
rm -f ../csharp-mode-0.8.8.tar
rm -rf ./.tmp/csharp-mode-0.8.8
rm -rf LCS
    defined: csharp-is-square-parentasis-block-p, imenu--subalist-p
➜  csharp-mode git:(0a2a3b0) ✗ git bisect bad                                              
Bisecting: 1 revision left to test after this (roughly 1 step)
[a86839b42b02955ee7670235d475f6f8779b6aaa] Merged from master.
➜  csharp-mode git:(a86839b) ✗ make clean && make all 2>&1 | egrep -a "csharp-log|square-p"
rm -f ../csharp-mode-0.8.8.tar
rm -rf ./.tmp/csharp-mode-0.8.8
rm -rf LCS
➜  csharp-mode git:(a86839b) ✗ git bisect good
Bisecting: 0 revisions left to test after this (roughly 0 steps)
[e4ce203571137ac248dcc77af6792504861868be] Updated readme.
➜  csharp-mode git:(e4ce203) ✗ make clean && make all 2>&1 | egrep -a "csharp-log|square-p"
rm -f ../csharp-mode-0.8.8.tar
rm -rf ./.tmp/csharp-mode-0.8.8
rm -rf LCS
➜  csharp-mode git:(e4ce203) ✗ git bisect good                                             
0a2a3b0943c2463ca38b673c6c42d2e3e9bc2d5a is the first bad commit
commit 0a2a3b0943c2463ca38b673c6c42d2e3e9bc2d5a
Author: Jostein Kjønigsen <jostein@kjonigsen.net>
Date:   Sun Jan 25 16:08:32 2015 +0100

    Fix Emacs-lockup during fontification.

    Fixes non-exiting loop in detection of square-parentasis regions.

    Closes https://github.com/josteink/csharp-mode/issues/17.

:100644 100644 75e31305caf8534ff17fb6bb6267e3203b828120 e2003e7849035c472e30e9f320ad1897220dd544 M  csharp-mode.el

Based on this, I'll try to get things fixed.

But at the same time, I'd also love for the build process to fail if stuff like this happens again. Unresolvable functions are not cool. I'll create a separate issue for that.

[josteink]

Issue with square parenthesis defun is now fixed. Remaining is csharp-log and then this issue can be closed.

@wasamasa : A git bisect identifies you as the guilty party for this one. You fix? :)

➜  csharp-mode git:(7fa38d2) ✗ make clean && make all 2>&1 | egrep -a "csharp-log"
rm -f ../csharp-mode-0.8.13.tar
rm -rf ./.tmp/csharp-mode-0.8.13
rm -rf csharp-mode.elc csharp-mode-tests.elc
➜  csharp-mode git:(7fa38d2) ✗ git bisect good                                    
bd201c295f7bbacc9df3fed671c5ad4a31cf4ce8 is the first bad commit
commit bd201c295f7bbacc9df3fed671c5ad4a31cf4ce8
Author: Vasilij Schneidermann <v.schneidermann@gmail.com>
Date:   Sat May 21 11:16:16 2016 +0200

    Remove now unused code, move log functions to top

:100644 100644 7f73889189fcc3e9771bc8ace69615b76c3799c7 c8461a351fba7ad6a31e6502effa81f109d7238d M  csharp-mode.el
➜  csharp-mode git:(7fa38d2) ✗ 

[wasamasa]

Oh damnit. Well, if it's been the removal of unused code, I'll be able to find out what part of it has been still in use after all.

[wasamasa]

Weird, I do no longer get a warning about the log warning on HEAD.

[josteink]

I removed my hacks which was used to mask the problem, and undid the "move log functions to the top" part of your commit, and that seems to have fixed it.

Basically moving the log-functions further to the top, caused compiler warnings about them not being defined.

For now, we can consider this issue fixed, but we clearly have some very very weird evaluation order going on here...

[wasamasa]

Wow, that sure seems weird.

My compilation log looks like this at the moment:

Leaving directory `/home/wasa/.emacs.d/elpa/csharp-mode-0.9.0'

Compiling file /home/wasa/.emacs.d/elpa/csharp-mode-0.9.0/csharp-mode-pkg.el at Sun Jun  5 09:23:27 2016
Entering directory `/home/wasa/.emacs.d/elpa/csharp-mode-0.9.0/'

Compiling file /home/wasa/.emacs.d/elpa/csharp-mode-0.9.0/csharp-mode.el at Sun Jun  5 09:23:27 2016

In csharp-mode:
csharp-mode.el:2978:9:Warning: (lambda nil ...) quoted with ' rather than with
    #'
csharp-mode.el:2978:9:Warning: (lambda nil ...) quoted with ' rather than with
    #'

In end of data:
csharp-mode.el:2984:1:Warning: the following functions might not be defined at runtime:
    c-skip-comments-and-strings, c-font-lock-invalid-string,
    c-font-lock-declarators, c-fontify-recorded-types-and-refs,
    c-define-abbrev-table, c-make-inherited-keymap,
    c-initialize-cc-mode, c-populate-syntax-table, c-common-init

[josteink]

Yeah. Same as me. I seemed to remember that we never managed to get rid of those final warnings, so I didn't bother bisecting further.

Or do I remember incorrectly?

Would you bother checking out how much managed to clean this up before we had this regression?

[wasamasa]

I get similar output on 69d95d2. so I'm closing this one.

[josteink]

Agreed. I checked the git log for last time we fixed something like this (basically this commit here):

6289cfd

We still had the same warnings back then, so I totally agree. That means any possible resolution for #81 will need to whitelist cc-mode related function-references which we cannot fix.