Fix org.el compilation warnings

Fix org.el compilation warnings

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 1554 bytes --]

Hello,

I was trying to fix the org.el compilation warnings.

I could fix just this one:

From 4e2de052dd05e66ed71ce070e4413859e2c13238 Mon Sep 17 00:00:00 2001
From: Kaushal Modi <kaushal.modi@gmail.com>
Date: Thu, 25 May 2017 18:24:34 -0400
Subject: [PATCH] org: Silence byte-compiler

* lisp/org.el (org-agenda-include-inactive-timestamps): Silence
  byte-compiler.
---
 lisp/org.el | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lisp/org.el b/lisp/org.el
index 946d8af8c..d1c70ab97 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -175,6 +175,8 @@ Stars are put in group 1 and the trimmed body in group
2.")
 (declare-function org-export-get-environment "ox" (&optional backend
subtreep ext-plist))
 (declare-function org-latex-make-preamble "ox-latex" (info &optional
template snippet?))

+(defvar org-agenda-include-inactive-timestamps) ;Silence byte-compiler
+
 (defsubst org-uniquify (list)
   "Non-destructively remove duplicate elements from LIST."
   (let ((res (copy-sequence list))) (delete-dups res)))
-- 
2.13.0


There are still these 4 warnings for org.el:

In org-goto-map:
org.el:7502:9:Warning: function org-goto-map used to take 0+ arguments, now
    takes 0

In org-todo:
org.el:12488:12:Warning: function org-todo used to take 0+ arguments, now
    takes 0-1

In org-store-log-note:
org.el:13725:8:Warning: function org-store-log-note used to take 0+
arguments,
    now takes 0

In org-self-insert-command:
org.el:20000:32:Warning: function org-self-insert-command used to take 0+
    arguments, now takes 1

-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 2355 bytes --]

Re: Fix org.el compilation warnings

[Kyle Meyer]

Kaushal Modi <kaushal.modi@gmail.com> writes:

> I was trying to fix the org.el compilation warnings.

I don't see any compiler warnings when I run 'make compile' with either
master (2960dc971) or maint (89bd7ad87) on Emacs 25.2.

-- 
Kyle

Re: Fix org.el compilation warnings

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 609 bytes --]

On Thu, May 25, 2017, 7:59 PM Kyle Meyer <kyle@kyleam.com> wrote:

> Kaushal Modi <kaushal.modi@gmail.com> writes:
>
> > I was trying to fix the org.el compilation warnings.
>
> I don't see any compiler warnings when I run 'make compile' with either
> master (2960dc971) or maint (89bd7ad87) on Emacs 25.2.
>

Those warnings were on master using the latest build of emacs from master.

I was compiling just org.el, may be that's why?

But even then, shouldn't the free variable warning, that I fix in the patch
I just provided, be fixed?

I'll try 'make compile' when I get to a computer.

> --

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1275 bytes --]

Re: Fix org.el compilation warnings

[Kyle Meyer]

Kaushal Modi <kaushal.modi@gmail.com> writes:

> On Thu, May 25, 2017, 7:59 PM Kyle Meyer <kyle@kyleam.com> wrote:
>
>> Kaushal Modi <kaushal.modi@gmail.com> writes:
>>
>> > I was trying to fix the org.el compilation warnings.
>>
>> I don't see any compiler warnings when I run 'make compile' with either
>> master (2960dc971) or maint (89bd7ad87) on Emacs 25.2.
>>
>
> Those warnings were on master using the latest build of emacs from master.
>
> I was compiling just org.el, may be that's why?

Ah, right ... 'make compile' uses a single Emacs instance, while 'make
single' uses a separate Emacs instance for each file.

With 'make single' on master, I get a compile error due to the
eval-when-compile's added in 53ee147f4 (Add support for new switches to
org-get-heading, 2017-01-17) and 6dc6eb3b0 (Fix failing test,
2017-01-19).

    In toplevel form:
    org.el:7914:51:Error: Symbol’s value as variable is void: org-comment-string

