Discussion:
[fpc-devel] Wrong docs: not initialized global variables
Ondrej Pokorny
2018-03-24 19:49:16 UTC
Permalink
Due to the "Multiple variable initialization" thread I took a look into
FPC documentation: https://www.freepascal.org/docs-html/ref/refse24.html

There it says:

By default, simple variables in Pascal are not initialized after their
declaration. Any assumption that they contain 0 or any other default
value is erroneous: They can contain rubbish.

This is not correct. Global simple variables are always initialized. At
least in Delphi it is so:
http://docwiki.embarcadero.com/RADStudio/Tokyo/en/Variables_(Delphi) "If
you do not explicitly initialize a global variable, the compiler
initializes it to 0."

-----

Another question about this sentence in FPC docs: "Managed types are
always initialized: in general this means setting the reference count to
zero, or setting the pointer value of the type to Nil."

Does it mean I can assume a local string variable is always (=whenever
the routine is called) initialized to ''? I.e. that TestB always returns
'a'?

function TestB: string;
var B: string;
begin
  B := B + 'a';
  Result := B;
end;

Until now I though that the above code is unsafe and I always
initialized all local variables (also managed) before use. I assumed the
initialization to empty string does not need to happen at every routine
call.

Ondrej

_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/m
Sven Barth via fpc-devel
2018-03-24 22:00:38 UTC
Permalink
Ondrej Pokorny <***@kluug.net> schrieb am Sa., 24. MÀrz 2018, 20:49:

> This is not correct. Global simple variables are always initialized. At
> least in Delphi it is so:
> http://docwiki.embarcadero.com/RADStudio/Tokyo/en/Variables_(Delphi) "If
> you do not explicitly initialize a global variable, the compiler
> initializes it to 0."
>

So, Delphi *does* document it. Okay, in that case the documentation needs
to be updated, cause up to now the assumption has been that this is an
implementation detail.

-----
>
> Another question about this sentence in FPC docs: "Managed types are
> always initialized: in general this means setting the reference count to
> zero, or setting the pointer value of the type to Nil."
>
> Does it mean I can assume a local string variable is always (=whenever
> the routine is called) initialized to ''? I.e. that TestB always returns
> 'a'?
>

For managed types this is indeed guaranteed. Otherwise there'd be
exceptions upon the first use of the variable as the RTL would assume that
the value is valid.

Regards,
Sven
Ondrej Pokorny
2018-03-24 22:33:38 UTC
Permalink
On 24.03.2018 23:00, Sven Barth via fpc-devel wrote:
> Ondrej Pokorny <***@kluug.net <mailto:***@kluug.net>> schrieb
> am Sa., 24. MÀrz 2018, 20:49:
>
> This is not correct. Global simple variables are always
> initialized. At
> least in Delphi it is so:
> http://docwiki.embarcadero.com/RADStudio/Tokyo/en/Variables_(Delphi)
> <http://docwiki.embarcadero.com/RADStudio/Tokyo/en/Variables_%28Delphi%29>
> "If
> you do not explicitly initialize a global variable, the compiler
> initializes it to 0."
>
>
> So, Delphi *does* document it. Okay, in that case the documentation
> needs to be updated, cause up to now the assumption has been that this
> is an implementation detail.

