bogus 'unsafe variable' warning with try-else

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

bogus 'unsafe variable' warning with try-else

Mikael Pettersson-5
The following, adapted from an example in the online Erlang
Reference Manual, looks like a bug to me:

> cat bug.erl
-module(bug).
-compile(export_all).

termize_file(Name) ->
  {ok,F} = file:open(Name, [read,binary]),
  try
    {ok,Bin} = file:read(F, 1024*1024)
  after
    file:close(F)
  end,
  binary_to_term(Bin).
> erlc bug.erl
bug.erl:11: variable 'Bin' unsafe in 'try' (line 6)

Unless I'm missing something, there is no way to reach the
binary_to_term/1 call without successfully evaluating the
{ok,Bin} = ... match expression; therefore the use is not
unsafe.  If the file:read/2 call or the {ok,Bin} match fails,
the after clause runs and the entire try expression fails,
and the binary_to_term/1 is not reached.

Reproduced with 17.3 and r15b03-1.

Workaround: match on the value of the try itself:
"{ok,Bin} = try file:read(...) after ... end".

/Mikael
_______________________________________________
erlang-bugs mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-bugs
Reply | Threaded
Open this post in threaded view
|

Re: bogus 'unsafe variable' warning with try-else

dmkolesnikov
Hello,

This is very valid error.  
The variable Bin is defined within code block but it is used out-side of it.

I’ve never heard that any one complained about following error:

   try
   {
      int x = 1;
   } catch (Exception _) {
   }
   x++;

   error: cannot find symbol


Best Regards,
Dmitry


> On 20 Nov 2014, at 13:49, Mikael Pettersson <[hidden email]> wrote:
>
> The following, adapted from an example in the online Erlang
> Reference Manual, looks like a bug to me:
>
>> cat bug.erl
> -module(bug).
> -compile(export_all).
>
> termize_file(Name) ->
>  {ok,F} = file:open(Name, [read,binary]),
>  try
>    {ok,Bin} = file:read(F, 1024*1024)
>  after
>    file:close(F)
>  end,
>  binary_to_term(Bin).
>> erlc bug.erl
> bug.erl:11: variable 'Bin' unsafe in 'try' (line 6)
>
> Unless I'm missing something, there is no way to reach the
> binary_to_term/1 call without successfully evaluating the
> {ok,Bin} = ... match expression; therefore the use is not
> unsafe.  If the file:read/2 call or the {ok,Bin} match fails,
> the after clause runs and the entire try expression fails,
> and the binary_to_term/1 is not reached.
>
> Reproduced with 17.3 and r15b03-1.
>
> Workaround: match on the value of the try itself:
> "{ok,Bin} = try file:read(...) after ... end".
>
> /Mikael
> _______________________________________________
> erlang-bugs mailing list
> [hidden email]
> http://erlang.org/mailman/listinfo/erlang-bugs

_______________________________________________
erlang-bugs mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-bugs
Reply | Threaded
Open this post in threaded view
|

Re: bogus 'unsafe variable' warning with try-else

Kostis Sagonas-2
On 11/20/14 14:39, Dmitry Kolesnikov wrote:

> Hello,
>
> This is very valid error.
> The variable Bin is defined within code block but it is used out-side of it.
>
> I’ve never heard that any one complained about following error:
>
>     try
>     {
>        int x = 1;
>     } catch (Exception _) {
>     }
>     x++;
>
>     error: cannot find symbol

The issue has nothing to do with variables defined in blocks and used
outside.  If it had, then the following Erlang code would also not be
allowed, but it is:

termize_file(Name) ->
   {ok,F} = file:open(Name, [read,binary]),
   case file:read(F, 1024*1024) of
     {ok,Bin} -> ok
   end,
   binary_to_term(Bin).


I suggest you try to understand the (perhaps quite unorthodox) variable
scoping rules of Erlang, and of try-catch in particular, and think about
the differences between the code that was posted by Mikael (who is
right, of course) and the Java one you posted.

Kostis