If I wrap (defconst org-comment-string ...) in eval-and-compile to get
rid of those, I see your reported warning and a few others (but not the
others that you reported):

    In org-at-timestamp-p:
    org.el:17946:40:Warning: reference to free variable
        ‘org-agenda-include-inactive-timestamps’
    
    In org--get-expected-indentation:
    org.el:22608:28:Warning: reference to free variable
        ‘org-element-greater-elements’
    
    In end of data:
    org.el:24980:1:Warning: the following functions are not known to be defined:
        org-table-sort-lines, org-duration-from-minutes

I think all of these should be addressed.

> But even then, shouldn't the free variable warning, that I fix in the patch
> I just provided, be fixed?

Your patch is only the appropriate fix if we're confident that
org-agenda will be loaded at the time that the
org-agenda-include-inactive-timestamps code path of org-at-timestamp-p
is executed.  Otherwise, we're just silencing a warning about a possible
run-time void variable error.  Have you looked this?

-- 
Kyle

[PATCH] Silence byte-compiler under "make single"

[Kyle Meyer]

Kyle Meyer <kyle@kyleam.com> writes:

> With 'make single' on master, I get a compile error due to the
> eval-when-compile's added in 53ee147f4 (Add support for new switches to
> org-get-heading, 2017-01-17) and 6dc6eb3b0 (Fix failing test,
> 2017-01-19).
>
>     In toplevel form:
>     org.el:7914:51:Error: Symbol’s value as variable is void: org-comment-string
>
> If I wrap (defconst org-comment-string ...) in eval-and-compile to get
> rid of those, I see your reported warning and a few others (but not the
> others that you reported):
>
>     In org-at-timestamp-p:
>     org.el:17946:40:Warning: reference to free variable
>         ‘org-agenda-include-inactive-timestamps’
>     
>     In org--get-expected-indentation:
>     org.el:22608:28:Warning: reference to free variable
>         ‘org-element-greater-elements’
>     
>     In end of data:
>     org.el:24980:1:Warning: the following functions are not known to be defined:
>         org-table-sort-lines, org-duration-from-minutes

The org-element-greater-elements warning was the only warning present on
maint, and that's covered by 6d4c188e3 now on the tip of maint.  The
following patch should take care of the rest of the "make single" issues
on master (with Emacs 25.2, at least).

-- >8 --
Subject: [PATCH] Silence byte-compiler under "make single"

* lisp/org.el (org-comment-string): Wrap definition in an
eval-and-compile because this variable is used within the body of
eval-when-compile, leading to an error under "make single".
(org-at-timestamp-p): Use bound-and-true-p to check
org-agenda-include-inactive-timestamps because org-agenda may not be
loaded yet.

Reported-by: Kaushal Modi <kaushal.modi@gmail.com>
<https://lists.gnu.org/archive/html/emacs-orgmode/2017-05/msg00326.html>
---
 lisp/org.el | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index a50966ea1..59cc9b4cb 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -129,6 +129,8 @@ (declare-function org-clock-timestamps-down "org-clock" (&optional n))
 (declare-function org-clock-timestamps-up "org-clock" (&optional n))
 (declare-function org-clock-update-time-maybe "org-clock" ())
 (declare-function org-clocktable-shift "org-clock" (dir n))
+(declare-function
+ org-duration-from-minutes "org-duration" (minutes &optional fmt canonical))
 (declare-function org-element-at-point "org-element" ())
 (declare-function org-element-cache-refresh "org-element" (pos))
 (declare-function org-element-cache-reset "org-element" (&optional all))
@@ -167,6 +169,9 @@ (declare-function org-table-maybe-recalculate-line "org-table" ())
 (declare-function org-table-next-row "org-table" ())
 (declare-function org-table-paste-rectangle "org-table" ())
 (declare-function org-table-recalculate "org-table" (&optional all noalign))