It has always been so in Delphi - at least since I learnt it. The Delphi
7 docs (I can't find older docs online) state it as well:
http://docs.embarcadero.com/products/rad_studio/cbuilder6/EN/CB6_ObjPascalLangGuide_EN.pdf
on page 5-38 "If you don’t explicitly initialize a global variable, the
compiler initializes it to 0." (Docs are at
http://docs.embarcadero.com/products/rad_studio/ )

> Another question about this sentence in FPC docs: "Managed types are
> always initialized: in general this means setting the reference
> count to
> zero, or setting the pointer value of the type to Nil."
>
> Does it mean I can assume a local string variable is always (=whenever
> the routine is called) initialized to ''? I.e. that TestB always
> returns
> 'a'?
>
>
> For managed types this is indeed guaranteed. Otherwise there'd be
> exceptions upon the first use of the variable as the RTL would assume
> that the value is valid.

OK, thanks. My assumption was that setting local managed types to nil is
guaranteed only once at the first routine call. And that it is an
implementation detail if it is niled at the second call or reused from
the first call.

I probably mixed it up with the Result variable (what Maciej wrote about).

Ondrej
Ondrej Pokorny
2018-03-24 23:09:46 UTC
Permalink
On 24.03.2018 23:33, Ondrej Pokorny wrote:
>>
>> Another question about this sentence in FPC docs: "Managed types are
>> always initialized: in general this means setting the reference
>> count to
>> zero, or setting the pointer value of the type to Nil."
>>
>> Does it mean I can assume a local string variable is always
>> (=whenever
>> the routine is called) initialized to ''? I.e. that TestB always
>> returns
>> 'a'?
>>
>>
>> For managed types this is indeed guaranteed. Otherwise there'd be
>> exceptions upon the first use of the variable as the RTL would assume
>> that the value is valid.
>
> OK, thanks. My assumption was that setting local managed types to nil
> is guaranteed only once at the first routine call. And that it is an
> implementation detail if it is niled at the second call or reused from
> the first call.
>
> I probably mixed it up with the Result variable (what Maciej wrote about).

I found the source of my assumption, I get a compiler warning in this case:

program StringsTest;
function Test: string;
var S: string;
begin
  S := S + 'a';
  Result := S;
end;
begin
  Writeln(Test);
end.

Emits:
StringsTest.lpr(5,8) Warning: Local variable "S" of a managed type does
not seem to be initialized

Why do I get the compiler warning when the variable is indeed guaranteed
to be initialized to nil/empty string?

Ondrej
Jonas Maebe
2018-03-25 14:28:22 UTC
Permalink
On 2018-03-25 00:09, Ondrej Pokorny wrote:
> I found the source of my assumption, I get a compiler warning in this
> case:
>
> program StringsTest;
> function Test: string;
> var S: string;
> begin
> S := S + 'a';
> Result := S;
> end;
> begin
> Writeln(Test);
> end.
>
> Emits:
> StringsTest.lpr(5,8) Warning: Local variable "S" of a managed type
> does not seem to be initialized
>
> Why do I get the compiler warning when the variable is indeed
> guaranteed to be initialized to nil/empty string?

https://bugs.freepascal.org/view.php?id=24601


Jonas
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freep
Ondrej Pokorny
2018-04-04 08:47:53 UTC
Permalink
On 25.03.2018 16:28, Jonas Maebe wrote:
> On 2018-03-25 00:09, Ondrej Pokorny wrote:
>> Why do I get the compiler warning when the variable is indeed
>> guaranteed to be initialized to nil/empty string?
>
> https://bugs.freepascal.org/view.php?id=24601

Very good bug report.

I would lower the level of notification for "trivial: string" to a hint.
It is nonsense to emit the same level of notification (warning) for
dangerous code ("serious: integer") and valid code ("trivial: string").

Warnings are something to care about, the "trivial: string" case is not.

Ondrej
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://l
Jonas Maebe
2018-04-04 15:49:01 UTC
Permalink
On 04/04/18 10:47, Ondrej Pokorny wrote:
> I would lower the level of notification for "trivial: string" to a hint.
> It is nonsense to emit the same level of notification (warning) for
> dangerous code ("serious: integer") and valid code ("trivial: string").
>
> Warnings are something to care about, the "trivial: string" case is not.

The warnings for managed types have a different ID now. Hence, you can
already disable them separately if you want.


Jonas
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-de
Ondrej Pokorny
2018-04-04 16:44:34 UTC
Permalink
On 04.04.2018 17:49, Jonas Maebe wrote:
> On 04/04/18 10:47, Ondrej Pokorny wrote:
>> I would lower the level of notification for "trivial: string" to a
>> hint. It is nonsense to emit the same level of notification (warning)
>> for dangerous code ("serious: integer") and valid code ("trivial:
>> string").
>>
>> Warnings are something to care about, the "trivial: string" case is not.
>
> The warnings for managed types have a different ID now. Hence, you can
> already disable them separately if you want.

Please do not mix different things. I know that - and yes I read the bug
report you kindly provided a link to, where this was discussed. I did
not state I cannot disable them separately.

I want to stress that the compiler emits a warning on code that does not
have (and also cannot have) an error - even on other platforms and
targets. A warning is a level too high. A hint or maybe even a note is
more appropriate here:
https://www.freepascal.org/docs-html/user/usersu13.html#x36-430005.1.2

E.g. The Lazarus/LCL code may not emit any warnings (we fix them) but we
don't care much about hints and notes. Because of this warning I have to
change a perfectly defined and flawless code.

Ondrej
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listin
Jonas Maebe
2018-04-04 16:53:13 UTC
Permalink
On 04/04/18 18:44, Ondrej Pokorny wrote:
> I want to stress that the compiler emits a warning on code that does not
> have (and also cannot have) an error

An error is wrong code behaviour. If you do not initialise a variable
with the correct value, then you can have an error. This correct value
can be different from "empty string" or "nil".

For the same reason, we also warn about uninitialised global variables
(if this can be detected without interprocedural analysis, i.e., if they
are only used in the main program code; but that is merely due to a
limitation of the analysis).


Jonas
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mai
Ondrej Pokorny
2018-04-04 17:32:09 UTC
Permalink
On 04.04.2018 18:53, Jonas Maebe wrote:
> On 04/04/18 18:44, Ondrej Pokorny wrote:
>> I want to stress that the compiler emits a warning on code that does
>> not have (and also cannot have) an error
>
> An error is wrong code behaviour. If you do not initialise a variable
> with the correct value, then you can have an error.

No, no, no and again no. Do you try to convince me that I cannot rely on
well documented compiler behavior? Even Sven stated earlier in this
thread that the FPC docs are correct in case of managed variables:
"Managed types are an exception to this rule: Managed types are always
initialized: in general this means setting the reference count to zero,
or setting the pointer value of the type to Nil." and "It should be
stressed that initialized variables are initialized when they come into
scope" https://www.freepascal.org/docs-html/ref/refse24.html

Again, the code
procedure Test;
var S: string; I: Integer;
begin
  for I := 0 to 1 do
    S := S + '1';
  Writeln(S);
end;

does not have an error, cannot have an error and the behavior is
guaranteed and well documented => no warning should be here.

+ Do you try to convince me that the code below can have an error and
should emit a warning as well because it does not initialize the FS and
FI fields?

program Project1;
type
  TMyObject = class
  private
    FS: string;
    FI: Integer;
  public
    property S: string read FS write FS;
    property I: Integer read FI write FI;
  end;

begin
  with TMyObject.Create do
  begin
    Writeln(S);
    Writeln(I);
    Free;
  end;
end.

It's the same case - object fields are documented to be initialized. You
mean I should not rely on this feature and it is an error that I did not
initialize them with the correct values?

> This correct value can be different from "empty string" or "nil".

If I want to have a different value from "empty string" or "nil" I know
I have to initialize it by myself. I don't need a warning for it.

> For the same reason, we also warn about uninitialised global variables
> (if this can be detected without interprocedural analysis, i.e., if
> they are only used in the main program code; but that is merely due to
> a limitation of the analysis).

Uninitialised global variables are the same case: the compiler should
not emit a warning here because again, it is well documented that they
are implicitely initialized (at least in the Object Pascal Language
Guide from Borland from 2002 it is documented).

Ondrej
Jonas Maebe
2018-04-04 18:03:28 UTC
Permalink
On 04/04/18 19:32, Ondrej Pokorny wrote:
> On 04.04.2018 18:53, Jonas Maebe wrote:
>> On 04/04/18 18:44, Ondrej Pokorny wrote:
>>> I want to stress that the compiler emits a warning on code that does
>>> not have (and also cannot have) an error
>>
>> An error is wrong code behaviour. If you do not initialise a variable
>> with the correct value, then you can have an error.
>
> No, no, no and again no. Do you try to convince me that I cannot rely on
> well documented compiler behavior?

No. You replied to the above sentence out of context. The context was
given in the next sentence:

>> This correct value can be different from "empty string" or "nil".
>
> If I want to have a different value from "empty string" or "nil" I know
> I have to initialize it by myself. I don't need a warning for it.

Then turn it off.


Jonas
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/c
Ondrej Pokorny
2018-04-04 18:26:29 UTC
Permalink
On 04.04.2018 20:03, Jonas Maebe wrote:
> On 04/04/18 19:32, Ondrej Pokorny wrote:
>> On 04.04.2018 18:53, Jonas Maebe wrote:
>>> On 04/04/18 18:44, Ondrej Pokorny wrote:
>>>> I want to stress that the compiler emits a warning on code that
>>>> does not have (and also cannot have) an error
>>>
>>> An error is wrong code behaviour. If you do not initialise a
>>> variable with the correct value, then you can have an error.
>>
>> No, no, no and again no. Do you try to convince me that I cannot rely
>> on well documented compiler behavior?
>
> No. You replied to the above sentence out of context. The context was
> given in the next sentence:
>
>>> This correct value can be different from "empty string" or "nil".

No, I replied perfectly within the context. Read again:

Myself: "I want to stress that the compiler emits a warning on code that
does not have (and also cannot have) an error".
You: "An error is wrong code behaviour. If you do not initialise a
variable with the correct value, then you can have an error. This
correct value can be different from "empty string" or "nil"."

The compiler initializes the variable implicitely for myself - this is
documented and I know it. There cannot be "wrong code behaviour" (as you
stated) and thus I cannot have an error. This is simple logic.

With your logic I should get a warning all over the code. Even after an
assignment:

I := 5;

The compiler may think that 5 is not the correct value I want there?
Maybe it should warn me that I wanted a 6 there? The warning could
state: "An error is wrong code behaviour. If you do not initialise a
variable with the correct value, then you can have an error. This
correct value can be different from "5"."

>> If I want to have a different value from "empty string" or "nil" I
>> know I have to initialize it by myself. I don't need a warning for it.
>
> Then turn it off.

I must not. Turning off warnings is forbidden. Turning off hints and
notes is OK. That is the rules.

Please do not get me wrong - I am perfectly fine that the compiler emits
a message in this case. But I am not fine with the fact that the level
of this message is a WARNING - the same level that real code errors are
reported.

Ondrej
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-deve
Jonas Maebe
2018-04-04 19:51:10 UTC
Permalink
On 04/04/18 20:26, Ondrej Pokorny wrote:
> The compiler initializes the variable implicitely for myself - this is
> documented and I know it. There cannot be "wrong code behaviour" (as you
> stated) and thus I cannot have an error. This is simple logic.

"Wrong behaviour" is not the same as "undefined behaviour" or "crash".
It merely means "something different than the programmer intended". It
is of course possible that you wanted the behaviour that you get when
the variable contains the empty string, nil or 0, but because this
initialisation is implicit rather than explicit, the compiler does not
rely on this. After all, it is equally possible that you forgot to
assign the correct (non-nil/0/empty string) value to the variable before
using it the first time (in which case it would be an error in the program).

> I must not. Turning off warnings is forbidden. Turning off hints and notes is OK. That is the rules.

The inflexibility of that rule in the Lazarus project is not a good
reason to change the priority of the message.

> But I am not fine with the fact that the level of this message is a WARNING - the same level that real code errors are reported.

Warnings are not errors. Warnings indicate possible errors, but they can
also be false positives.


Jonas
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/cg
Ondrej Pokorny
2018-04-04 22:20:01 UTC
Permalink
On 04.04.2018 21:51, Jonas Maebe wrote:
> On 04/04/18 20:26, Ondrej Pokorny wrote:
>> The compiler initializes the variable implicitely for myself - this
>> is documented and I know it. There cannot be "wrong code behaviour"
>> (as you stated) and thus I cannot have an error. This is simple logic.
>
> "Wrong behaviour" is not the same as "undefined behaviour" or "crash".
> It merely means "something different than the programmer intended". It
> is of course possible that you wanted the behaviour that you get when
> the variable contains the empty string, nil or 0, but because this
> initialisation is implicit rather than explicit, the compiler does not
> rely on this. After all, it is equally possible that you forgot to
> assign the correct (non-nil/0/empty string) value to the variable
> before using it the first time (in which case it would be an error in
> the program).

How can I forget to assign the correct (non-nil/0/empty string) value to
a variable? Oh god, this is such a crazy argument - completely out of
the reality. At latest the first execution of the program tells me about
this error. Warnings should warn me about possible errors that I cannot
easily find and that are potentially dangerous.

No, this discussion is useless.

>> I must not. Turning off warnings is forbidden. Turning off hints and
>> notes is OK. That is the rules.
>
> The inflexibility of that rule in the Lazarus project is not a good
> reason to change the priority of the message.

...and the difference between a "truly uninitialized variable" and an
"implicitely initialized variable" is not a reason either...

Funnily enough there are both message levels (warning and hint) for all
variants of "not explicitely initialized variable". So somebody decided
when a hint and when a warning is emitted and you seem to be convinced
this decision is engraved in stone.

Even more funny - in
https://bugs.freepascal.org/view.php?id=24601#c75617 you used the same
tactics as now - you explain in a general way what the current behavior
is and ignore arguments for a possible change to make the behavior
clearer. Later, Florian changed it himself.

Ondrej
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-d
Michael Van Canneyt
2018-04-04 22:34:39 UTC
Permalink
On Thu, 5 Apr 2018, Ondrej Pokorny wrote:

> On 04.04.2018 21:51, Jonas Maebe wrote:
>> On 04/04/18 20:26, Ondrej Pokorny wrote:
>>> The compiler initializes the variable implicitely for myself - this
>>> is documented and I know it. There cannot be "wrong code behaviour"
>>> (as you stated) and thus I cannot have an error. This is simple logic.
>>
>> "Wrong behaviour" is not the same as "undefined behaviour" or "crash".
>> It merely means "something different than the programmer intended". It
>> is of course possible that you wanted the behaviour that you get when
>> the variable contains the empty string, nil or 0, but because this
>> initialisation is implicit rather than explicit, the compiler does not
>> rely on this. After all, it is equally possible that you forgot to
>> assign the correct (non-nil/0/empty string) value to the variable
>> before using it the first time (in which case it would be an error in
>> the program).
>
> How can I forget to assign the correct (non-nil/0/empty string) value to
> a variable? Oh god, this is such a crazy argument - completely out of
> the reality. At latest the first execution of the program tells me about
> this error. Warnings should warn me about possible errors that I cannot
> easily find and that are potentially dangerous.
>
> No, this discussion is useless.

What, because you do not agree ?

Pascal states that variables should be considered uninitialized.

So, the compiler tells you that you didn't initialize a variable before using
it.

Whether this variable is managed or not is irrelevant to the discussion.

Now, that the compiler initializes a managed type with a well-known value is
only relevant for its internal bookkeeping, but this says nothing about the
awareness of the programmer of this fact.

So it warns.

Now, it is also correct that the compiler developers are aware that many
people rely on this implementation detail.

So, to make it possible for you to distinguish between managed and unmanaged
types, there are 2 separate warnings.

But indeed, you must still actually indicate to the compiler that you rely on
these well-known values being used, and you do this by actively telling it
that this particular warning is unnecessary.

So the Lazarus team would be better off giving a list of warnings that are permitted to
be switched off and then your whole problem is solved.

Michael.
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel
Ondrej Pokorny
2018-04-04 22:43:10 UTC
Permalink
On 05.04.2018 0:34, Michael Van Canneyt wrote:
> What, because you do not agree ?

No, because I didn't get any arguments against.

> Now, it is also correct that the compiler developers are aware that
> many people rely on this implementation detail.

Since when is documented behavior considered as "implementation detail"?
This is not an implementation detail. It is in official documentation.

If it was an implementation detail, I would 100% agree with you. But
indeed it is not an implementation detail.

Ondrej
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-deve
Michael Van Canneyt
2018-04-05 06:35:05 UTC
Permalink
On Thu, 5 Apr 2018, Ondrej Pokorny wrote:

> On 05.04.2018 0:34, Michael Van Canneyt wrote:
>> What, because you do not agree ?
>
> No, because I didn't get any arguments against.

You did, you just don't consider them valid.

There are 2 narratives, and you choose to ignore the 2nd one.

>
>> Now, it is also correct that the compiler developers are aware that
>> many people rely on this implementation detail.
>
> Since when is documented behavior considered as "implementation detail"?
> This is not an implementation detail. It is in official documentation.

You and I know very well that this is an implementation detail, needed
for internal bookkeeping of the compiler. That the delphi manual states
otherwise, I have always considered a grave error on their part:
It contradicts one of the core tenets of pascal - see below.

If the compiler devs wanted, they could initialize every string with the
'<undefined string>' constant, every interface with a reference to a constant
IUnknown interface, and that would be equally correct as the current
behaviour from a compiler perspective.
(in fact, it would be an interesting test to do so. See how much code breaks)

Unfortunately, a value is needed for internal bookkeeping of the compiler.
Which value that is, is entirely irrelevant from a compiler perspective.
0 is simply convenient, no more, no less.

Pascal states: do not assume that variables are initialized.

That is the "rule".

From this rule, it follows that every variable must be explicitly initialized by the
programmer at least once before using it, to have correct and predictable
behaviour: Be it with an assignment or an 'var a: type = someonstant;'.

So, if the compiler sees code that uses a variable which is not explicitly
initialized, it warns.

Simple and consequent.

That the warning has been split in 2 warnings is for your convenience.
I suggest you put it to good use.





Michael.
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepasca
Ondrej Pokorny
2018-04-05 07:12:41 UTC
Permalink
On 05.04.2018 8:35, Michael Van Canneyt wrote:
>>> Now, it is also correct that the compiler developers are aware that
>>> many people rely on this implementation detail.
>>
>> Since when is documented behavior considered as "implementation
>> detail"? This is not an implementation detail. It is in official
>> documentation.
>
> You and I know very well that this is an implementation detail, needed
> for internal bookkeeping of the compiler. That the delphi manual states
> otherwise, I have always considered a grave error on their part:
> It contradicts one of the core tenets of pascal - see below.

Huh, no, I don't know this is an implementation detail. For me the
initialization rules always were:

1.) Global variables are initialized. (Why is simple global variables
initialization needed for internal bookkeeping of the compiler?)
2.) Class fields are initialized. (Why is simple class fields
initialization needed for internal bookkeeping of the compiler?)
3.) Everything else is not initialized.

Yes, I have always relied on both 1.+2. For me both points 1.+2. are
well documented and wanted behavior. Isn't it so?

About point 3: your very own docs state otherwise for managed types (see
my initial email in this thread) and I was surprised to read it. I am
not aware the Delphi manual states anything about initializing managed
types. AFAIK Delphi manual states that the compiler itself cares about
memory management automatically but it doesn't state anything about when
initialization or finalization must happen. It doesn't state local
managed variables are initialized to empty string/nil in particular.

Until now I have never relied on implicitely initialized local
variables. Your very own docs document the guaranteed local managed
variable initialization, not Delphi's. If you say it is an
implementation detail, you must delete it from the docs. For me the
"grave error" is on FPC's side and not Delphi's.

Ondrej
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/list
Michael Van Canneyt
2018-04-05 07:24:25 UTC
Permalink
On Thu, 5 Apr 2018, Ondrej Pokorny wrote:

> On 05.04.2018 8:35, Michael Van Canneyt wrote:
>>>> Now, it is also correct that the compiler developers are aware that
>>>> many people rely on this implementation detail.
>>>
>>> Since when is documented behavior considered as "implementation
>>> detail"? This is not an implementation detail. It is in official
>>> documentation.
>>
>> You and I know very well that this is an implementation detail, needed
>> for internal bookkeeping of the compiler. That the delphi manual states
>> otherwise, I have always considered a grave error on their part:
>> It contradicts one of the core tenets of pascal - see below.
>
> Huh, no, I don't know this is an implementation detail. For me the
> initialization rules always were:
>
> 1.) Global variables are initialized. (Why is simple global variables
> initialization needed for internal bookkeeping of the compiler?)
> 2.) Class fields are initialized. (Why is simple class fields
> initialization needed for internal bookkeeping of the compiler?)
> 3.) Everything else is not initialized.


