std.experimental.make allows use to create instances of structs or instances of classes that are not known by the garbage collector.


However, I've recently observed that it could leads to severe bugs when the aggregate that's allocated with 'make()' contains at least one member that's managed by the GC. The issue is here: https://issues.dlang.org/show_bug.cgi?id=15790.


A solution would be, in 'make()', to statically determine if the aggregate contains arrays, pointers or classes. This is very simple, as seen for example in EMSI container library with the 'shouldAddGCRange' template. (https://github.com/economicmodeling/containers/blob/master/src/containers/internal/node.d#L59).


The problem with this template is that it returns false positives. For example an array will be seen as a managed type while actually it may be only modified by 'expandArray()' and 'shrinkArray()' with Mallocator.


The obvious solution is simply to allow to anotate variables with @nogc. But...if it's already accepted by the compiler, it cannot be detected by the built-in traits. Why ? Because '@nogc' is a function attribute and is detectable only with a trait that works on functions.


Finally my proposition is:
0. verify that front really take @nogc on a variable.
1. add a trait to get the non-UDA attributes of a variable.
2. add to phobos the 'shouldAddGCRange' template, but enhanced with the detection of the variables marked @nogc
3. update 'make()' so that it uses the enhanced 'shouldAddGCRange' to declare the content of the aggragate to the GC
4. close issue 15790.


I need your point of view about 0. & 1. Also I'm not a dmd commiter so someone will have to do it (i can do point 2 and 3). If you don't understand well what I'm trying to do, look at this paste, it's more or less what I'd like to add to phobos (except that currently it uses a UDA) https://dpaste.dzfl.pl/25a32c5cf106.

On Tuesday, 31 May 2016 at 13:33:03 UTC, Basile B. wrote:
> [...]
> A solution would be, in 'make()', to statically determine if the aggregate contains arrays, pointers or classes. This is very simple, as seen for example in EMSI container library with the 'shouldAddGCRange' template. [...]

Please also note well that this template will certainly be affected by the fact the trait 'getMember' has a serious issue related to the target visibility, as reported here:

https://issues.dlang.org/show_bug.cgi?id=15371

I "solved" it with a UDA called GcScan in my code. It can be attached to any field or type and is a tri-state. In the undecided case (GcScan.auto) it recursively scans for potential GC pointers, excluding those marked GcScan.no. In the decided case (GcScan.yes/no) it simply assumes the user knows better and short-circuits the recursion.

If I use a third party struct that contains pointers and does not use my GcScan UDA, but is known not to point to GC memory, I'd tag the outer level with @GcScan.no.

Memory allocation via malloc is also wrapped in a function (a typed allocator) that takes GcScan as a hint, so it may or may not add the freshly allocated memory as a GC range. Again, GcScan.auto would trigger auto-detection from the type.

-- 
Marco

On Tuesday, 31 May 2016 at 14:47:12 UTC, Marco Leise wrote:
> I "solved" it with a UDA called GcScan in my code. It can be attached to any field or type and is a tri-state. In the undecided case (GcScan.auto) it recursively scans for potential GC pointers, excluding those marked GcScan.no. In the decided case (GcScan.yes/no) it simply assumes the user knows better and short-circuits the recursion.
>
> [...]

This solution seems smarter than using the existing '@nogc' attribute. Plus one also for the fact that nothing has to be done in DMD.

Did you encounter the issue with protected and private members ?

For me when i've tested the template i've directly got some warnings. DMD interprets my 'getMember' calls as a deprecated abuse of bug 314 but in dmd 2.069 I would get true errors.

Am Tue, 31 May 2016 15:53:44 +0000
schrieb Basile B. <b2.temp@gmx.com>:

> This solution seems smarter than using the existing '@nogc' attribute. Plus one also for the fact that nothing has to be done in DMD.

I just constrained myself to what can be done in user code from the start. :)

> Did you encounter the issue with protected and private members ?
> 
> For me when i've tested the template i've directly got some warnings. DMD interprets my 'getMember' calls as a deprecated abuse of bug 314 but in dmd 2.069 I would get true errors.

Actually it is in a large half-ported code base from C++ and I haven't ever had a running executable, nor did I test it with recent dmd versions. My idea was to mostly have @nogc code, but allow it for a transition time or places where GC use does not have an impact. Here is the code, free to use for all purposes.


enum GcScan { no, yes, automatic }
enum noScan = GcScan.no;