>> >On 20 Nov 2014, at 13:49, Mikael Pettersson<[hidden email]>  wrote:
>> >
>> >The following, adapted from an example in the online Erlang
>> >Reference Manual, looks like a bug to me:
>> >
>>> >>cat bug.erl
>> >-module(bug).
>> >-compile(export_all).
>> >
>> >termize_file(Name) ->
>> >  {ok,F} =file:open(Name, [read,binary]),
>> >  try
>> >    {ok,Bin} =file:read(F, 1024*1024)
>> >  after
>> >    file:close(F)
>> >  end,
>> >  binary_to_term(Bin).
>>> >>erlc bug.erl
>> >bug.erl:11: variable 'Bin' unsafe in 'try' (line 6)
>> >
>> >Unless I'm missing something, there is no way to reach the
>> >binary_to_term/1 call without successfully evaluating the
>> >{ok,Bin} = ... match expression; therefore the use is not
>> >unsafe.  If thefile:read/2  call or the {ok,Bin} match fails,
>> >the after clause runs and the entire try expression fails,
>> >and the binary_to_term/1 is not reached.
>> >
>> >Reproduced with 17.3 and r15b03-1.
>> >
>> >Workaround: match on the value of the try itself:
>> >"{ok,Bin} = tryfile:read(...)  after ... end".
>> >
>> >/Mikael
>> >_______________________________________________
>> >erlang-bugs mailing list
>> >[hidden email]
>> >http://erlang.org/mailman/listinfo/erlang-bugs
> _______________________________________________
> erlang-bugs mailing list
> [hidden email]
> http://erlang.org/mailman/listinfo/erlang-bugs

_______________________________________________
erlang-bugs mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-bugs
Reply | Threaded
Open this post in threaded view
|

Re: bogus 'unsafe variable' warning with try-else

Mikael Pettersson-5
In reply to this post by dmkolesnikov
Dmitry Kolesnikov writes:
 > Hello,
 >
 > This is very valid error.  
 > The variable Bin is defined within code block but it is used out-side of it.
 >
 > I’ve never heard that any one complained about following error:
 >
 >    try
 >    {
 >       int x = 1;
 >    } catch (Exception _) {
 >    }
 >    x++;
 >
 >    error: cannot find symbol

Erlang has (very) different scope rules compared to C++.

 > > On 20 Nov 2014, at 13:49, Mikael Pettersson <[hidden email]> wrote:
 > >
 > > The following, adapted from an example in the online Erlang
 > > Reference Manual, looks like a bug to me:
 > >
 > >> cat bug.erl
 > > -module(bug).
 > > -compile(export_all).
 > >
 > > termize_file(Name) ->
 > >  {ok,F} = file:open(Name, [read,binary]),
 > >  try
 > >    {ok,Bin} = file:read(F, 1024*1024)
 > >  after
 > >    file:close(F)
 > >  end,
 > >  binary_to_term(Bin).
 > >> erlc bug.erl
 > > bug.erl:11: variable 'Bin' unsafe in 'try' (line 6)
 > >
 > > Unless I'm missing something, there is no way to reach the
 > > binary_to_term/1 call without successfully evaluating the
 > > {ok,Bin} = ... match expression; therefore the use is not
 > > unsafe.  If the file:read/2 call or the {ok,Bin} match fails,
 > > the after clause runs and the entire try expression fails,
 > > and the binary_to_term/1 is not reached.
 > >
 > > Reproduced with 17.3 and r15b03-1.
 > >
 > > Workaround: match on the value of the try itself:
 > > "{ok,Bin} = try file:read(...) after ... end".
 > >
 > > /Mikael
 > > _______________________________________________
 > > erlang-bugs mailing list
 > > [hidden email]
 > > http://erlang.org/mailman/listinfo/erlang-bugs

--
_______________________________________________
erlang-bugs mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-bugs
Reply | Threaded
Open this post in threaded view
|

Re: bogus 'unsafe variable' warning with try-else

Anthony Ramine-4
In reply to this post by Mikael Pettersson-5
try file:read(F, 1024 * 1024) of
    {ok,Bin} -> binary_to_term(Bin)
after
    file:close(F)
end


Le 20 nov. 2014 à 12:49, Mikael Pettersson <[hidden email]> a écrit :