Very well done :)

You explained exactly why I think the delphi docs are wrong.

If delphi wanted to do things correctly and consistently they would
simply say 'everything is initialized to 0'. Period.

Simple, consistent, easy.

The above is a kludge.

The point of view of 'Pascal' is that you should consider every variable
uninitialized.

Simple, straightforward.

Instead, Delphi elevates some arbitrary implementation details to 'rule'
with the above kludge as a result.

It means you count on things which the delphi developer of that day decided to
implement. Maybe because he was drunk. Maybe he was under the influence of
drugs. Maybe because he decided that the 0 key "looks kinda nice". or simply
because he by accident used a section in the binary that the OS just happens
to guarantee to be 0. We don't know. But this accidental behaviour
unfortnately made it in the docs.

We'll just have to live with it, it seems.

Michael.
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.or
Alexander Klenin
2018-04-05 10:47:27 UTC
Permalink
On Thu, Apr 5, 2018 at 10:24 AM, Michael Van Canneyt
<***@freepascal.org> wrote:
>
> You explained exactly why I think the delphi docs are wrong.
>
> If delphi wanted to do things correctly and consistently they would simply
> say 'everything is initialized to 0'. Period.
>
> Simple, consistent, easy.
>
> The above is a kludge.
>
> The point of view of 'Pascal' is that you should consider every variable
> uninitialized.
>
> Simple, straightforward.
>
Allow me to yet again to single out this philosophy of
strongly preferring abstract purity to concrete user experience.
This is IMHO a significant contributing factor of Pascal decline.
I would not argue this specific point (although I quite agree with Ondrej),
I just want to remind that the cost in terms of diminishing userbase
is real, and that makes me sad.

--
Alexander S. Klenin
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepas
Martin Schreiber
2018-04-05 11:24:52 UTC
Permalink
On Thursday 05 April 2018 12:47:27 Alexander Klenin wrote:
>
> Allow me to yet again to single out this philosophy of
> strongly preferring abstract purity to concrete user experience.
> This is IMHO a significant contributing factor of Pascal decline.
> I would not argue this specific point (although I quite agree with Ondrej),
> I just want to remind that the cost in terms of diminishing userbase
> is real, and that makes me sad.

Agreed 100%.

Martin
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lis
Michael Van Canneyt
2018-04-05 11:26:05 UTC
Permalink
On Thu, 5 Apr 2018, Alexander Klenin wrote:

> On Thu, Apr 5, 2018 at 10:24 AM, Michael Van Canneyt
> <***@freepascal.org> wrote:
>>
>> You explained exactly why I think the delphi docs are wrong.
>>
>> If delphi wanted to do things correctly and consistently they would simply
>> say 'everything is initialized to 0'. Period.
>>
>> Simple, consistent, easy.
>>
>> The above is a kludge.
>>
>> The point of view of 'Pascal' is that you should consider every variable
>> uninitialized.
>>
>> Simple, straightforward.
>>
> Allow me to yet again to single out this philosophy of
> strongly preferring abstract purity to concrete user experience.

Stop:

I am not preferring anything.

I'm just pointing out that the current situation is a kludge, it is the
consequence of some ill-advised pragmatism on the part of
Borland/embarcadero/inprise/Codegear.

If it is decided to simply zero out *all* variables - which is a decision based
on a principle and as such an 'abstract purity' - you will not hear me complain.
(Why do you think I mentioned it in the first place ?)

Such a decision may well be beneficial for the user experience.

But until that time, the warning as issued by the compiler is correct,
consequent and in the philosphy of the language, and that is what the
discussion is about.

Michael.
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lis
Nikolai Zhubr
2018-04-05 08:58:13 UTC
Permalink
Hi,

05.04.2018 10:12, Ondrej Pokorny пишет:
[...]
> 1.) Global variables are initialized. (Why is simple global variables
> initialization needed for internal bookkeeping of the compiler?)

IIRC this was historically introduced by Borland (ages ago) because it
was very cheap and easy to ask an OS of their choice to zero-out .data
segment upon executable module loading, effectively getting
initialization for free, and at that time (ages ago!) speed was what
they cared most about.

> 2.) Class fields are initialized. (Why is simple class fields
> initialization needed for internal bookkeeping of the compiler?)

Here again, it was relatively cheap and easy to fillchar/memset object's
body internally before use. Not competely free, but pretty efficient
still. So essentially same reason, I'd say.

> 3.) Everything else is not initialized.

Apparently this was not so cheap and not so easy for Borland (at least
at that time). So apparently they chose to ignore.


From my personal perspective, this indeed is implementation detail,
however wildely abused (me included), and it is a rather unfortunate
situation. E.g. I'd be in a huge disaster if implicitely initialized
strings suddenly start to be '<uninitialized string>' instead of empty.
So ideally, something clever should be done about it at some point.
Maybe indeed declaring it as a guaranteed behaviour or something. I kind
of doubt there is a realistic chance of changing it anyway, due to
compatability reasons...


Thank you.

Regards,
Nikolai
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.free
Karoly Balogh (Charlie/SGR)
2018-04-05 14:22:01 UTC
Permalink
Hi,

Just some technical background to this discussion, I don't care who
documented what, I'm just stating how it works. :P

On Thu, 5 Apr 2018, Ondrej Pokorny wrote:

> >>> Now, it is also correct that the compiler developers are aware that
> >>> many people rely on this implementation detail.
> >>
> >> Since when is documented behavior considered as "implementation
> >> detail"? This is not an implementation detail. It is in official
> >> documentation.
> >
> > You and I know very well that this is an implementation detail, needed
> > for internal bookkeeping of the compiler. That the delphi manual states
> > otherwise, I have always considered a grave error on their part:
> > It contradicts one of the core tenets of pascal - see below.

The Delphi manual probably states the de-facto behavior. You cannot not
initialize global variables to zero on most modern Un*ces and Windows,
because the OS will zero out the bss section on executable loading, which
is where most global variables live. (Unless they're in the data section,
in which case they have a specified initial value, but that's more like
typed consts with an initial value in Pascal's context.)

But again, it's zeroed out, not really "initialized". So for example if
you have a type with say, 5..7 value range, it will still contain zero
after start. Hence, uninitialized, therefore the warning is correct. (See
below for examples.)

> Huh, no, I don't know this is an implementation detail. For me the
> initialization rules always were:
>
> 1.) Global variables are initialized. (Why is simple global variables
> initialization needed for internal bookkeeping of the compiler?)

As I wrote above, we cannot not "initialize" (as in: zero out) the global
variables on modern systems, because the compiler doesn't do it, but the
executable loader of the OS. (Well, it's actually a bit more complicated,
if threadvars are involved, but that's platform specific what happens
there, and the end result is the same - global variables are zeroed out.)

> 2.) Class fields are initialized. (Why is simple class fields
> initialization needed for internal bookkeeping of the compiler?)

I think this is something which happens when constructing the class, so
this is runtime behavior, and not compile or exe loading time, and again,
it's just clearing the allocation after GetMem()'ing the heap area for
the class, so you don't get trash in the fields. But they're don't
necessarily initialized to valid values (if zero is not valid).

> 3.) Everything else is not initialized.

> Yes, I have always relied on both 1.+2. For me both points 1.+2. are
> well documented and wanted behavior. Isn't it so?

Do you want initialized, or zeroed out? Zero out will happen,
initialization will not:

Take these two examples:

type
qq = (foo, bar, klepp, mopp, fubb);
q = klepp..fubb;

var
c: q;

begin
writeln(ord(c)); // will write 0;
writeln(c); // will fail with runtime error (out of range)
end.


And:

{$MODE OBJFPC}

type
qq = (foo, bar, klepp, mopp, fubb);
q = klepp..fubb;

type
clfoo = class
c: q;
end;

var
x: clfoo;

begin
x:=clfoo.create;
writeln(ord(x.c)); // write 0;
writeln(x.c); // runtime error (out of range)
end.

Tested with FPC 3.0.4 32bit on macOS.

I'd be interesting to know if Delphi behaved otherwise. But in the above
case the compiler warning about uninitialized variable is completely
valud, even if it's not random but initialized to zero due to OS and RTL
behavior.

Charlie
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listi
Bart
2018-04-05 17:34:41 UTC
Permalink
On Thu, Apr 5, 2018 at 4:22 PM, Karoly Balogh (Charlie/SGR)
<***@scenergy.dfmk.hu> wrote:


> But again, it's zeroed out, not really "initialized". So for example if
> you have a type with say, 5..7 value range, it will still contain zero
> after start. Hence, uninitialized, therefore the warning is correct. (See
> below for examples.)

Never realized this...


> type
> qq = (foo, bar, klepp, mopp, fubb);
> q = klepp..fubb;
>
> var
> c: q;
>
> begin
> writeln(ord(c)); // will write 0;
> writeln(c); // will fail with runtime error (out of range)
> end.
>
>
> And:
>
> {$MODE OBJFPC}
>
> type
> qq = (foo, bar, klepp, mopp, fubb);
> q = klepp..fubb;
>
> type
> clfoo = class
> c: q;
> end;
>
> var
> x: clfoo;
>
> begin
> x:=clfoo.create;
> writeln(ord(x.c)); // write 0;
> writeln(x.c); // runtime error (out of range)
> end.
>
> Tested with FPC 3.0.4 32bit on macOS.
>
> I'd be interesting to know if Delphi behaved otherwise.

D7 behaves the same, except that in the 2nd example the line
writeln(x.c) cannot be compiled, so I could not test that, but the
line above prints 0.

Bart
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/c
Nikolay Nikolov
2018-07-03 22:00:11 UTC
Permalink
On 04/05/2018 05:22 PM, Karoly Balogh (Charlie/SGR) wrote:
>
>> 2.) Class fields are initialized. (Why is simple class fields
>> initialization needed for internal bookkeeping of the compiler?)
> I think this is something which happens when constructing the class, so
> this is runtime behavior, and not compile or exe loading time, and again,
> it's just clearing the allocation after GetMem()'ing the heap area for
> the class, so you don't get trash in the fields. But they're don't
> necessarily initialized to valid values (if zero is not valid).
Actually, this is done, so that you can safely free all the objects
owned by the class safely in the destructor. If it wasn't done, instead
of this:

constructor TMyClass.Create;
begin
  FObject1 := TSomeClass.Create;
  FObject2 := TSomeClass.Create;
end;

destructor TMyClass.Destroy;
begin
  FObject2.Free;
  FObject1.Free;
end;

you would have to do this in the constructor:

constructor TMyClass.Create;
begin
  FObject1 := nil;
  FObject2 := nil;
  FObject1 := TSomeClass.Create;
  FObject2 := TSomeClass.Create;
end;

Why? Because, if something raises an exception inside the constructor,
the destructor will be called on the partially constructed object. And
if the exception was raised somewhere before 'FObject2 :=
TSomeClass.Create', then, if you didn't initialize FObject2 to nil in
the beginning, the destructor would try to free FObject2, which would be
a random uninitialized pointer.

