Hi, I just wanted to hand you a patch I did to add a new option to show deprecation messages as warnings (-di), following the discussion in the announcement group. I know I should probably get an account in github and do those pull requests everybody talks about, but it's too much trouble for an occasional collaboration, I hope you are willing to take a look at the patches and apply them if you like them (and if not, any feedback would be appreciated). I didn't do a lot of testing either, because is not trivial at all to set up a testing environment for the compiler, so testing would be appreciated too (I know is easy to run the test suite once you have a proper development environment set up). See the patch descriptions for more details.

I hope you find the patches useful.

Thanks.

PS: You can easily commit the patches using 'git am'.

-- 
Leandro Lucarella (AKA luca)                     http://llucax.com.ar/
----------------------------------------------------------------------
GPG Key: 5F5A8D05 (F8CD F9A7 BF00 5431 4145  104C 949E BFB6 5F5A 8D05)
----------------------------------------------------------------------
La m?quina de la moneda, mir? como te queda!
	-- Sidharta Kiwi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-option-to-show-deprecated-errors-as-warnings.patch
Type: text/x-diff
Size: 29054 bytes
Desc: not available
URL: <http://lists.puremagic.com/pipermail/dmd-internals/attachments/20110712/7ec21d96/attachment-0002.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Add-wi-to-the-man-page.patch
Type: text/x-diff
Size: 630 bytes
Desc: not available
URL: <http://lists.puremagic.com/pipermail/dmd-internals/attachments/20110712/7ec21d96/attachment-0003.patch>

Le 2011-07-12 ? 19:32, Leandro Lucarella a ?crit :

> Hi, I just wanted to hand you a patch I did to add a new option to show deprecation messages as warnings (-di), following the discussion in the announcement group. I know I should probably get an account in github and do those pull requests everybody talks about, but it's too much trouble for an occasional collaboration, I hope you are willing to take a look at the patches and apply them if you like them (and if not, any feedback would be appreciated). I didn't do a lot of testing either, because is not trivial at all to set up a testing environment for the compiler, so testing would be appreciated too (I know is easy to run the test suite once you have a proper development environment set up). See the patch descriptions for more details.
> 
> I hope you find the patches useful.

Turned them into pull requests.

<https://github.com/D-Programming-Language/dmd/pull/244> <https://github.com/D-Programming-Language/dmd/pull/245>

But then I had to close the pull request about deprecation warnings.

Not only it fails the test suite (I should have checked sooner), but for some strange reasons it generates errors while compiling druntime, and those errors aren't fatal as they should and thus don't prevent the build from being successful. They're also pretty strange considering the nature of the changes:

src/core/thread.d(3723): Error: undefined identifier ucontext_t
src/core/thread.d(2845): Error: undefined identifier GetSystemInfo
src/core/thread.d(2854): Error: undefined identifier _SC_PAGESIZE
src/core/thread.d(2895): Error: undefined identifier ucontext_t
src/core/thread.d(3199): Error: undefined identifier ucontext_t
src/core/thread.d(3206): Error: undefined identifier ucontext_t
src/core/thread.d(3302): Error: undefined identifier ucontext_t
src/core/thread.d(3332): Error: undefined identifier ucontext_t
src/core/thread.d(3370): Error: undefined identifier ucontext_t
src/core/thread.d(3464): Error: undefined identifier VirtualAlloc
src/core/thread.d(3574): Error: undefined identifier VirtualAlloc



-- 
Michel Fortin
michel.fortin at michelf.com
http://michelf.com/

Michel Fortin, el 12 de julio a las 21:40 me escribiste:
> Le 2011-07-12 ? 19:32, Leandro Lucarella a ?crit :
>
> > Hi, I just wanted to hand you a patch I did to add a new option to show deprecation messages as warnings (-di), following the discussion in the announcement group. I know I should probably get an account in github and do those pull requests everybody talks about, but it's too much trouble for an occasional collaboration, I hope you are willing to take a look at the patches and apply them if you like them (and if not, any feedback would be appreciated). I didn't do a lot of testing either, because is not trivial at all to set up a testing environment for the compiler, so testing would be appreciated too (I know is easy to run the test suite once you have a proper development environment set up). See the patch descriptions for more details.
> >
> > I hope you find the patches useful.
>
> Turned them into pull requests.
>
> <https://github.com/D-Programming-Language/dmd/pull/244> <https://github.com/D-Programming-Language/dmd/pull/245>

