[m-rev.] for review: io.stream_db.m

Zoltan Somogyi zoltan.somogyi at runbox.com
Fri Mar 11 17:25:02 AEDT 2022


2022-03-11 16:14 GMT+11:00 "Peter Wang" <novalazy at gmail.com>:
>> --- a/browser/browse.m
>> +++ b/browser/browse.m
>> @@ -190,6 +190,7 @@
>>  :- import_module int.
>>  :- import_module io.call_system.
>>  :- import_module io.environment.
>> +:- import_module io.stream_db.
>>  :- import_module io.file.
> 
> Sort that.

Done.

>> diff --git a/library/io.m b/library/io.m
>> index 59da37102..d47279af4 100644
>> --- a/library/io.m
>> +++ b/library/io.m
> 
>> -
>> -:- func get_stream_id(stream) = stream_id.
>> -
>> -:- pragma foreign_proc("C",
>> -    get_stream_id(Stream::in) = (Id::out),
>> -    [will_not_call_mercury, promise_pure, thread_safe,
>> -        does_not_affect_liveness, no_sharing],
>> -"
>> -#ifndef MR_NATIVE_GC
>> -    // Most of the time, we can just use the pointer to the stream
>> -    // as a unique identifier.
>> -    Id = (MR_Word) Stream;
>> -#else
>> -    // For accurate GC we embed an ID in the MercuryFile
>> -    // and retrieve it here.
>> -    Id = (Stream)->id;
>> -#endif
>> -").
>> -
>> -:- pragma foreign_proc("C#",
>> -    get_stream_id(Stream::in) = (Id::out),
>> -    [will_not_call_mercury, promise_pure],
>> -"
>> -    Id = Stream.id;
>> -").
>> -
>> -:- pragma foreign_proc("Java",
>> -    get_stream_id(Stream::in) = (Id::out),
>> -    [will_not_call_mercury, promise_pure, may_not_duplicate],
>> -"
>> -    Id = Stream.id;
>> -").
>> -
> 
> I would leave get_stream_id in io.m along with the definitions of the
> stream type, and other stream_id stuff.

get_stream_id is called only from io.stream_db.m, and nowhere else.
If the id field of streams had another use besides being the key in the
stream db, I would agree with you, but it does not.

Julien, would you care to break the tie?

>> diff --git a/library/io.stream_db.m b/library/io.stream_db.m
>> index e69de29bb..c7cd1d7a0 100644
>> --- a/library/io.stream_db.m
>> +++ b/library/io.stream_db.m
> ...
>> +:- pred lock_stream_db(io::di, io::uo) is det.
>> +
>> +:- pragma foreign_proc("C",
>> +    lock_stream_db(_IO0::di, _IO::uo),
>> +    [will_not_call_mercury, promise_pure, thread_safe, tabled_for_io,
>> +        no_sharing],
>> +"
>> +    MR_LOCK(&ML_io_stream_db_lock, ""io.lock_stream_db/2""); ").
> 
> Fix the formatting.

Done.
 
> The rest looks fine.

Thank you.

Zoltan.


More information about the reviews mailing list