So, even though filling objects with zero introduces a small runtime
overhead, it saves a lot of pain and prevents difficult to find bugs
(people quite often don't test exceptions raised in the constructor).

Nikolay
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.or
Sven Barth via fpc-devel
2018-07-04 05:28:18 UTC
Permalink
Am 04.07.2018 um 00:00 schrieb Nikolay Nikolov:
> So, even though filling objects with zero introduces a small runtime
> overhead, it saves a lot of pain and prevents difficult to find bugs
> (people quite often don't test exceptions raised in the constructor).
And I don't know how often I've been bitten by C++ not zero-initializing
class members by default, so I'm definitely a fan of this behaviour in
Object Pascal :D

Regards,
Sven
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.or
Martok
2018-04-05 14:29:01 UTC
Permalink
Am 05.04.2018 um 08:35 schrieb Michael Van Canneyt:
> If the compiler devs wanted, they could initialize every string with the
> '<undefined string>' constant,
That is in fact the -gt option.

> Pascal states: do not assume that variables are initialized.
Corollary: there is no guarantee that "class operator Initialize()" is ever
called, and Maciej can roll back management operators. I very much doubt this is
what you want to imply.
To preempt the argument - no, this is not a different case. Managed types are
initialized at the latest when they come into scope, period.

> From this rule, it follows that every variable must be explicitly initialized [...]
> Be it with an assignment or an 'var a: type = someonstant;'.
... for which the syntactic sugar was rejected not two weeks ago.


<Semi-OT>
To the people referring to Borland Language Guides: As I painfully discovered,
the Borland guides are *not* considered normative material for FPC by certain
FPC developers. Not even for -Mtp.
For some reason, ISO 7185 *is* a reference, even for decidedly not-ISO modes.

Am 05.04.2018 um 12:47 schrieb Alexander Klenin:
> Allow me to yet again to single out this philosophy of
> strongly preferring abstract purity to concrete user experience.
If it at least was consistent purity!

Sorry. Needed to be said.
</Semi-OT>

--
Regards,
Martok

Ceterum censeo b32079 esse sanandam.

_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/
Sven Barth via fpc-devel
2018-04-05 15:10:40 UTC
Permalink
Martok <***@martoks-place.de> schrieb am Do., 5. Apr. 2018, 16:29:

> > From this rule, it follows that every variable must be explicitly
> initialized [...]
> > Be it with an assignment or an 'var a: type = someonstant;'.
> ... for which the syntactic sugar was rejected not two weeks ago.
>

Did we read the same thread?

"var a: type = x" is already supported for both global and local variables
and Florian agreed that there is no reason to restrict that only to a
single variable (but only with the same value for each).
Okay, Jonas disagreed, but personally am in favor of allowing that as well.

Regards,
Sven

>
Ondrej Pokorny
2018-04-12 11:26:38 UTC
Permalink
On 05.04.2018 17:10, Sven Barth via fpc-devel wrote:
> Martok <***@martoks-place.de <mailto:***@martoks-place.de>>
> schrieb am Do., 5. Apr. 2018, 16:29:
>
> > From this rule, it follows that every variable must be
> explicitly initialized [...]
> > Be it with an assignment or an 'var a: type = someonstant;'.
> ... for which the syntactic sugar was rejected not two weeks ago.
>
>
> Did we read the same thread?
>
> "var a: type = x" is already supported for both global and local
> variables and Florian agreed that there is no reason to restrict that
> only to a single variable (but only with the same value for each).
> Okay, Jonas disagreed, but personally am in favor of allowing that as
> well.

Good to know your opinion.

From what I read the position of rejecting the idea was much stronger
than the support.

Ondrej
Michael Van Canneyt
2018-04-05 15:18:27 UTC
Permalink
On Thu, 5 Apr 2018, Martok wrote:

> Am 05.04.2018 um 08:35 schrieb Michael Van Canneyt:
>> If the compiler devs wanted, they could initialize every string with the
>> '<undefined string>' constant,
> That is in fact the -gt option.

I doubt -gt does something for managed types?

>
>> Pascal states: do not assume that variables are initialized.
> Corollary: there is no guarantee that "class operator Initialize()" is ever
> called, and Maciej can roll back management operators. I very much doubt this is
> what you want to imply.

And why not ?

This is the whole problem and source of the "kludge' I referred to:

When introducing managed types, Delphi should have performed the full monty,
and simply zero-out everything at any point.

This would eliminate in large part the need for management operators, since
you can then detect whether a type is initialized or not if so desired, and
act on it.

TMyManagedType = Record
IsInitialized : Boolean;
// The rest of what you actually need
end;


Michael.
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepas
Jonas Maebe
2018-04-05 17:16:50 UTC
Permalink
On 05/04/18 17:18, Michael Van Canneyt wrote:
> On Thu, 5 Apr 2018, Martok wrote:
>
>> Am 05.04.2018 um 08:35 schrieb Michael Van Canneyt:
>>> If the compiler devs wanted, they could initialize every string with the
>>> '<undefined string>' constant,
>> That is in fact the -gt option.
>
> I doubt -gt does something for managed types?

It does not, except for managed string results (because those are not
guaranteed to be empty by Delphi either).


Jonas
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://l
Ondrej Pokorny
2018-07-03 16:40:56 UTC
Permalink
On 04.04.2018 21:51, Jonas Maebe wrote:
> On 04/04/18 20:26, Ondrej Pokorny wrote:
>> The compiler initializes the variable implicitely for myself - this
>> is documented and I know it. There cannot be "wrong code behaviour"
>> (as you stated) and thus I cannot have an error. This is simple logic.
>
> "Wrong behaviour" is not the same as "undefined behaviour" or "crash".
> It merely means "something different than the programmer intended". It
> is of course possible that you wanted the behaviour that you get when
> the variable contains the empty string, nil or 0, but because this
> initialisation is implicit rather than explicit, the compiler does not
> rely on this. After all, it is equally possible that you forgot to
> assign the correct (non-nil/0/empty string) value to the variable
> before using it the first time (in which case it would be an error in
> the program).
>
>> I must not. Turning off warnings is forbidden. Turning off hints and
>> notes is OK. That is the rules.
>
> The inflexibility of that rule in the Lazarus project is not a good
> reason to change the priority of the message.
>
>> But I am not fine with the fact that the level of this message is a
>> WARNING - the same level that real code errors are reported.
>
> Warnings are not errors. Warnings indicate possible errors, but they
> can also be false positives.

The recent change in SetLength emitting an "uninitialized variable"
error is a 100% false positive, completely useless and produces only
garbage in compiler output.

Reasons:

1.) SetLength itself produces an unitialized block of data within the
variable if the new length is bigger than the old length.

Therefore it would be more appropriate to get an "uninitialized warning"
after SetLength:

program Project1;
var
  S: string;
begin
  SetLength(S, 10); // <<< warning here     --->> WHY ????
  Writeln(S[2]);    // <<< no warning here  --->> WHY ????
end.

2.) If the variable was uninitialized before SetLength it will be
uninitialized after SetLength as well, regardless if the new length if
bigger or smaller.

3.) The old data is not really read or used anyhow. It is just copied on
raw-basis.

4.) I use "SetLength(A, 0);" to explicitely initialize an array. The
compiler warns me now that A is not initialized. Seriously?
I know that with a recent FPC version I can use:

program Project1;
var
  A: array of Integer;
begin
  A := [];
end.

or

program Project1;
type
  IntArray = array of Integer;
var
  A: IntArray;
begin
  A := Default(IntArray);
end.

but using "SetLength(A, 0)" is the old way to do it:

program Project1;
var
  A: array of Integer;
begin
  SetLength(A, 0); // <<< warning here     --->> WHY ????
end.

5.) If you really want to handle SetLength, then SetLength should be a
trigger for the warning detection in subsequent code (warnings should
appear after SetLength):

program Project1;
var
  S: array of string;
begin
  S := [];
  SetLength(S, 10);
  Writeln(S[2]);    // <<< no warning here  --->> WHY ????
end.

---

All in all, there should not be a warning in SetLength at all. Because
SetLength is a compiler intrinsic with well-defined and documented
behavior I am sure it can be handled as I described (desired = no
warning at SetLength, but warning afterwards).

Ondrej
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/cgi-bi
Ralf Quint
2018-07-03 17:00:09 UTC
Permalink
On 7/3/2018 9:40 AM, Ondrej Pokorny wrote:
>
> 1.) SetLength itself produces an unitialized block of data within the
> variable if the new length is bigger than the old length.
>
> Therefore it would be more appropriate to get an "uninitialized
> warning" after SetLength:
>
> program Project1;
> var
>   S: string;
> begin
>   SetLength(S, 10); // <<< warning here     --->> WHY ????
>   Writeln(S[2]);    // <<< no warning here  --->> WHY ????
> end.
The program does EXACTLY what you told it to do and the compiler gives
you an explicit warning that that what you are doing is potentially
wrong/dangerous.
If you set the length of a string, without it having been
initialized/assigned a value, all you do is to tell the program that you
have now a string of a specified length. And as long as you do not
exceed that specified length, there is no reason for the compiler to
complain about it. YOU explicitly defined that accessing any
character/element up to that defined length is valid!
You as a programmer failed to heed the warning at the SetLength()
function and adjusted your code accordingly.  You as a programmer should
realize that there are random characters in the string if you extend the
length of the string. How is the extended length to be initialized?
Blanks (0x20), Zeroes (0x00)? What about the following example?

Program ABC;
Var S : String;
begin
  S := '12345';
  SetLength (S, 10);
  WriteLn (S [2]);
  WriteLn (S [7]);
end.   

Ralf
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.f
Ondrej Pokorny
2018-07-03 17:27:18 UTC
Permalink
On 03.07.2018 19:00, Ralf Quint wrote:
> On 7/3/2018 9:40 AM, Ondrej Pokorny wrote:
>> 1.) SetLength itself produces an unitialized block of data within the
>> variable if the new length is bigger than the old length.
>>
>> Therefore it would be more appropriate to get an "uninitialized
>> warning" after SetLength:
>>
>> program Project1;
>> var
>>   S: string;
>> begin
>>   SetLength(S, 10); // <<< warning here     --->> WHY ????
>>   Writeln(S[2]);    // <<< no warning here  --->> WHY ????
>> end.
> The program does EXACTLY what you told it to do and the compiler gives
> you an explicit warning that that what you are doing is potentially
> wrong/dangerous.

No no no. You miss the point and you are wrong.

The compiler tells me that the line "SetLength(S, 10);" is potentially
dangerous, which it is not.
The line "Writeln(S[2]);" is potentially dangerous, but the compiler
does not warn me about it.
=> The compiler warns me about the wrong line.

See:

program Project1;
var
  S: string[10];
begin
  Writeln(S[2]);    // <<< warning here  --->> OK
end.

With your arguments I should get a warning while at line "S:
string[10];" and no warning at "Writeln(S[2]);". Seriously?

> If you set the length of a string, without it having been
> initialized/assigned a value, all you do is to tell the program that you
> have now a string of a specified length. And as long as you do not
> exceed that specified length, there is no reason for the compiler to
> complain about it. YOU explicitly defined that accessing any
> character/element up to that defined length is valid!
> You as a programmer failed to heed the warning at the SetLength()
> function and adjusted your code accordingly.  You as a programmer should
> realize that there are random characters in the string if you extend the
> length of the string. How is the extended length to be initialized?
> Blanks (0x20), Zeroes (0x00)?

You obviously miss my point. Please read my previous email again. I
don't feel the need to go into this discussion because it is off-topic.


> What about the following example?
>
> Program ABC;
> Var S : String;
> begin
>   S := '12345';
>   SetLength (S, 10);
>   WriteLn (S [2]);
>   WriteLn (S [7]);
> end.

I should get a warning in line "WriteLn (S [2]);". (Btw. if you read my
previous email carefully you would see that this case is covered in
point 5.)

The same as I get a warning in (the same example as above):

program Project1;
var
  S: string[10];
begin
  Writeln(S[2]);    // <<< warning here  --->> OK
end.

Ondrej
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/
Martin
2018-07-03 18:06:49 UTC
Permalink
On 03/07/2018 19:27, Ondrej Pokorny wrote:
>> On 7/3/2018 9:40 AM, Ondrej Pokorny wrote:
>>> program Project1;
>>> var
>>>    S: string;
>>> begin
>>>    SetLength(S, 10); // <<< warning here --->> WHY ????
>>>    Writeln(S[2]);    // <<< no warning here --->> WHY ????
>>> end.
>>
>
> The compiler tells me that the line "SetLength(S, 10);" is potentially
> dangerous, which it is not.
"writeln(a)" where a is "integer" is not >dangerous< either. Yet it is
an uninitialized value that you write, and therefore likely not what you
want (or you would have used random).
"potential error" <> "dangerous"