Thanks for that.

> But then I had to close the pull request about deprecation warnings.
>
> Not only it fails the test suite (I should have checked sooner), but for some strange reasons it generates errors while compiling druntime, and those errors aren't fatal as they should and thus don't prevent the build from being successful. They're also pretty strange considering the nature of the changes:
>
> src/core/thread.d(3723): Error: undefined identifier ucontext_t
> src/core/thread.d(2845): Error: undefined identifier GetSystemInfo
> src/core/thread.d(2854): Error: undefined identifier _SC_PAGESIZE
> src/core/thread.d(2895): Error: undefined identifier ucontext_t
> src/core/thread.d(3199): Error: undefined identifier ucontext_t
> src/core/thread.d(3206): Error: undefined identifier ucontext_t
> src/core/thread.d(3302): Error: undefined identifier ucontext_t
> src/core/thread.d(3332): Error: undefined identifier ucontext_t
> src/core/thread.d(3370): Error: undefined identifier ucontext_t
> src/core/thread.d(3464): Error: undefined identifier VirtualAlloc
> src/core/thread.d(3574): Error: undefined identifier VirtualAlloc

And thanks for the testing too. That happened using -di, -d or neither?

I thought it might be that when a symbol is deprecated it might not be added to the symbol table, but I can't find any place where that happens, and all those symbols doesn't look like they are deprecated anyway, right?

-- 
Leandro Lucarella (AKA luca)                     http://llucax.com.ar/
----------------------------------------------------------------------
GPG Key: 5F5A8D05 (F8CD F9A7 BF00 5431 4145  104C 949E BFB6 5F5A 8D05)
----------------------------------------------------------------------
I am so psychosomatic it makes me sick just thinking about it!
        -- George Constanza

Michel Fortin, el 12 de julio a las 21:40 me escribiste:
> Le 2011-07-12 ? 19:32, Leandro Lucarella a ?crit :
> 
> > Hi, I just wanted to hand you a patch I did to add a new option to show deprecation messages as warnings (-di), following the discussion in the announcement group. I know I should probably get an account in github and do those pull requests everybody talks about, but it's too much trouble for an occasional collaboration, I hope you are willing to take a look at the patches and apply them if you like them (and if not, any feedback would be appreciated). I didn't do a lot of testing either, because is not trivial at all to set up a testing environment for the compiler, so testing would be appreciated too (I know is easy to run the test suite once you have a proper development environment set up). See the patch descriptions for more details.
> > 
> > I hope you find the patches useful.
> 
> Turned them into pull requests.
> 
> <https://github.com/D-Programming-Language/dmd/pull/244> <https://github.com/D-Programming-Language/dmd/pull/245>
> 
> But then I had to close the pull request about deprecation warnings.
> 
> Not only it fails the test suite (I should have checked sooner), but for some strange reasons it generates errors while compiling druntime, and those errors aren't fatal as they should and thus don't prevent the build from being successful. They're also pretty strange considering the nature of the changes:
> 
> src/core/thread.d(3723): Error: undefined identifier ucontext_t
> src/core/thread.d(2845): Error: undefined identifier GetSystemInfo
> src/core/thread.d(2854): Error: undefined identifier _SC_PAGESIZE
> src/core/thread.d(2895): Error: undefined identifier ucontext_t
> src/core/thread.d(3199): Error: undefined identifier ucontext_t
> src/core/thread.d(3206): Error: undefined identifier ucontext_t
> src/core/thread.d(3302): Error: undefined identifier ucontext_t
> src/core/thread.d(3332): Error: undefined identifier ucontext_t
> src/core/thread.d(3370): Error: undefined identifier ucontext_t
> src/core/thread.d(3464): Error: undefined identifier VirtualAlloc
> src/core/thread.d(3574): Error: undefined identifier VirtualAlloc

OK, all that errors have an static if (__tratits(compiles, symbol))

I'll try to figure out what's making the mess with __traits(compiles)...

-- 
Leandro Lucarella (AKA luca)                     http://llucax.com.ar/
----------------------------------------------------------------------
GPG Key: 5F5A8D05 (F8CD F9A7 BF00 5431 4145  104C 949E BFB6 5F5A 8D05)
----------------------------------------------------------------------
cederr?n.
	(De CD-ROM, y este sigla del ingl. Compact Disc Read-Only Memory).
	1. m. Inform. CD-ROM.
	-- Real Academia Espa?ola