> The following, adapted from an example in the online Erlang
> Reference Manual, looks like a bug to me:
>
>> cat bug.erl
> -module(bug).
> -compile(export_all).
>
> termize_file(Name) ->
>  {ok,F} = file:open(Name, [read,binary]),
>  try
>    {ok,Bin} = file:read(F, 1024*1024)
>  after
>    file:close(F)
>  end,
>  binary_to_term(Bin).
>> erlc bug.erl
> bug.erl:11: variable 'Bin' unsafe in 'try' (line 6)
>
> Unless I'm missing something, there is no way to reach the
> binary_to_term/1 call without successfully evaluating the
> {ok,Bin} = ... match expression; therefore the use is not
> unsafe.  If the file:read/2 call or the {ok,Bin} match fails,
> the after clause runs and the entire try expression fails,
> and the binary_to_term/1 is not reached.
>
> Reproduced with 17.3 and r15b03-1.
>
> Workaround: match on the value of the try itself:
> "{ok,Bin} = try file:read(...) after ... end".
>
> /Mikael
> _______________________________________________
> erlang-bugs mailing list
> [hidden email]
> http://erlang.org/mailman/listinfo/erlang-bugs

_______________________________________________
erlang-bugs mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-bugs
Reply | Threaded
Open this post in threaded view
|

Re: bogus 'unsafe variable' warning with try-else

Kostis Sagonas-2
On 11/20/14 15:13, Anthony Ramine wrote:
> try file:read(F, 1024 * 1024) of
>      {ok,Bin} -> binary_to_term(Bin)
> after
>      file:close(F)
> end


And the reason to _having_ to write this is?  (*)

Cheers,
Kostis

(*) Less bugs are exposed in the compiler and less baby cats die in
China if we all write like that, perhaps? :)

Jokes aside, I also agree the above code is nicer, but that does not
mean that the current warning is bogus and the analysis should be fixed.
_______________________________________________
erlang-bugs mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-bugs
Reply | Threaded
Open this post in threaded view
|

Re: bogus 'unsafe variable' warning with try-else

Pierre Fenoll-2
I guess the best option (and the one intended by OP) is to file:close/1 as soon as possible.

Thus Anthony's proposition would be unsuitable (though valid) and OP would need something like Kostis' case proposition.

I don't get why the scoping rules differ between try…after and case…of. I don't think they should (but I am surely missing an edge case where the difference makes sense).


Cheers,
-- 
Pierre Fenoll


On 20 November 2014 14:22, Kostis Sagonas <[hidden email]> wrote:
On 11/20/14 15:13, Anthony Ramine wrote:
try file:read(F, 1024 * 1024) of
     {ok,Bin} -> binary_to_term(Bin)
after
     file:close(F)
end


And the reason to _having_ to write this is?  (*)

Cheers,
Kostis

(*) Less bugs are exposed in the compiler and less baby cats die in China if we all write like that, perhaps? :)

Jokes aside, I also agree the above code is nicer, but that does not mean that the current warning is bogus and the analysis should be fixed.

_______________________________________________
erlang-bugs mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-bugs


_______________________________________________
erlang-bugs mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-bugs
Reply | Threaded
Open this post in threaded view
|

Re: bogus 'unsafe variable' warning with try-else

Joel Ericson
In reply to this post by Kostis Sagonas-2
The binding {ok,Bin} = file:read(F, 1024*1024) *can* fail. E.g., if I were to load in my own file library, where file:read/2 returns {data,Data}, this fails. And as far as I know, one is not supposed to assume anything about what other modules return during compile time.

Regards
Joel

On Thu, Nov 20, 2014 at 2:22 PM, Kostis Sagonas <[hidden email]> wrote:
On 11/20/14 15:13, Anthony Ramine wrote:
try file:read(F, 1024 * 1024) of
     {ok,Bin} -> binary_to_term(Bin)
after
     file:close(F)
end


And the reason to _having_ to write this is?  (*)

Cheers,
Kostis

(*) Less bugs are exposed in the compiler and less baby cats die in China if we all write like that, perhaps? :)

Jokes aside, I also agree the above code is nicer, but that does not mean that the current warning is bogus and the analysis should be fixed.

_______________________________________________
erlang-bugs mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-bugs


_______________________________________________
erlang-bugs mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-bugs
Reply | Threaded
Open this post in threaded view
|

Re: bogus 'unsafe variable' warning with try-else

Raimo Niskanen-6
On Thu, Nov 20, 2014 at 02:44:55PM +0100, Joel Ericson wrote:
> The binding {ok,Bin} = file:read(F, 1024*1024) *can* fail. E.g., if I were
> to load in my own file library, where file:read/2 returns {data,Data}, this