In any case,
- "S" is the array (the container, the length, and internally the
pointer to the memory).
- SetLength is a function taking a "var param"

S has no specific value yet. So SetLenght gets a random value as param.
Therefore this warning is correct.
To initialize "S":
   S := nil; // no warning

> The line "Writeln(S[2]);" is potentially dangerous, but the compiler
> does not warn me about it.
It is a "potential error", dangerous or not.

I agree in a perfect world the compiler would keep track of each
individual element in an array, and know which ones are initialized. In
reality this doesn't work.

In theory there are 3 states that the compiler can have:
- it knows a variable is not initialized => It warns (even if the case
it harmless)
- it knows a variable is initialized => it doesn't warn
- it doesn't know if a variable is initialized => it doesn't warn (and
that is IMHO correct)

SetLength(S, 10); // The compiler knows S is not initialized
Writeln(S[2]);    // The compiler does not know (this is about S[2], not
about S)

In the "s[2]" case, it could theoretically be implemented as a very
special case. But the majority of array element access is not practical
to implement.
And even in this case the cost/value is not balancing out.


> => The compiler warns me about the wrong line.
No, but it only gives you one out of 2 warnings.


_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lis
Ondrej Pokorny
2018-07-03 19:54:12 UTC
Permalink
On 03.07.2018 20:06, Martin wrote:
> On 03/07/2018 19:27, Ondrej Pokorny wrote:
>>> On 7/3/2018 9:40 AM, Ondrej Pokorny wrote:
>>>> program Project1;
>>>> var
>>>>    S: string;
>>>> begin
>>>>    SetLength(S, 10); // <<< warning here --->> WHY ????
>>>>    Writeln(S[2]);    // <<< no warning here --->> WHY ????
>>>> end.
>>>
>>
>> The compiler tells me that the line "SetLength(S, 10);" is
>> potentially dangerous, which it is not.
> "writeln(a)" where a is "integer" is not >dangerous< either. Yet it is
> an uninitialized value that you write, and therefore likely not what
> you want (or you would have used random).
> "potential error" <> "dangerous"
>
> In any case,
> - "S" is the array (the container, the length, and internally the
> pointer to the memory).
> - SetLength is a function taking a "var param"
>
> S has no specific value yet. So SetLenght gets a random value as
> param. Therefore this warning is correct.
> To initialize "S":
>    S := nil; // no warning
>
>> The line "Writeln(S[2]);" is potentially dangerous, but the compiler
>> does not warn me about it.
> It is a "potential error", dangerous or not.
>
> I agree in a perfect world the compiler would keep track of each
> individual element in an array, and know which ones are initialized.
> In reality this doesn't work.
>
> In theory there are 3 states that the compiler can have:
> - it knows a variable is not initialized => It warns (even if the case
> it harmless)
> - it knows a variable is initialized => it doesn't warn
> - it doesn't know if a variable is initialized => it doesn't warn (and
> that is IMHO correct)
>
> SetLength(S, 10); // The compiler knows S is not initialized
> Writeln(S[2]);    // The compiler does not know (this is about S[2],
> not about S)
>
> In the "s[2]" case, it could theoretically be implemented as a very
> special case. But the majority of array element access is not
> practical to implement.
> And even in this case the cost/value is not balancing out.

You three (Ralf, Stefan and you) completely missed my point.

The warning and my point is not about array elements at all. Yes, I used
an array element as an example but I could easily use the whole
array/string in a different way, too.


>> => The compiler warns me about the wrong line.
> No, but it only gives you one out of 2 warnings.

No, the compiler warns me about the wrong line. SetLength/FillChar
should not emit a warning at all because there is nothing to warn about.
See my reply to Florian if my first email with the points was not clear
enough.

Ondrej
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listi
Stefan Glienke
2018-07-03 17:53:23 UTC
Permalink
SetLength should not cause anything uninitialized.

It enlarges or shrinks the data and keeps any prior data as it was, new allocated memory is zeroed (at least that is how it works in the Delphi RTL and I would be very surprised if FPC would do any different).

The core issue imo is temp var reuse the compiler does which causes hidden data reusage/copy which is only an issue with dynamic arrays because they don't have CoW semantic when writing to some index. So the temp variable still points to the first array which then gets passed to the next function as result. There a call to SetLength applies copy on write semantics carrying over any content from the previous array that is still in the temp variable to the newly allocated array.

I think we discussed this on friday - maybe I did not make my point clear enough.

> On 03 July 2018 at 18:40 Ondrej Pokorny <***@kluug.net> wrote:
>
>
> On 04.04.2018 21:51, Jonas Maebe wrote:
> > On 04/04/18 20:26, Ondrej Pokorny wrote:
> >> The compiler initializes the variable implicitely for myself - this
> >> is documented and I know it. There cannot be "wrong code behaviour"
> >> (as you stated) and thus I cannot have an error. This is simple logic.
> >
> > "Wrong behaviour" is not the same as "undefined behaviour" or "crash".
> > It merely means "something different than the programmer intended". It
> > is of course possible that you wanted the behaviour that you get when
> > the variable contains the empty string, nil or 0, but because this
> > initialisation is implicit rather than explicit, the compiler does not
> > rely on this. After all, it is equally possible that you forgot to
> > assign the correct (non-nil/0/empty string) value to the variable
> > before using it the first time (in which case it would be an error in
> > the program).
> >
> >> I must not. Turning off warnings is forbidden. Turning off hints and
> >> notes is OK. That is the rules.
> >
> > The inflexibility of that rule in the Lazarus project is not a good
> > reason to change the priority of the message.
> >
> >> But I am not fine with the fact that the level of this message is a
> >> WARNING - the same level that real code errors are reported.
> >
> > Warnings are not errors. Warnings indicate possible errors, but they
> > can also be false positives.
>
> The recent change in SetLength emitting an "uninitialized variable"
> error is a 100% false positive, completely useless and produces only
> garbage in compiler output.
>
> Reasons:
>
> 1.) SetLength itself produces an unitialized block of data within the
> variable if the new length is bigger than the old length.
>
> Therefore it would be more appropriate to get an "uninitialized warning"
> after SetLength:
>
> program Project1;
> var
> S: string;
> begin
> SetLength(S, 10); // <<< warning here --->> WHY ????
> Writeln(S[2]); // <<< no warning here --->> WHY ????
> end.
>
> 2.) If the variable was uninitialized before SetLength it will be
> uninitialized after SetLength as well, regardless if the new length if
> bigger or smaller.
>
> 3.) The old data is not really read or used anyhow. It is just copied on
> raw-basis.
>
> 4.) I use "SetLength(A, 0);" to explicitely initialize an array. The
> compiler warns me now that A is not initialized. Seriously?
> I know that with a recent FPC version I can use:
>
> program Project1;
> var
> A: array of Integer;
> begin
> A := [];
> end.
>
> or
>
> program Project1;
> type
> IntArray = array of Integer;
> var
> A: IntArray;
> begin
> A := Default(IntArray);
> end.
>
> but using "SetLength(A, 0)" is the old way to do it:
>
> program Project1;
> var
> A: array of Integer;
> begin
> SetLength(A, 0); // <<< warning here --->> WHY ????
> end.
>
> 5.) If you really want to handle SetLength, then SetLength should be a
> trigger for the warning detection in subsequent code (warnings should
> appear after SetLength):
>
> program Project1;
> var
> S: array of string;
> begin
> S := [];
> SetLength(S, 10);
> Writeln(S[2]); // <<< no warning here --->> WHY ????
> end.
>
> ---
>
> All in all, there should not be a warning in SetLength at all. Because
> SetLength is a compiler intrinsic with well-defined and documented
> behavior I am sure it can be handled as I described (desired = no
> warning at SetLength, but warning afterwards).
>
> Ondrej
> _______________________________________________
> fpc-devel maillist - fpc-***@lists.freepascal.org
> http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/c
Ondrej Pokorny
2018-07-03 18:46:47 UTC
Permalink
On 03.07.2018 19:53, Stefan Glienke wrote:
> SetLength should not cause anything uninitialized.
>
> It enlarges or shrinks the data and keeps any prior data as it was, new allocated memory is zeroed (at least that is how it works in the Delphi RTL and I would be very surprised if FPC would do any different).

This is not relevant to what I wrote about.

> The core issue imo is temp var reuse the compiler does which causes hidden data reusage/copy which is only an issue with dynamic arrays because they don't have CoW semantic when writing to some index. So the temp variable still points to the first array which then gets passed to the next function as result. There a call to SetLength applies copy on write semantics carrying over any content from the previous array that is still in the temp variable to the newly allocated array.
>
> I think we discussed this on friday - maybe I did not make my point clear enough.

Again, implicit initialization / zeroing / whatever in the generated
code is not relevant about what I wrote about.

My point is about the warning only.

Ondrej
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http:/
Florian Klämpfl
2018-07-03 18:54:04 UTC
Permalink
Am 03.07.2018 um 20:46 schrieb Ondrej Pokorny:
> On 03.07.2018 19:53, Stefan Glienke wrote:
>> SetLength should not cause anything uninitialized.
>>
>> It enlarges or shrinks the data and keeps any prior data as it was, new allocated memory is zeroed (at least that is
>> how it works in the Delphi RTL and I would be very surprised if FPC would do any different).
>
> This is not relevant to what I wrote about.
>
>> The core issue imo is temp var reuse the compiler does which causes hidden data reusage/copy which is only an issue
>> with dynamic arrays because they don't have CoW semantic when writing to some index. So the temp variable still points
>> to the first array which then gets passed to the next function as result. There a call to SetLength applies copy on
>> write semantics carrying over any content from the previous array that is still in the temp variable to the newly
>> allocated array.
>>
>> I think we discussed this on friday - maybe I did not make my point clear enough.
>
> Again, implicit initialization / zeroing / whatever in the generated code is not relevant about what I wrote about.
>
> My point is about the warning only.

The warning happens also for any other call which takes an uninitialized variable as var parameter (see e.g. fillchar).
So the warning increases only orthogonality of the language. It was an oversight that it not was thrown before.

_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/cgi-b
Ondrej Pokorny
2018-07-03 19:30:57 UTC
Permalink
On 03.07.2018 20:54, Florian Klämpfl wrote:
> The warning happens also for any other call which takes an
> uninitialized variable as var parameter (see e.g. fillchar). So the
> warning increases only orthogonality of the language. It was an
> oversight that it not was thrown before.

This is correct in the oversimplified world only.