Leandro Lucarella, el 13 de julio a las 01:16 me escribiste:
> Michel Fortin, el 12 de julio a las 21:40 me escribiste:
> > Le 2011-07-12 ? 19:32, Leandro Lucarella a ?crit :
> > 
> > > Hi, I just wanted to hand you a patch I did to add a new option to show deprecation messages as warnings (-di), following the discussion in the announcement group. I know I should probably get an account in github and do those pull requests everybody talks about, but it's too much trouble for an occasional collaboration, I hope you are willing to take a look at the patches and apply them if you like them (and if not, any feedback would be appreciated). I didn't do a lot of testing either, because is not trivial at all to set up a testing environment for the compiler, so testing would be appreciated too (I know is easy to run the test suite once you have a proper development environment set up). See the patch descriptions for more details.
> > > 
> > > I hope you find the patches useful.
> > 
> > Turned them into pull requests.
> > 
> > <https://github.com/D-Programming-Language/dmd/pull/244> <https://github.com/D-Programming-Language/dmd/pull/245>
> > 
> > But then I had to close the pull request about deprecation warnings.
> > 
> > Not only it fails the test suite (I should have checked sooner), but for some strange reasons it generates errors while compiling druntime, and those errors aren't fatal as they should and thus don't prevent the build from being successful. They're also pretty strange considering the nature of the changes:
> > 
> > src/core/thread.d(3723): Error: undefined identifier ucontext_t
> > src/core/thread.d(2845): Error: undefined identifier GetSystemInfo
> > src/core/thread.d(2854): Error: undefined identifier _SC_PAGESIZE
> > src/core/thread.d(2895): Error: undefined identifier ucontext_t
> > src/core/thread.d(3199): Error: undefined identifier ucontext_t
> > src/core/thread.d(3206): Error: undefined identifier ucontext_t
> > src/core/thread.d(3302): Error: undefined identifier ucontext_t
> > src/core/thread.d(3332): Error: undefined identifier ucontext_t
> > src/core/thread.d(3370): Error: undefined identifier ucontext_t
> > src/core/thread.d(3464): Error: undefined identifier VirtualAlloc
> > src/core/thread.d(3574): Error: undefined identifier VirtualAlloc
> 
> OK, all that errors have an static if (__tratits(compiles, symbol))
> 
> I'll try to figure out what's making the mess with __traits(compiles)...

OK, I download and compiled druntime with all -d, -di and none and couldn't reproduce this problem. I got another errors though:

src/core/runtime.d(310): Error: function core.runtime.backtrace (void**, uint) is not callable using argument types (void*[128u],int)
src/core/runtime.d(310): Error: cannot implicitly convert expression (callstack) of type void*[128u] to void**
src/core/runtime.d(311): Error: function core.runtime.backtrace_symbols_fd (void**, int, int) is not callable
using argument types (void*[128u],int,int)
src/core/runtime.d(311): Error: cannot implicitly convert expression (callstack) of type void*[128u] to void**

I fixed them by changing just one line in the patch:

--- a/src/mtype.c
+++ b/src/mtype.c
@@ -56,7 +56,7 @@ FuncDeclaration *hasThis(Scope *sc);
 #define LOGDEFAULTINIT  0       // log ::defaultInit()

 // Allow implicit conversion of T[] to T*