Yes, but since the try .. of .. after .. end construct does not catch any
exceptions there is no way to reach the code after it without having
succeeded with the variable binding.

> fails. And as far as I know, one is not supposed to assume anything about
> what other modules return during compile time.
>
> Regards
> Joel
>
> On Thu, Nov 20, 2014 at 2:22 PM, Kostis Sagonas <[hidden email]> wrote:
>
> > On 11/20/14 15:13, Anthony Ramine wrote:
> >
> >> try file:read(F, 1024 * 1024) of
> >>      {ok,Bin} -> binary_to_term(Bin)
> >> after
> >>      file:close(F)
> >> end
> >>
> >
> >
> > And the reason to _having_ to write this is?  (*)
> >
> > Cheers,
> > Kostis
> >
> > (*) Less bugs are exposed in the compiler and less baby cats die in China
> > if we all write like that, perhaps? :)
> >
> > Jokes aside, I also agree the above code is nicer, but that does not mean
> > that the current warning is bogus and the analysis should be fixed.
> >
> > _______________________________________________
> > erlang-bugs mailing list
> > [hidden email]
> > http://erlang.org/mailman/listinfo/erlang-bugs
> >

> _______________________________________________
> erlang-bugs mailing list
> [hidden email]
> http://erlang.org/mailman/listinfo/erlang-bugs


--

/ Raimo Niskanen, Erlang/OTP, Ericsson AB
_______________________________________________
erlang-bugs mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-bugs
Reply | Threaded
Open this post in threaded view
|

Re: bogus 'unsafe variable' warning with try-else

Raimo Niskanen-6
In reply to this post by Pierre Fenoll-2
On Thu, Nov 20, 2014 at 02:37:52PM +0100, Pierre Fenoll wrote:
> I guess the best option (and the one intended by OP) is to file:close/1 as
> soon as possible.
>
> Thus Anthony's proposition would be unsuitable (though valid) and OP would
> need something like Kostis' case proposition.
>
> I don't get why the scoping rules differ between try…after and case…of. I
> don't think they should (but I am surely missing an edge case where the
> difference makes sense).

Let's take a general try..end construct:

    try f()
    of
    {ok,A} ->
            B = {a,A};
        {error,A} ->
            C = {r,A}
    catch
        error:Error ->
            {error,Error};
        exit:Exit ->
            {exit,Exit}
    after
        G = {A,B,C,Error,Exit} % which bound?
    end,
    {A,B,C,Error,Exit,G} % which bound?

Now, which variables should be bound in the after..end section,
and which should be bound after the end?

It would be nice if A was safe somwhere after the of..catch
section just as for the of..end section in case constructs, but since we
might get an exception anywhere we might not have bound A neither when
reaching any clause in the catch..after section, nor the after..end
section and certainly not after the end.  The only safe binding could be
the variable G since the after..end section is always executed.

So variables from the after..end section could have been exported, but not
any other.  But in that special case below withount a catch..after section
the case construct rules could hold.  There might be other special cases.
But for the sake of consistency the simplest rule is that no variables are
exported from the try..end construct.

/ Raimo Niskanen

>
>
> Cheers,
> --
> Pierre Fenoll
>
>
> On 20 November 2014 14:22, Kostis Sagonas <[hidden email]> wrote:
>
> > On 11/20/14 15:13, Anthony Ramine wrote:
> >
> >> try file:read(F, 1024 * 1024) of
> >>      {ok,Bin} -> binary_to_term(Bin)
> >> after
> >>      file:close(F)
> >> end
> >>
> >
> >
> > And the reason to _having_ to write this is?  (*)
> >
> > Cheers,
> > Kostis
> >
> > (*) Less bugs are exposed in the compiler and less baby cats die in China
> > if we all write like that, perhaps? :)
> >
> > Jokes aside, I also agree the above code is nicer, but that does not mean
> > that the current warning is bogus and the analysis should be fixed.
> >
> > _______________________________________________
> > erlang-bugs mailing list
> > [hidden email]
> > http://erlang.org/mailman/listinfo/erlang-bugs
> >

> _______________________________________________
> erlang-bugs mailing list
> [hidden email]
> http://erlang.org/mailman/listinfo/erlang-bugs


--

/ Raimo Niskanen, Erlang/OTP, Ericsson AB
_______________________________________________
erlang-bugs mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-bugs