www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 11240] New: assumeSafeAppend could implicitly break immutablity

reply d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=11240

           Summary: assumeSafeAppend could implicitly break immutablity
           Product: D
           Version: D2
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: major
          Priority: P2
         Component: druntime
        AssignedTo: nobody puremagic.com
        ReportedBy: k.hara.pg gmail.com



I think assumeSafeAppend should reject array references which has non-mutable
element type.

Test case:

import std.stdio;
void main()
{
    immutable(int[]) arr = [1,2,3];
    immutable(int)[] a = arr[0..2];

    writeln("a.capacity = ", a.capacity);   // == 0
    a = assumeSafeAppend(a[]);
    writeln("a.capacity = ", a.capacity);   // != 0

    a ~= 100;

    writeln(a);     // [1, 2, 100]
    writeln(arr);   // [1, 2, 100] <-- !!
}

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Oct 12 2013
next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=11240


monarchdodra gmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |monarchdodra gmail.com



Is this valid though?

assumeSafeAppend is an unsafe function that *requires* no one else have a view
on the items after the end of the array.

Just the same, you will overwrite the old items, without destroying them, nor
assigning over them.

I think this is just an unsafe function that was used wrong. The result is
simply undefined behavior.

I think it would be a needless restriction to not allow assumeSafeAppend on
immutable (and const).

This seems invalid to me.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Oct 13 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=11240






 Is this valid though?
 
 assumeSafeAppend is an unsafe function that *requires* no one else have a view
 on the items after the end of the array.
 
 Just the same, you will overwrite the old items, without destroying them, nor
 assigning over them.
 
 I think this is just an unsafe function that was used wrong. The result is
 simply undefined behavior.
 
 I think it would be a needless restriction to not allow assumeSafeAppend on
 immutable (and const).
 
 This seems invalid to me.


Because the unsafe-ness is hidden in assumeSafeAppend function template. If you
pass immubtale(int)[] array reference to it in generic code, it could easily
break type-system silently (And yes, I didn't noticed the risk until now). As
the API design, it would be better to reject such a misuse.

In other words, if you really wants to charge the capacity of immutable(int)[],
enforcing explicit cast on caller side would be better.

    immutable(int)[] a = ...;
    //a = assubeSafeAppend(a);    // compile error
    a = cast(typeof(a))aassubeSafeAppend(cast(int[])a);  // ugly, but explicit

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Oct 13 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=11240


Jonathan M Davis <jmdavisProg gmx.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jmdavisProg gmx.com



PDT ---
I concur with monarchdodra. I think that this would be too strong a
restriction. assumeSafeAppend is already  system. You're already not supposed
to be calling it on an array that isn't actually safe to append to. The
constness of the array doesn't change that. Maybe more explicit warnings should
be put in the documentation for assumeSafeAppend, but the documentation is
already pretty clear that you shouldn't use it if it's not actually safe to
append.

I think that if assumeSafeAppend is misused, it's clearly a bug on the part of
the caller and not assumeSafeAppend, even if that bug involves const or
immutable. And given that the most likely type to use with assumeSafeAppend is
probably string, restricting it to only work with mutable elements would be
really reduce its usefulness.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Oct 13 2013
prev sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=11240




PDT ---
I think that making it so that assumeSafeAppend didn't work with const or
immutable would be akin to making free not work with const or immutable. Both
function are inherently unsafe and must be used correctly and could cause
serious issues if misused, and both are related to indicating which memory can
be used. But both are also just fine if used correctly. The key thing is in
making sure that it's clear in the documentation how the function is intented
to be used and that using it differently is unsafe.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Oct 13 2013