make function symbols store function parameters, needed for UFCS.

[vushu]

To not rely on callTips and properly support UFCS, we need functions to store their applicable argument symbols, this PR adds does this.

[WebFreak001]

seems useful, but need to check again why you always set includeParameterSymbols there. Do you actually always need it or is that a left-over from debugging? In the cases where we want it we can simply set the value to true in the constructor, instead of forcing it to users. Alternatively if you can measure that there is no to little overhead in always including them (and posting your results on that) we can simply remove the boolean like I suggested in the first comment.

[WebFreak001]

this should not be here (it effectively now makes the constructor parameter useless, as parameters would always be included)

If it's never a problem, consider instead removing the symbol completely, deprecating the ctor and stripping all the ifs that checked on it

[WebFreak001]

mixed tabs and spaces

[WebFreak001]

mixed tabs and spaces

[vushu]

@WebFreak001 indeed looks like we probably can remove the includeParameterSymbols if it always should be true.

I have looked into the changes and ran the default tests in DCD before and after the changes.

before the change:

one run:

real	0m12.235s
user	0m0.905s
sys	0m0.388s

10 runs:

real	2m1.003s
user	0m8.368s
sys	0m3.226s

after the change:

one run:

real	0m12.323s
user	0m0.970s
sys	0m0.397s

10 runs:

real	2m3.326s
user	0m9.289s
sys	0m4.732s

Doesn't seem like it affects too much.

[maxhaton]

Does it need to be an array of pointers?

[vushu]

I guess so since DSymbol isn't copyable unless we change that.

struct DSymbol
{
	// Copying is disabled
	@disable this();
	@disable this(this);
,,,

[WebFreak001]

usually all the DSymbol references are done by pointers, but it might be effective to use them inline in arrays (e.g. move them into the array)

We already know the count of items that get put into array before putting them there anyway, so we can already preallocate all the space and directly construct them in-place.

[WebFreak001]

@WebFreak001 indeed looks like we probably can remove the includeParameterSymbols if it always should be true.

I have looked into the changes and ran the default tests in DCD before and after the changes.

before the change:

one run:

real	0m12.235s
user	0m0.905s
sys	0m0.388s

10 runs:

real	2m1.003s
user	0m8.368s
sys	0m3.226s

after the change:

one run:

real	0m12.323s
user	0m0.970s
sys	0m0.397s

10 runs:

real	2m3.326s
user	0m9.289s
sys	0m4.732s

Doesn't seem like it affects too much.

performance wise it looks ok.

Did you also check memory impact opening a Phobos or DMD source file and checking process memory usage completing a single symbol? That would be particularly interesting I think.

Otherwise LGTM to me now.

[WebFreak001]

this line here (below the deleted one) was also part of the deleted comment, also delete this line.

[vushu]

@WebFreak001, Right good point. I made a test where I measure the memory after a completion:

client test:
./dcd-client -c191 -I"/usr/include/dmd/phobos" test.d

test.d:

import std.algorithm;
import std.container;
import std.array;
import std.algorithm.iteration;
import std.regex;
import std.functional;
import std.string;
import std.stdio;
void main()
{
    s
}

where cursor is on s

I have measured the memory using pmap

dcd-server memory usage:

before change
total 1186852K

after change
total 1191868K

so an increase of 5mb

Test opening dmd/compiler/src/dmd/cppmangle.d, then make it complete on the letter s

before change
total 1069360K

after change:
total 1114172K

so an increase of 44mb

The data increase is expected since we store more information now. On big files like cppmangle.d which today
has 2578 lines of code the memory usage will be more significantly, but I think that is expected when opening such big files.