The not-oversimplified world is if the compiler was able to detect if
the data in the VAR parameter is being read/used (and not initialized
first) and emit a warning only for the methods that do read the data.
This would both increase orthogonality of the warning (warning is not
really the language, isn't it?) and it would be logical. E.g.:

program Project1;
type
  TMyArray = array[0..10] of Integer;
  procedure Init2(var A: TMyArray);
  begin
    A[2] := 1;
  end;
var
  A: TMyArray;
begin
  Init2(A);
end.

should not emit the warning because A is not being read in Init2.

IMPORTANT: the compiler knows that SetLength and FillChar don't read/use
the data so no warning is appropriate there. Neither in FillChar, nor in
SetLength.

---

What you achieved with SetLength and FillChar emitting a warning is that
you made the compiler more stupid instead of making it more clever.

If you wanted to make the compiler clever, you would try to detect if
the VAR parameter data is really being read/used and emit a warning only
if you know it reads the data or when in doubt. IMO there are ways to
detect if the VAR parameter is being read in a user-defined routine
without initializing it there - they are similar to the ways that you
use to detect an uninitialized local variable.

---

Furthermore from the programmer's point of view nothing can be more away
from the purpose of the warning - both FillChar and SetLength.

The warning should warn me about potential errors in my code. Using
SetLength or FillChar on an uninitialized variable is not and never can
be a potential error. The compiler knows it because both SetLength and
FillChar are compiler intrinsics. The compiler should be smart enough to
know it and should not emit a warning in this case.

---

Btw. what I don't understand either: the above code (Init2(A);) and
FillChar emit a hint only, SetLength emits a warning... It doesn't look
orthogonal to me at all:

program Project1;
type
  TMyArray = array of Integer;
var
  A: TMyArray;
begin
  FillChar(A, 1, 0); // <<< HINT
end.

program Project1;
type
  TMyArray = array of Integer;
var
  A: TMyArray;
begin
  SetLength(A, 1); // <<< WARNING
end.


Ondrej
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lis
Florian Klämpfl
2018-07-03 20:08:52 UTC
Permalink
Am 03.07.2018 um 21:30 schrieb Ondrej Pokorny:
> On 03.07.2018 20:54, Florian Klämpfl wrote:
>> The warning happens also for any other call which takes an uninitialized variable as var parameter (see e.g.
>> fillchar). So the warning increases only orthogonality of the language. It was an oversight that it not was thrown
>> before.
>
> This is correct in the oversimplified world only.

I changed it because some people thought they might make their point with it. Thank them.

>
> The not-oversimplified world is if the compiler was able to detect if the data in the VAR parameter is being read/used
> (and not initialized first) and emit a warning only for the methods that do read the data. This would both increase
> orthogonality of the warning (warning is not really the language, isn't it?) and it would be logical. E.g.:
>
> program Project1;
> type
>   TMyArray = array[0..10] of Integer;
>   procedure Init2(var A: TMyArray);
>   begin
>     A[2] := 1;
>   end;
> var
>   A: TMyArray;
> begin
>   Init2(A);
> end.
>
> should not emit the warning because A is not being read in Init2.

(please note it throws only a hint in this case as A *is* initialized as it is a global variable).

FPC has no global data flow analysis (and I do not plan to do so yet), so this is warning is currently far out of reach.

>
> IMPORTANT: the compiler knows that SetLength and FillChar don't read/use the data so no warning is appropriate there.

SetLength does somehow :), as the examples showed. FillChar is a different thing, it has the problem due of its TP
inheritance (and the compiler does not know that fillchar is special).

> Neither in FillChar, nor in SetLength.
>
> ---
>
> What you achieved with SetLength and FillChar emitting a warning is that you made the compiler more stupid instead of
> making it more clever.

The compiler has not to be clever, it has to follow rules. And the rules which need to be followed is the meaning of the
var parameter. It means: data in/data out.
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailma
Ondrej Pokorny
2018-07-03 20:57:27 UTC
Permalink
On 03.07.2018 22:08, Florian Klämpfl wrote:
> Am 03.07.2018 um 21:30 schrieb Ondrej Pokorny:
>> On 03.07.2018 20:54, Florian Klämpfl wrote:
>> program Project1;
>> type
>>    TMyArray = array[0..10] of Integer;
>>    procedure Init2(var A: TMyArray);
>>    begin
>>      A[2] := 1;
>>    end;
>> var
>>    A: TMyArray;
>> begin
>>    Init2(A);
>> end.
>>
>> should not emit the warning because A is not being read in Init2.
>
> (please note it throws only a hint in this case as A *is* initialized
> as it is a global variable).
>
> FPC has no global data flow analysis (and I do not plan to do so yet),
> so this is warning is currently far out of reach.

Doing it for SetLength/FillChar would be a good start.


>> IMPORTANT: the compiler knows that SetLength and FillChar don't
>> read/use the data so no warning is appropriate there.
>
> SetLength does somehow :), as the examples showed.

Not really. It keeps the "read/used" data within the variable - the
possibly uninitialized data is not written or used outside the variable
itself.


>> What you achieved with SetLength and FillChar emitting a warning is
>> that you made the compiler more stupid instead of making it more clever.
>
> The compiler has not to be clever, it has to follow rules. And the
> rules which need to be followed is the meaning of the var parameter.
> It means: data in/data out.

It's hard to argue like this with all the different hints/warnings/notes
being emitted on the notification side and different optimizations being
done on the compiler side. To me it looks more like this: "the compiler
may be clever only when we want it to be".

---

Probably I am the only one who thinks that the code below is ridiculous...

    procedure TExternalAssemblerOutputFile.AsmWriteFiltered(p: pchar;
len: longint);
      var
        s: ansistring;
      begin
        MaybeAddLinePrefix;
        s:=''; // <<< ????
        setlength(s,len);
        move(p^,s[1],len);
        AsmWriteAnsiStringUnfiltered(decorator.LineFilter(s));
      end;

---

I have split feelings: I really like to get the warning/hint at places
where it makes sense so I don't want to disable it globally. But the
amount of false positives increased significantly :/

Ondrej
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepas
Florian Klämpfl
2018-07-03 21:09:57 UTC
Permalink
Am 03.07.2018 um 22:57 schrieb Ondrej Pokorny:
> On 03.07.2018 22:08, Florian Klämpfl wrote:
>> Am 03.07.2018 um 21:30 schrieb Ondrej Pokorny:
>>> On 03.07.2018 20:54, Florian Klämpfl wrote:
>>> program Project1;
>>> type
>>>    TMyArray = array[0..10] of Integer;
>>>    procedure Init2(var A: TMyArray);
>>>    begin
>>>      A[2] := 1;
>>>    end;
>>> var
>>>    A: TMyArray;
>>> begin
>>>    Init2(A);
>>> end.
>>>
>>> should not emit the warning because A is not being read in Init2.
>>
>> (please note it throws only a hint in this case as A *is* initialized as it is a global variable).
>>
>> FPC has no global data flow analysis (and I do not plan to do so yet), so this is warning is currently far out of reach.
>
> Doing it for SetLength/FillChar would be a good start.

For me: Too much effort for very little gain. Global data flow analysis in general is really a difficult topic and its
advantages are oversee-able imo.

>
>
>>> IMPORTANT: the compiler knows that SetLength and FillChar don't read/use the data so no warning is appropriate there.
>>
>> SetLength does somehow :), as the examples showed.
>
> Not really. It keeps the "read/used" data within the variable - the possibly uninitialized data is not written or used
> outside the variable itself.

?

>
>
>>> What you achieved with SetLength and FillChar emitting a warning is that you made the compiler more stupid instead of
>>> making it more clever.
>>
>> The compiler has not to be clever, it has to follow rules. And the rules which need to be followed is the meaning of
>> the var parameter. It means: data in/data out.
>
> It's hard to argue like this with all the different hints/warnings/notes being emitted on the notification side and
> different optimizations being done on the compiler side. To me it looks more like this: "the compiler may be clever only
> when we want it to be".
>
> ---
>
> Probably I am the only one who thinks that the code below is ridiculous...
>
>     procedure TExternalAssemblerOutputFile.AsmWriteFiltered(p: pchar; len: longint);
>       var
>         s: ansistring;
>       begin
>         MaybeAddLinePrefix;
>         s:=''; // <<< ????
>         setlength(s,len);
>         move(p^,s[1],len);
>         AsmWriteAnsiStringUnfiltered(decorator.LineFilter(s));
>       end;
>
> ---
>
> I have split feelings: I really like to get the warning/hint at places where it makes sense so I don't want to disable
> it globally. But the amount of false positives increased significantly :/

The compiler is ~400k lines and I had to fix maybe 40 locations, so I wouldn't call this significantly and there are
situations where it matters, see the result case.

_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/c
Ondrej Pokorny
2018-07-03 22:26:00 UTC
Permalink
On 03.07.2018 23:09, Florian Klämpfl wrote:
> Am 03.07.2018 um 22:57 schrieb Ondrej Pokorny:
>> On 03.07.2018 22:08, Florian Klämpfl wrote:
>>> Am 03.07.2018 um 21:30 schrieb Ondrej Pokorny:
>>>> On 03.07.2018 20:54, Florian Klämpfl wrote:
>>>> program Project1;
>>>> type
>>>>    TMyArray = array[0..10] of Integer;
>>>>    procedure Init2(var A: TMyArray);
>>>>    begin
>>>>      A[2] := 1;
>>>>    end;
>>>> var
>>>>    A: TMyArray;
>>>> begin
>>>>    Init2(A);
>>>> end.
>>>>
>>>> should not emit the warning because A is not being read in Init2.
>>>
>>> (please note it throws only a hint in this case as A *is*
>>> initialized as it is a global variable).
>>>
>>> FPC has no global data flow analysis (and I do not plan to do so
>>> yet), so this is warning is currently far out of reach.
>>
>> Doing it for SetLength/FillChar would be a good start.
>
> For me: Too much effort for very little gain. Global data flow
> analysis in general is really a difficult topic and its advantages are
> oversee-able imo.

Although being a little bit off-topic, IMO it's not that hard and the
tools are in FPC already now. You have analysis for uninitialized local
variable already. Use the same approach for the var argument and store
the result as an argument flag:

procedure ProcA(var A: Integer);
var
  AA: Integer;
begin
  AA := AA + 1; // warning here
  A := A + 1; // same situation as with AA: warning for AA -> set the
"var A" flag to "data in" + "data out"
end;

procedure ProcB(var B: Integer);
var
  BB: Integer;
begin
  BB := 1; // no warning here
  B := 1; // same situation as with BB: no warning for BB -> set the
"var B" flag to "data not in" + "data out"
end;

procedure Test;
var
  X, Y: Integer;
begin
  ProcA(X); // << warning here because ProcA.A flag is "data in"
  ProcB(Y); // << no warning here because ProcB.B flag is "data not in"
end;

Then you should handle the "data not out"/"data out" flag as well. Yes,
probably this analysis is an overkill and it only slows down the
compiler with little advantage.

Such an analysis is not needed for SetLength because the flag "data not
in"+"data not out" is there per definition. I agree about FillChar - I
forgot it is a normal routine and not an intrinsic, so no luck with an
assumption here.


> The compiler is ~400k lines and I had to fix maybe 40 locations, so I
> wouldn't call this significantly and there are situations where it
> matters, see the result case.

If you refer to:

function DoSomething (len: Integer): Vector;
begin
  SetLength(Result, len);
end;

IMO SetLength should be either invisible for variable initialization
warning detection (because "data not in"+"data not out") or it should be
a trigger for an uninitialized warning in subsequent code. (Probably the
former case is easier and better.)

In both cases I described in the last paragraph, you should not emit a
warning at SetLength call but you should emit a warning as if the
routine was empty:
function DoSomething (len: Integer): Vector;
begin
end;
>>> project1.lpr(6,10) Warning: Function result does not seem to be set

But yes, I agree that emitting a warning at SetLength is even easier. So
yes, you are right. Better to show a warning at SetLength than not
showing it at all.

Ondrej
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.
Yuriy Sydorov
2018-07-04 10:08:47 UTC
Permalink
On 7/4/2018 12:09 AM, Florian Klämpfl wrote:
> Am 03.07.2018 um 22:57 schrieb Ondrej Pokorny:
>> Probably I am the only one who thinks that the code below is ridiculous...
>>
>>      procedure TExternalAssemblerOutputFile.AsmWriteFiltered(p: pchar; len: longint);
>>        var
>>          s: ansistring;
>>        begin
>>          MaybeAddLinePrefix;
>>          s:=''; // <<< ????
>>          setlength(s,len);
>>          move(p^,s[1],len);
>>          AsmWriteAnsiStringUnfiltered(decorator.LineFilter(s));
>>        end;
>>
>> ---
>>
>> I have split feelings: I really like to get the warning/hint at places where it makes sense so I don't want to disable
>> it globally. But the amount of false positives increased significantly :/
>
> The compiler is ~400k lines and I had to fix maybe 40 locations, so I wouldn't call this significantly and there are
> situations where it matters, see the result case.