template gcScanOf(T)
{
	import std.typetuple;

	static if (is(T == struct) || is(T == union))
	{
		enum isGcScan(alias uda) = is(typeof(uda) == GcScan);

		GcScan findGcScan(List...)()
		{
			auto result = GcScan.automatic;
			foreach (attr; List) if (is(typeof(attr) == GcScan))
				result = attr;
			return result;
		}

		enum gcScanOf()
		{
			auto result = GcScan.no;
			foreach (i; Iota!(T.tupleof.length))
			{
				enum memberGcScan = findMatchingUda!(T.tupleof[i], isGcScan, true);
				static if (memberGcScan.length == 0)
					enum eval = gcScanOf!(typeof(T.tupleof[i]));
				else
					enum eval = evalGcScan!(memberGcScan, typeof(T.tupleof[i]));

				static if (eval)
				{
					result = eval;
					break;
				}
			}
			return result;
		}
	}
	else
	{
		static if (isStaticArray!T && is(T : E[N], E, size_t N))
			enum gcScanOf = is(E == void) ? GcScan.yes : gcScanOf!E;
		else
			enum gcScanOf = hasIndirections!T ? GcScan.yes : GcScan.no;
	}
}

enum evalGcScan(GcScan gc, T) = (gc == GcScan.automatic) ? gcScanOf!T : gc;

template findMatchingUda(alias symbol, alias func, bool optional = false, bool multiple = false)
{
	import std.typetuple;

	enum symbolName = __traits(identifier, symbol);
	enum funcName   = __traits(identifier, func);

	template Filter(List...)
	{
		static if (List.length == 0)
			alias Filter = TypeTuple!();
		else static if (__traits(compiles, func!(List[0])) && func!(List[0]))
			alias Filter = TypeTuple!(List[0], Filter!(List[1 .. $]));
		else
			alias Filter = Filter!(List[1 .. $]);
	}

	alias filtered = Filter!(__traits(getAttributes, symbol));
	static assert(filtered.length <= 1 || multiple,
	              symbolName ~ " may only have one UDA matching " ~ funcName ~ ".");
	static assert(filtered.length >= 1 || optional,
	              symbolName ~ " requires a UDA matching " ~ funcName ~ ".");

	static if (multiple || optional)
		alias findMatchingUda = filtered;
	else static if (filtered.length == 1)
		alias findMatchingUda = filtered[0];
}

-- 
Marco

On Tuesday, 31 May 2016 at 19:04:39 UTC, Marco Leise wrote:
> Am Tue, 31 May 2016 15:53:44 +0000
> schrieb Basile B. <b2.temp@gmx.com>:
>
>> This solution seems smarter than using the existing '@nogc' attribute. Plus one also for the fact that nothing has to be done in DMD.
>
> I just constrained myself to what can be done in user code from the start. :)
>
>> Did you encounter the issue with protected and private members ?
>> 
>> For me when i've tested the template i've directly got some warnings. DMD interprets my 'getMember' calls as a deprecated abuse of bug 314 but in dmd 2.069 I would get true errors.
>
> Actually it is in a large half-ported code base from C++ and I haven't ever had a running executable, nor did I test it with recent dmd versions. My idea was to mostly have @nogc code, but allow it for a transition time or places where GC use does not have an impact. Here is the code, free to use for all purposes.

Thx for sharing the template. When using '.tupleof' instead of the traits 'allMember'/'getMember' there's no issue with the visibility, which is awesome. It means that the template can be proposed very quickly in phobos.

The only thing is that I'm not sure about is the tri-state and the recursion. I cannot find a case where it would be justified.

Am Tue, 31 May 2016 20:41:09 +0000
schrieb Basile B. <b2.temp@gmx.com>:

> The only thing is that I'm not sure about is the tri-state and the recursion. I cannot find a case where it would be justified.

The recursion is simply there to find pointers in nested structs and their GcScan annotations:

// A does not need scanning
struct A
{
    B b;
}

struct B
{
    @noScan void* p;
}

The tri-state may not be necessary, I don't remember my rationale there. I do use GcScan.automatic as the default in memory allocation for example with the option to force it to yes or no. It gives you more control, just in case.

-- 
Marco

On Tuesday, 31 May 2016 at 23:46:59 UTC, Marco Leise wrote:
> Am Tue, 31 May 2016 20:41:09 +0000
> schrieb Basile B. <b2.temp@gmx.com>:
>
>> The only thing is that I'm not sure about is the tri-state and the recursion. I cannot find a case where it would be justified.
>
> The recursion is simply there to find pointers in nested structs and their GcScan annotations:

- the "auto" is like if there's no annotation.
- the "yes" seems useless because there is no case where the scanner should fail to detect members that are managed by the GC. It's for this case that things are a bit vague.

Otherwise only the "no" remains.

So far I'll go for this: https://dpaste.dzfl.pl/e3023ba6a7e2
with another annotation type name, for example 'AddGcRange' or 'GcScan'.