-#define IMPLICIT_ARRAY_TO_PTR   (!global.params.deprecation)
+#define IMPLICIT_ARRAY_TO_PTR   (global.params.deprecation != 1)

 /* These have default values for 32 bit code, they get
  * adjusted for 64 bit code.

The problem is, this type of deprecation can't show a warning, I guess, because it just add or removes functions as possible candidates, I tried to make it show a sensible warning but couldn't find a way without getting a lot of spurious warnings, maybe someone who knows better can fix this though. Worst case, we have to decide if -di should die with an error of an unmatched function or if it should pass silently (what I did in this new patch).

Also I fixed another small bug that could prevent some warnings to be shown:

--- a/src/mars.c
+++ b/src/mars.c
@@ -159,10 +159,7 @@ void deprecation(Loc loc, const char *format, ...)
 {
     va_list ap;
     va_start(ap, format);
-    if (global.params.deprecation > 1)
-        vwarning(loc, format, ap);
-    else
-        verror(loc, format, ap);
+    vdeprecation(loc, format, ap);
     va_end( ap );
 }

Attached is the updated patch. If you can try it, it would be appreciated, and
if it's still failing, I would appreciate if you can give me detailed steps to
reproduce it.

Thanks!

-- 
Leandro Lucarella (AKA luca)                     http://llucax.com.ar/
----------------------------------------------------------------------
GPG Key: 5F5A8D05 (F8CD F9A7 BF00 5431 4145  104C 949E BFB6 5F5A 8D05)
----------------------------------------------------------------------
Every day 21 new born babies will be given to the wrong parents
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-option-to-show-deprecated-errors-as-warnings.patch
Type: text/x-diff
Size: 28977 bytes
Desc: not available
URL: <http://lists.puremagic.com/pipermail/dmd-internals/attachments/20110713/a178c31e/attachment-0001.patch>

Le 2011-07-13 ? 13:00, Leandro Lucarella a ?crit :

> OK, I download and compiled druntime with all -d, -di and none and couldn't reproduce this problem.

Today I rebased yesterday's changes on the latest master instead of the yesterday's master, then I updated druntime to the latest master too, then all went well, compiling druntime, phobos and the dmd test suite.

I'm not sure what went wrong yesterday. I'm pretty sure I did test both with and without your patch to make sure the master wasn't the problem, but perhaps I just did that wrong.

> [...]
> Attached is the updated patch. If you can try it, it would be appreciated, and
> if it's still failing, I would appreciate if you can give me detailed steps to
> reproduce it.

This new patch seems to work fine too.

Here is a new pull request: <https://github.com/D-Programming-Language/dmd/pull/248>


-- 
Michel Fortin
michel.fortin at michelf.com
http://michelf.com/

Michel Fortin, el 13 de julio a las 15:54 me escribiste:
> Le 2011-07-13 ? 13:00, Leandro Lucarella a ?crit :
> 
> > OK, I download and compiled druntime with all -d, -di and none and couldn't reproduce this problem.
> 
> Today I rebased yesterday's changes on the latest master instead of the yesterday's master, then I updated druntime to the latest master too, then all went well, compiling druntime, phobos and the dmd test suite.
> 
> I'm not sure what went wrong yesterday. I'm pretty sure I did test both with and without your patch to make sure the master wasn't the problem, but perhaps I just did that wrong.
> 
> > [...]
> > Attached is the updated patch. If you can try it, it would be appreciated, and
> > if it's still failing, I would appreciate if you can give me detailed steps to
> > reproduce it.
> 
> This new patch seems to work fine too.
> 
> Here is a new pull request: <https://github.com/D-Programming-Language/dmd/pull/248>

Thanks! Here are a couple new patches that tidy up things a little further. In the way, I noticed there were some features what were reported as deprecated, but were really plain errors, since they were issued when this condition is met: 1 || !global.params.useDeprecated, i.e. always :)

In this patch I thread that as plain errors (and change "deprecated"
with "no longer legal", as it says with other removed features). If we
really want a deprecation message there, is trivial to do so (change
error() with deprecation()).

If you can include this one too, it would be awesome! =)

Thanks!

-- 
Leandro Lucarella (AKA luca)                     http://llucax.com.ar/
----------------------------------------------------------------------
GPG Key: 5F5A8D05 (F8CD F9A7 BF00 5431 4145  104C 949E BFB6 5F5A 8D05)
----------------------------------------------------------------------
El otro d?a ten?a un plan
Pero despu?s me olvid? y me com? un flan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Make-deprecated-smarter.patch
Type: text/x-diff
Size: 13700 bytes
Desc: not available
URL: <http://lists.puremagic.com/pipermail/dmd-internals/attachments/20110713/d43be5e6/attachment.patch>

Le 2011-07-13 ? 16:39, Leandro Lucarella a ?crit :

> Michel Fortin, el 13 de julio a las 15:54 me escribiste:
>> Here is a new pull request: <https://github.com/D-Programming-Language/dmd/pull/248>
> 
> Thanks! Here are a couple new patches that tidy up things a little further. In the way, I noticed there were some features what were reported as deprecated, but were really plain errors, since they were issued when this condition is met: 1 || !global.params.useDeprecated, i.e. always :)
> 
> In this patch I thread that as plain errors (and change "deprecated"
> with "no longer legal", as it says with other removed features). If we
> really want a deprecation message there, is trivial to do so (change
> error() with deprecation()).
> 
> If you can include this one too, it would be awesome! =)

Included.

-- 
Michel Fortin
michel.fortin at michelf.com
http://michelf.com/