Issuing deprecation warnings

Issuing deprecation warnings

[Daniel Kraft]

Hi,

I've just tried to build latest-git guile with --disable-deprecated 
(full configure was --enable-maintainer-mode --disable-nls 
--disable-deprecated), and it refuses to compile because (at least) in 
libguile/gc-mark.c a deprecation warning is issued without checking if 
deprecation is enabled.

As I understand it, the call there warns if scm_gc_mark is called but 
not from the internal GC handling.  As this is "no feature" that could 
be excluded (except making scm_gc_mark invisible for 
--disable-deprecated or something like that), I don't see a way to fix 
this there other than just removing the warning in case of 
--disable-deprecated.

However, I also get the idea that scm_c_issue_deprecation_warning and 
friends are not considered "private internal functions" but instead part 
of the Guile API (are they?).  In this case, I don't think removing them 
with --disable-deprecated is appropriate, as user code might try to use 
them to signal deprecation of some user's extension procedures.  On the 
other hand, libguile/deprecation.h has a comment that explicitly states 
those functions *should* be excluded with --disable-deprecated (the 
explanation makes sense to me, but I still think the argument above is 
valid).

Thus here my question:  What's the opinion on the matter?  I'd like to 
provide a simple patch for this issue (it is trivial, but hey :)), but 
want to know which way to go for it.  As stated above, my feeling would 
be to not remove the warning-issuing-methods on --disable-deprected.

Thanks,
Daniel

Re: Issuing deprecation warnings

[Andy Wingo]

Hi Daniel,

On Sun 28 Dec 2008 10:31, Daniel Kraft <d@domob.eu> writes:

> Thus here my question:  What's the opinion on the matter?  I'd like to
> provide a simple patch for this issue (it is trivial, but hey :)), but
> want to know which way to go for it.  As stated above, my feeling would
> be to not remove the warning-issuing-methods on --disable-deprected.

I agree. It is useful to have this API for extensions.

Andy
-- 
http://wingolog.org/

Re: Issuing deprecation warnings

[Daniel Kraft]

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

Andy Wingo wrote:
>> Thus here my question:  What's the opinion on the matter?  I'd like to
>> provide a simple patch for this issue (it is trivial, but hey :)), but
>> want to know which way to go for it.  As stated above, my feeling would
>> be to not remove the warning-issuing-methods on --disable-deprected.
> 
> I agree. It is useful to have this API for extensions.

Attached is an attempt to provide a patch for this; I hope I did get 
everything right.  The default value for GUILE_WARN_DEPRECATED when 
--disable-deprecated is set is probably something to dicuss, though.

Yours,
Daniel

[-- Attachment #2: patch.changelog --]
[-- Type: text/plain, Size: 374 bytes --]

2008-12-28  Daniel Kraft  <d@domob.eu>

	* configure.in (enable_deprecated): Set SCM_WARN_DEPRECATED_DEFAULT
	even when --disable-deprecated is passed.
	* libguile/deprecation.h: Declare deprecation-issuing methods even
	if SCM_ENABLE_DEPRECATED is not set.
	* libguile/deprecation.c: Ditto.
	(scm_init_deprecation): Include full body even for unset
	SCM_ENABLE_DEPRECATED.

[-- Attachment #3: patch.diff --]
[-- Type: text/plain, Size: 2684 bytes --]

diff --git a/configure.in b/configure.in
index a7fd9c9..385f4d9 100644
--- a/configure.in
+++ b/configure.in
@@ -159,6 +159,7 @@ AC_ARG_ENABLE([deprecated],
 
 if test "$enable_deprecated" = no; then
   SCM_I_GSC_ENABLE_DEPRECATED=0
+  warn_default=no
 else
   if test "$enable_deprecated" = yes || test "$enable_deprecated" = ""; then
     warn_default=summary
@@ -168,9 +169,9 @@ else
     warn_default=$enable_deprecated
   fi
   SCM_I_GSC_ENABLE_DEPRECATED=1
-  AC_DEFINE_UNQUOTED(SCM_WARN_DEPRECATED_DEFAULT, "$warn_default",
-  [Define this to control the default warning level for deprecated features.])
 fi
+AC_DEFINE_UNQUOTED(SCM_WARN_DEPRECATED_DEFAULT, "$warn_default",
+[Define this to control the default warning level for deprecated features.])
 
 AC_ARG_ENABLE(elisp,
   [  --disable-elisp         omit Emacs Lisp support],,
diff --git a/libguile/deprecation.c b/libguile/deprecation.c
index 338c47c..780e246 100644
--- a/libguile/deprecation.c
+++ b/libguile/deprecation.c
@@ -41,8 +41,6 @@
 
 \f
 
-#if (SCM_ENABLE_DEPRECATED == 1)
-
 struct issued_warning {
   struct issued_warning *prev;
   const char *message;
@@ -138,8 +136,6 @@ print_deprecation_summary (void)
     }
 }
 
-#endif /* SCM_ENABLE_DEPRECATED == 1 */
-
 SCM_DEFINE(scm_include_deprecated_features,
 	   "include-deprecated-features", 0, 0, 0,
 	   (),
@@ -157,7 +153,6 @@ SCM_DEFINE(scm_include_deprecated_features,
 void
 scm_init_deprecation ()
 {
-#if (SCM_ENABLE_DEPRECATED == 1)
   const char *level = getenv ("GUILE_WARN_DEPRECATED");
   if (level == NULL)
     level = SCM_WARN_DEPRECATED_DEFAULT;
@@ -170,7 +165,6 @@ scm_init_deprecation ()
       SCM_WARN_DEPRECATED = 0;
       atexit (print_deprecation_summary);
     }
-#endif
 #include "libguile/deprecation.x"
 }
 
diff --git a/libguile/deprecation.h b/libguile/deprecation.h
index 7885327..05928b7 100644
--- a/libguile/deprecation.h
+++ b/libguile/deprecation.h
@@ -26,20 +26,15 @@
 
 \f
 
-#if (SCM_ENABLE_DEPRECATED == 1)
-
-/* These functions are _not_ deprecated, but we exclude them along
-   with the really deprecated features to be sure that no-one is
-   trying to emit deprecation warnings when libguile is supposed to be
-   clean of them.
+/* These functions are a possibly useful part of the API and not only used
+   internally, thus they are exported always, not depending on
+   SCM_ENABLE_DEPRECATED.
 */
 
 SCM_API void scm_c_issue_deprecation_warning (const char *msg);
 SCM_API void scm_c_issue_deprecation_warning_fmt (const char *msg, ...);
 SCM_API SCM scm_issue_deprecation_warning (SCM msgs);
 
-#endif
-
 SCM_API SCM scm_include_deprecated_features (void);
 SCM_INTERNAL void scm_init_deprecation (void);
 

Re: Issuing deprecation warnings

[Ludovic Courtès]

Hello Daniel,

Daniel Kraft <d@domob.eu> writes:

> Andy Wingo wrote:
>>> Thus here my question:  What's the opinion on the matter?  I'd like to
>>> provide a simple patch for this issue (it is trivial, but hey :)), but
>>> want to know which way to go for it.  As stated above, my feeling would
>>> be to not remove the warning-issuing-methods on --disable-deprected.
>>
>> I agree. It is useful to have this API for extensions.
>
> Attached is an attempt to provide a patch for this; I hope I did get
> everything right.  The default value for GUILE_WARN_DEPRECATED when
> --disable-deprecated is set is probably something to dicuss, though.

Sorry for the loooong delay.  Your patch looks OK to me.  However, we'll
need you to assign copyright to the FSF before we can commit it (we can
work it out off-line).

Thanks,
Ludo'.