I'm not happy with this 'uninitialized' warnings for vars of managed types too.
Maybe it's worth to loosen the warning rule to prevent emitting warnings when an 'uninitialized' variable of a managed
type (actually initialized) is passed as var parameter. Remove the warning or change it to a note in such case.

Yuriy.
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.fre
Michael Van Canneyt
2018-07-04 10:16:16 UTC
Permalink
On Wed, 4 Jul 2018, Yuriy Sydorov wrote:

> On 7/4/2018 12:09 AM, Florian KlÀmpfl wrote:
>> Am 03.07.2018 um 22:57 schrieb Ondrej Pokorny:
>>> Probably I am the only one who thinks that the code below is ridiculous...
>>>
>>>      procedure TExternalAssemblerOutputFile.AsmWriteFiltered(p: pchar;
> len: longint);
>>>        var
>>>          s: ansistring;
>>>        begin
>>>          MaybeAddLinePrefix;
>>>          s:=''; // <<< ????
>>>          setlength(s,len);
>>>          move(p^,s[1],len);
>>>          AsmWriteAnsiStringUnfiltered(decorator.LineFilter(s));
>>>        end;
>>>
>>> ---
>>>
>>> I have split feelings: I really like to get the warning/hint at places
> where it makes sense so I don't want to disable
>>> it globally. But the amount of false positives increased significantly :/
>>
>> The compiler is ~400k lines and I had to fix maybe 40 locations, so I
> wouldn't call this significantly and there are
>> situations where it matters, see the result case.
>
> I'm not happy with this 'uninitialized' warnings for vars of managed types
> too.
> Maybe it's worth to loosen the warning rule to prevent emitting warnings when
> an 'uninitialized' variable of a managed
> type (actually initialized) is passed as var parameter. Remove the warning or
> change it to a note in such case.

The managed variables are not initialized. They are 'set up' so they don't
crash the RTL. This is not the same.

I believe that pas2js has 2 such messages. One for managed types, one for
unmanaged (or something similar).

Maybe FPC can do the same (if it already didn't do this),
and you can simply disable the one for managed types on the command-line.

Michael.
Ondrej Pokorny
2018-07-07 05:35:11 UTC
Permalink
On 04.07.2018 12:08, Yuriy Sydorov wrote:
> Remove the warning or change it to a note in such case.

Earlier in this thread I tried to convince the FPC team to downgrade the
warning to a note. Without success. I was told that there is is no "good
reason to change the priority of the message".

Actually there is the same compiler message as a note as well already
now - the compiler decides if a warning or hint is shown on irrational
reasons. E.g.:

program Project1;
procedure TestSetLength;
var
  S: string;
begin
  SetLength(S, 0);
end;
procedure TestFillChar;
var
  S: string;
begin
  FillChar(S, 5, 0);
end;
begin
  TestSetLength;
  TestFillChar;
end.

Guess what line (SetLength or FillChar) is bad and guess what line has a
higher message level.

FPC has three (!) non-fatal message levels before error (hint, note,
warning) and yet they decided to use the highest rank in this case.
Probably because to use SetLength/... on an uninitialized managed local
variable it is the most unsafe yet compilable code ever typed into the
editor.

If I was FillChar I would sue the FPC team for discrimination in front
of the EU court that I want a warning as well, like SetLength. As
FillChar I would feel less important :P

Ondrej
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.
Sven Barth via fpc-devel
2018-07-03 21:50:33 UTC
Permalink
Ondrej Pokorny <***@kluug.net> schrieb am Di., 3. Juli 2018, 22:57:

> On 03.07.2018 22:08, Florian KlÀmpfl wrote:
> > Am 03.07.2018 um 21:30 schrieb Ondrej Pokorny:
> >> On 03.07.2018 20:54, Florian KlÀmpfl wrote:
> >> program Project1;
> >> type
> >> TMyArray = array[0..10] of Integer;
> >> procedure Init2(var A: TMyArray);
> >> begin
> >> A[2] := 1;
> >> end;
> >> var
> >> A: TMyArray;
> >> begin
> >> Init2(A);
> >> end.
> >>
> >> should not emit the warning because A is not being read in Init2.
> >
> > (please note it throws only a hint in this case as A *is* initialized
> > as it is a global variable).
> >
> > FPC has no global data flow analysis (and I do not plan to do so yet),
> > so this is warning is currently far out of reach.
>
> Doing it for SetLength/FillChar would be a good start.
>

As Florian already wrote: FillChar() is nothing special. It's merely an
ordinary procedure with an untyped var parameter. The compiler can't make
any assumptions about what the code inside is doing with the parameter.

Regards,
Sven

>
Thorsten Engler
2018-07-04 04:18:08 UTC
Permalink
> > Probably I am the only one who thinks that the code below is
> ridiculous...
> >
> > procedure TExternalAssemblerOutputFile.AsmWriteFiltered(p:
> pchar;
> > len: longint);
> > var
> > s: ansistring;
> > begin
> > MaybeAddLinePrefix;
> > s:=''; // <<< ????
> > setlength(s,len);
> > move(p^,s[1],len);
> > AsmWriteAnsiStringUnfiltered(decorator.LineFilter(s));
> > end;
> >
> > ---
> >
> > I have split feelings: I really like to get the warning/hint at
> places
> > where it makes sense so I don't want to disable it globally. But
> the
> > amount of false positives increased significantly :/
>
> The compiler is ~400k lines and I had to fix maybe 40 locations, so I
> wouldn't call this significantly and there are situations where it
> matters, see the result case.

To get a well defined string of a specific length in a single statement:

s := StringOfChar(#0, len);

I'm not sure if SetLength is a special case in the compiler (like it is in Delphi) or just a plain function call that's normally resolved, but if it is a special case:

To get rid of unnecessary warnings for SetLength(x, 0) maybe the compiler could rewrite that into x := nil early on? That should also improve performance.

In fact, instead of generating the warning the compiler may even be able to just rewrite non-zero length calls into that StringOfChar statement.

_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://l
Ondrej Pokorny
2018-07-07 06:05:48 UTC
Permalink
On 03.07.2018 22:08, Florian KlÀmpfl wrote:
> Am 03.07.2018 um 21:30 schrieb Ondrej Pokorny:
>> On 03.07.2018 20:54, Florian KlÀmpfl wrote:
>> program Project1;
>> type
>>    TMyArray = array[0..10] of Integer;
>>    procedure Init2(var A: TMyArray);
>>    begin
>>      A[2] := 1;
>>    end;
>> var
>>    A: TMyArray;
>> begin
>>    Init2(A);
>> end.
>>
>> should not emit the warning because A is not being read in Init2.
>
> (please note it throws only a hint in this case as A *is* initialized
> as it is a global variable).

You are wrong in both of the 2 assumptions you made in your statement:

*1.) Assumption 1: a warning is thrown when A is not a global variable.
Wrong.*

program Project1;
type
   TMyArray = array[0..10] of Integer;
   procedure Init2(var A: TMyArray);
   begin
     A[2] := 1;
   end;
  procedure Test;
  var
    A: TMyArray;
  begin
    Init2(A); // << project1.lpr(12,12) Hint: Local variable "A" does
not seem to be initialized
  end;
begin
  Test;
end.

*2.) Assumption 2: A is not initialized if is a not a global variable.
Wrong.*

See the very first email in this thread and Sven Barth's reply:

On 24.03.2018 23:00, Sven Barth via fpc-devel wrote:
> Ondrej Pokorny <***@kluug.net <mailto:***@kluug.net>> schrieb
> am Sa., 24. MÀrz 2018, 20:49:
>
> Another question about this sentence in FPC docs: "Managed types are
> always initialized: in general this means setting the reference
> count to
> zero, or setting the pointer value of the type to Nil."
>
> Does it mean I can assume a local string variable is always (=whenever
> the routine is called) initialized to ''? I.e. that TestB always
> returns
> 'a'?
>
>
> For managed types this is indeed guaranteed. Otherwise there'd be
> exceptions upon the first use of the variable as the RTL would assume
> that the value is valid.
>
> Regards,
> Sven

Reference in your FPC docs:
https://www.freepascal.org/docs-html/ref/refse24.html

"Managed types are always initialized: in general this means setting the
reference count to zero, or setting the pointer value of the type to Nil."

-----

The fact that even you as the very first FPC developer are confused by
the warning/hint and initialization/noinitialization documentation mess
makes me feel I have been always right in this thread...

Yet all my arguments to clear this mess up were refused with the
statement that there is no reason to change anything in FPC.

Ondrej
Ondrej Pokorny
2018-07-07 06:18:45 UTC
Permalink
On 07.07.2018 08:05, Ondrej Pokorny wrote:
> On 03.07.2018 22:08, Florian KlÀmpfl wrote:
>> Am 03.07.2018 um 21:30 schrieb Ondrej Pokorny:
>>> On 03.07.2018 20:54, Florian KlÀmpfl wrote:
>>> program Project1;
>>> type
>>>    TMyArray = array[0..10] of Integer;
>>>    procedure Init2(var A: TMyArray);
>>>    begin
>>>      A[2] := 1;
>>>    end;
>>> var
>>>    A: TMyArray;
>>> begin
>>>    Init2(A);
>>> end.
>>>
>>> should not emit the warning because A is not being read in Init2.
>>
>> (please note it throws only a hint in this case as A *is* initialized
>> as it is a global variable).
>
> *2.) Assumption 2: A is not initialized if is a not a global variable.
> Wrong.*


I forgot to say: this is valid for managed types only. But all my
arguments are valid anyway. See local variables:

program Project1;
  procedure InitS(var S: string);
  begin
  end;
  procedure InitI(var I: Integer);
  begin
  end;
  procedure Test;
  var
    S: string;
    I: Integer;
  begin
    InitS(S); // << project1.lpr(13,12) Hint: Local variable "S" of a
managed type does not seem to be initialized
    InitI(I); // << project1.lpr(14,12) Hint: Local variable "I" does
not seem to be initialized
  end;
begin
  Test;
end.

program Project1;
  procedure InitS(var S: string);
  begin
  end;
  procedure InitI(var I: Integer);
  begin
  end;
var
  S: string;
  I: Integer;
begin
  InitS(S); // << project1.lpr(13,10) Hint: Variable "S" of a managed
type does not seem to be initialized
  InitI(I); // << project1.lpr(14,10) Hint: Variable "I" does not seem
to be initialized
end.

As you see, all messages are hints only, no warnings. Regardless of
managed/not-managed global/local variable.

Ondrej
Florian Klämpfl
2018-07-07 08:54:43 UTC
Permalink
Am 07.07.2018 um 08:18 schrieb Ondrej Pokorny:
>
> As you see, all messages are hints only, no warnings. Regardless of managed/not-managed global/local variable.
>

There are a lot of exceptions when which warning happens, try the same with e.g. writeln and the file parameter (just
one of these exceptions). SetLength is a case were a warning is valid because it can result in very bogus results (as
this thread has shown).
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mai
Ondrej Pokorny
2018-07-07 10:11:44 UTC
Permalink
On 07.07.2018 10:54, Florian Klämpfl wrote:
> Am 07.07.2018 um 08:18 schrieb Ondrej Pokorny:
>>
>> As you see, all messages are hints only, no warnings. Regardless of
>> managed/not-managed global/local variable.
>>
>
> There are a lot of exceptions when which warning happens, try the same
> with e.g. writeln and the file parameter (just one of these exceptions).

"A lot of exceptions" doesn't look like very systematic, logical and
orthogonal in the first place.

> SetLength is a case were a warning is valid because it can result in
> very bogus results (as this thread has shown).

I think you mean the "[fpc-devel] Managed Types, Undefined Bhaviour" thread.

"it can result in very bogus results" only in combination with the
special Result function variable. Never with a local or global variable
because they are guaranteed to be niled/zeroed (SetLength can be used on
managed types only). Therefore the warning would make sense only in
combination with function Result if your argument is used. With
local/global variable the warning is not valid because it can NOT result
in bogus results.

This applies to every other routine that takes a var parameter, not only
SetLength. I don't understand why SetLength should be special. See the
code below:

program Project1;
type
  Vector = array of integer;
procedure DoSetLength(var V: Vector; len: Integer);
begin
  SetLength(V, 4);
end;
function DoSomething1 (len: Integer): Vector;
begin
  DoSetLength(Result, len); // << HINT
end;
function DoSomething2 (len: Integer): Vector;
begin
  SetLength(Result, len); // << WARNING
end;
type
  TFoo = class
    A, B: Vector;
  end;
procedure Main;
var
  foo: TFoo;
begin
  foo := TFoo.Create;
  foo.A := DoSomething1(3); // try DoSomething2
  foo.A[0] := 42;
  foo.B := DoSomething1(4); // try DoSomething2
  Writeln(foo.B[0]);
end;
begin
  Main;
  Readln;
end.

Both DoSomething1 and DoSomething2 produce the same bogus code and yet
you decided that DoSomething1 deserves a hint and DoSomething2 deserves
a warning.

Earlier in this thread you wrote:

On 03.07.2018 20:54, Florian Klämpfl wrote:
> The warning happens also for any other call which takes an
> uninitialized variable as var parameter (see e.g. fillchar). So the
> warning increases only orthogonality of the language.

Which is not true because "any other call which takes an uninitialized
variable as var parameter (see e.g. fillchar)" shows a hint only, not a
warning:

procedure DoSomething;
var
  O, P: string;
begin
  SetLength(O, 1);   // << Warning
  FillChar(P, 1, 0); // << Hint
end;

I ask myself where is the orthogonality you wrote about?

On 07.07.2018 10:44, Florian Klämpfl wrote:
> Am 07.07.2018 um 08:05 schrieb Ondrej Pokorny:
>> makes me feel I have been always right in this thread...
>
> Fine, so we can close the case.

I agree. I officially stop trying to show you inconsistencies in your
documentation and the compiler message system.

I find it ironic that you use arguments for the current state of
documentation and compiler message system that - if you actually prove
them - show exactly the inconsistencies I point to and are against the
current state of documentation and compiler message system.

-----

Already in March/April this year I tried to convince the FPC team in
this thread that the rules for warning/hint of initialized variables
should be changed and that some warnings should be downgraded to notes.
Jonas always replied with a disapproval.

Let me sum up my POV the last time:

throw a WARNING: if bogus results can happen.
throw a HINT: if bogus results can NOT happen.

You have then these 4 categories:
- uninitialized unmanaged+managed Result variable: bogus => WARNING
- uninitialized local unmanaged variable (ordinals etc): bogus => WARNING
- uninitialized local managed variable (strings etc): OK => NOTE
- uninitialized unmanaged+managed global variable: OK => NOTE

Especially you may not differentiate how the variable is used
(right-side of the assignment, SetLength, other routine with var
parameter etc.).

The current state is a strange mix of exceptions and rules that are
irrational and not orthogonal.

To demonstrate my POV I made a small program that shows the current
state and my POV:

program Project1;
uses StrUtils;
procedure DoVar(var S: string);
begin
  S := S + S;
end;
procedure DoVar(var O: Integer);
begin
  O := O + O;
end;
procedure Main;
var
  S1, S2, S3: string;
  O1, O2: string;
begin
  S1 := S1 + S1;             // << warning, should be: note    (WRONG)
  SetLength(S2, 0);          // << warning, should be: note    (WRONG)
  DoVar(S3);                 // << note,    should be: note    (OK)
  O1 := O1 + O1;             // << warning, should be: warning (OK)
  DoVar(O2);                 // << note,    should be: warning (WRONG)
end;
function M1: string;
begin
  Result := Result + Result; // << warning, should be: warning (OK)
end;
function M2: string;
begin
  SetLength(Result, 0);      // << warning, should be: warning (OK)
end;
function M3: string;
begin
  DoVar(Result);             // << note,    should be: warning (WRONG)
end;
function U1: Integer;
begin
  Result := Result + Result; // << warning, should be: warning (OK)
end;
function U2: Integer;
begin
  DoVar(Result);             // << note,    should be: warning (WRONG)
end;

var
  GS1, GS2, GS3: string;
  GO1, GO2: string;
begin
  GS1 := GS1 + GS1;          // << warning, should be: note    (WRONG)
  SetLength(GS2, 0);         // << warning, should be: note    (WRONG)
  DoVar(GS3);                // << note,    should be: note    (OK)
  GO1 := GO1 + GO1;          // << warning, should be: note    (WRONG)
  DoVar(GO2);                // << note,    should be: note    (OK)
end.

This should be the basis without any (additional) exceptions.

(The exceptions I wrote before like SetLength/FillChar being ignored for
compiler messages etc. are still valid from the programmer's POV but
don't have to be applied to keep the system simple and "orthogonal".)

Ondrej
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailm
Florian Klämpfl
2018-07-07 08:44:00 UTC
Permalink
Am 07.07.2018 um 08:05 schrieb Ondrej Pokorny:
> makes me feel I have been always right in this thread...

Fine, so we can close the case.
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/
Ondrej Pokorny
2018-07-07 08:25:49 UTC
Permalink
This post might be inappropriate. Click to display it.
Alexander Grotewohl
2018-04-04 18:33:32 UTC
Permalink
https://www.freepascal.org/docs-html/ref/refse20.html#x50-680003.9


On 04/04/2018 01:32 PM, Ondrej Pokorny wrote:
> On 04.04.2018 18:53, Jonas Maebe wrote:
>> On 04/04/18 18:44, Ondrej Pokorny wrote:
>>> I want to stress that the compiler emits a warning on code that does
>>> not have (and also cannot have) an error
>>
>> An error is wrong code behaviour. If you do not initialise a variable
>> with the correct value, then you can have an error.
>
> No, no, no and again no. Do you try to convince me that I cannot rely
> on well documented compiler behavior? Even Sven stated earlier in this
> thread that the FPC docs are correct in case of managed variables:
> "Managed types are an exception to this rule: Managed types are always
> initialized: in general this means setting the reference count to
> zero, or setting the pointer value of the type to Nil." and "It should
> be stressed that initialized variables are initialized when they come
> into scope" https://www.freepascal.org/docs-html/ref/refse24.html
>
> Again, the code
> procedure Test;
> var S: string; I: Integer;
> begin
>   for I := 0 to 1 do
>     S := S + '1';
>   Writeln(S);
> end;
>
> does not have an error, cannot have an error and the behavior is
> guaranteed and well documented => no warning should be here.
>
> + Do you try to convince me that the code below can have an error and
> should emit a warning as well because it does not initialize the FS
> and FI fields?
>
> program Project1;
> type
>   TMyObject = class
>   private
>     FS: string;
>     FI: Integer;
>   public
>     property S: string read FS write FS;
>     property I: Integer read FI write FI;
>   end;
>
> begin
>   with TMyObject.Create do
>   begin
>     Writeln(S);
>     Writeln(I);
>     Free;
>   end;
> end.
>
> It's the same case - object fields are documented to be initialized.
> You mean I should not rely on this feature and it is an error that I
> did not initialize them with the correct values?
>
>> This correct value can be different from "empty string" or "nil".
>
> If I want to have a different value from "empty string" or "nil" I
> know I have to initialize it by myself. I don't need a warning for it.
>
>> For the same reason, we also warn about uninitialised global
>> variables (if this can be detected without interprocedural analysis,
>> i.e., if they are only used in the main program code; but that is
>> merely due to a limitation of the analysis).
>
> Uninitialised global variables are the same case: the compiler should
> not emit a warning here because again, it is well documented that they
> are implicitely initialized (at least in the Object Pascal Language
> Guide from Borland from 2002 it is documented).
>
> Ondrej
>
>
> _______________________________________________
> fpc-devel maillist - fpc-***@lists.freepascal.org
> http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel
Michael Van Canneyt
2018-03-25 11:07:56 UTC
Permalink
On Sat, 24 Mar 2018, Ondrej Pokorny wrote:

> On 24.03.2018 23:00, Sven Barth via fpc-devel wrote:
>> Ondrej Pokorny <***@kluug.net <mailto:***@kluug.net>> schrieb am
>> Sa., 24. MÀrz 2018, 20:49:
>>
>> This is not correct. Global simple variables are always
>> initialized. At
>> least in Delphi it is so:
>> http://docwiki.embarcadero.com/RADStudio/Tokyo/en/Variables_(Delphi)
>> <http://docwiki.embarcadero.com/RADStudio/Tokyo/en/Variables_%28Delphi%29>
>> "If
>> you do not explicitly initialize a global variable, the compiler
>> initializes it to 0."
>>
>>
>> So, Delphi *does* document it. Okay, in that case the documentation needs
>> to be updated, cause up to now the assumption has been that this is an
>> implementation detail.
>
> It has always been so in Delphi - at least since I learnt it. The Delphi 7
> docs (I can't find older docs online) state it as well:
> http://docs.embarcadero.com/products/rad_studio/cbuilder6/EN/CB6_ObjPascalLangGuide_EN.pdf
> on page 5-38 "If you don’t explicitly initialize a global variable, the
> compiler initializes it to 0." (Docs are at
> http://docs.embarcadero.com/products/rad_studio/ )

Please file a bugreport.

Michael.
Maciej Izak
2018-03-24 22:17:11 UTC
Permalink
2018-03-24 20:49 GMT+01:00 Ondrej Pokorny <***@kluug.net>:

> Does it mean I can assume a local string variable is always (=whenever the
> routine is called) initialized to ''? I.e. that TestB always returns 'a'?
>
> function TestB: string;
> var B: string;
> begin
> B := B + 'a';
> Result := B;
> end;
>
> Until now I though that the above code is unsafe and I always initialized
> all local variables (also managed) before use. I assumed the initialization
> to empty string does not need to happen at every routine call.
>

Please note that string/dynamic array as Result is exception (it works like
*var* parameter). IMO this is the ugliest Pascal behavior ever:

=== example begin ===
function test1: string;
begin
Result := Result + 'a';
end;

function test2: TArray<byte>;
begin
SetLength(Result, Length(Result) + 1);
end;

begin
writeln(test1); // print a
writeln(test1); // print aa
writeln(test1); // print aaa
writeln(test1); // print aaaa
writeln(Length(test2)); // print 1
writeln(Length(test2)); // print 2
writeln(Length(test2)); // print 3
writeln(Length(test2)); // print 4
end.
=== example end ===

--
Best regards,
Maciej Izak
Marco van de Voort
2018-04-05 11:55:03 UTC
Permalink
In our previous episode, Alexander Klenin said:
> > Simple, straightforward.
> >
> Allow me to yet again to single out this philosophy of
> strongly preferring abstract purity to concrete user experience.
> This is IMHO a significant contributing factor of Pascal decline.

No. Not having something that creates new users is the main reason.

IN-OUT<0

In general, the assumption that the popularity of a development product
is mainly dependent on language is already faulty.

> I would not argue this specific point (although I quite agree with Ondrej),
> I just want to remind that the cost in terms of diminishing userbase
> is real, and that makes me sad.

I'm with Michael.
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.fre
Ched
2018-04-05 14:13:42 UTC
Permalink
Hello,

Le 05.04.2018 à 11:55, Marco van de Voort a écrit :
> In our previous episode, Alexander Klenin said:
>>
>> Allow me to yet again to single out this philosophy of
>> strongly preferring abstract purity to concrete user experience.
>> This is IMHO a significant contributing factor of Pascal decline.
>
> No. Not having something that creates new users is the main reason.
>
> IN-OUT<0
>
> In general, the assumption that the popularity of a development product
> is mainly dependent on language is already faulty.

Hélas, the language which is currently gaining popularity is Python. Even Java is loosing popularity.
But Python is not compiled and not suited for large programs (my own opinion). I rest with my favourite
language : (Free)Pascal :)

https://www.tiobe.com/tiobe-index/
https://pypl.github.io/PYPL.html?country=FR

Cheers, Ched'
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://
Loading...