+(declare-function
+ org-table-sort-lines "org-table"
+ (&optional with-case sorting-type getkey-func compare-func interactive?))
 (declare-function org-table-wrap-region "org-table" (arg))
 (declare-function org-tags-view "org-agenda" (&optional todo-only match))
 (declare-function orgtbl-ascii-plot "org-table" (&optional ask))
@@ -525,11 +530,12 @@ (defconst org-archive-tag "ARCHIVE"
 An archived subtree does not open during visibility cycling, and does
 not contribute to the agenda listings.")
 
-(defconst org-comment-string "COMMENT"
-  "Entries starting with this keyword will never be exported.
+(eval-and-compile
+  (defconst org-comment-string "COMMENT"
+    "Entries starting with this keyword will never be exported.
 \\<org-mode-map>
 An entry can be toggled between COMMENT and normal with
-`\\[org-toggle-comment]'.")
+`\\[org-toggle-comment]'."))
 
 
 ;;;; LaTeX Environments and Fragments
@@ -17942,7 +17948,8 @@ (defun org-at-timestamp-p (&optional extended)
 		     (or (and (eq extended 'agenda)
 			      (or (org-at-planning-p)
 				  (org-at-property-p)
-				  (and org-agenda-include-inactive-timestamps
+				  (and (bound-and-true-p
+					org-agenda-include-inactive-timestamps)
 				       (org-at-clock-log-p))))
 			 (eq 'timestamp
 			     (save-excursion
-- 
2.13.0

Re: Fix org.el compilation warnings

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 2510 bytes --]

Below update is after I updated to the latest org master:

On Thu, May 25, 2017 at 10:53 PM Kyle Meyer <kyle@kyleam.com> wrote:

> Ah, right ... 'make compile' uses a single Emacs instance, while 'make
> single' uses a separate Emacs instance for each file.
>

I wasn't even doing that. I was doing M-x byte-compile-file and selecting
org.el.

That gives me (without applying your patch):

Compiling file
/home/kmodi/stow/pub_dotfiles/emacs/dot-emacs.d/elisp/org-mode/lisp/org.el
at Fri May 26 14:04:52 2017

In org-goto-map:
org.el:7412:9:Warning: function org-goto-map used to take 0+ arguments, now
    takes 0

In org-todo:
org.el:12398:12:Warning: function org-todo used to take 0+ arguments, now
    takes 0-1

In org-store-log-note:
org.el:13635:8:Warning: function org-store-log-note used to take 0+
arguments,
    now takes 0

In org-at-timestamp-p:
org.el:17909:40:Warning: reference to free variable
    ‘org-agenda-include-inactive-timestamps’

In org-self-insert-command:
org.el:19910:32:Warning: function org-self-insert-command used to take 0+
    arguments, now takes 1

With 'make single' on master, I get a compile error due to the
> eval-when-compile's added in 53ee147f4 (Add support for new switches to
> org-get-heading, 2017-01-17) and 6dc6eb3b0 (Fix failing test,
> 2017-01-19).


>     In toplevel form:
>     org.el:7914:51:Error: Symbol’s value as variable is void:
> org-comment-string
>

I get that too.


> Your patch is only the appropriate fix if we're confident that
> org-agenda will be loaded at the time that the
> org-agenda-include-inactive-timestamps code path of org-at-timestamp-p
> is executed.  Otherwise, we're just silencing a warning about a possible
> run-time void variable error.  Have you looked this?


No, I didn't.. your bound-and-true-p patch makes more sense.

So.. should we consider the warnings output by simple byte-compile-file?
Even if not, the warnings do not make sense.. just taking this example:

In org-goto-map:
org.el:7412:9:Warning: function org-goto-map used to take 0+ arguments, now
    takes 0

Nowhere is org-goto-map defined to take 0+ arguments (meaning have
"&optional arg" as signature) and then redefined to have 0 arguments (which
is the actual case). Wondering why the compiler thinks that. I had a quick
look at byte-compile-arglist-warn function that outputs this warning.. but
couldn't understand the logic for sig1 in there.
-- 

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 3712 bytes --]

Re: Fix org.el compilation warnings

[Kyle Meyer]

Kaushal Modi <kaushal.modi@gmail.com> writes:

>> Ah, right ... 'make compile' uses a single Emacs instance, while 'make
>> single' uses a separate Emacs instance for each file.
>
> I wasn't even doing that. I was doing M-x byte-compile-file and selecting
> org.el.

Sure, I was just noting why I didn't see the
org-agenda-include-inactive-timestamps warning when I ran 'make
compile'.  .

> That gives me (without applying your patch):
>
> Compiling file
> /home/kmodi/stow/pub_dotfiles/emacs/dot-emacs.d/elisp/org-mode/lisp/org.el
> at Fri May 26 14:04:52 2017
>
> In org-goto-map:
> org.el:7412:9:Warning: function org-goto-map used to take 0+ arguments, now
>     takes 0
>
> In org-todo:
> org.el:12398:12:Warning: function org-todo used to take 0+ arguments, now
>     takes 0-1
>
> In org-store-log-note:
> org.el:13635:8:Warning: function org-store-log-note used to take 0+
> arguments,
>     now takes 0
>
> In org-at-timestamp-p:
> org.el:17909:40:Warning: reference to free variable
>     ‘org-agenda-include-inactive-timestamps’
>
> In org-self-insert-command:
> org.el:19910:32:Warning: function org-self-insert-command used to take 0+
>     arguments, now takes 1
>
> So.. should we consider the warnings output by simple byte-compile-file?

When I run byte-compile-file on org.el (no Org loaded, with master's Org
first on the load-path), I don't see any of the above warnings aside
from the one for org-agenda-include-inactive-timestamps, which is fixed
by the patch.  And because running byte-compile-file from an Emacs
instance is affected by what's already been loaded, I think it's better
to focus on the output of 'make single'.

-- 
Kyle

Re: Fix org.el compilation warnings

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 943 bytes --]

On Fri, May 26, 2017, 4:45 PM Kyle Meyer <kyle@kyleam.com> wrote:

>
> When I run byte-compile-file on org.el (no Org loaded, with master's Org
> first on the load-path), I don't see any of the above warnings aside
> from the one for org-agenda-include-inactive-timestamps, which is fixed
> by the patch.  And because running byte-compile-file from an Emacs
> instance is affected by what's already been loaded, I think it's better
> to focus on the output of 'make single'.
>

I thought that mixed org versions on load path could cause this. So I
checked the old org that ships with emacs master (About that, I'd like to
help syncing the org on emacs master with org 9.0.7 if I can.), but I didnt
find the function signature discrepancy as the warnings indicate.

So just for understanding, would like to know the root cause of those
warnings.

But yes, for true compilation checks, I'll start using make single.

Thanks.

> --

Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1541 bytes --]

syncing with Emacs (was: Fix org.el compilation warnings)

[Kyle Meyer]

Kaushal Modi <kaushal.modi@gmail.com> writes:

> (About that, I'd like to help syncing the org on emacs master with org
> 9.0.7 if I can.)

Rasmus was working on it the last time it came up.  I'm not sure what
the status of that is.

Each Org release, I've been updating the emacs-sync branch.  This branch
is a superset of the last release, with a few changes that I think are
appropriate for syncing with Emacs.  You can see these with

    git log --oneline --no-merges release_9.0.7..origin/emacs-sync

As far as backports, maint should be up to date, though I haven't
checked since last Sunday.  And based on the output of

    git log --oneline release_9.0.7..origin/maint

it seems that the last release includes all of the backports.

-- 
Kyle