[m-rev.] for review: improve dir./

[Julien Fischer]

On Tue, 12 Sep 2023, Zoltan Somogyi wrote:

> On 2023-09-12 08:47 +10:00 AEST, "Julien Fischer" <jfischer at opturion.com> wrote:
>>> For review and trying out by Julien on Windows, though I would
>>> also like Peter's opinion on the questions below.
>>
>> An hlc.gc bootcheck on Cygwin succeeds and tests/hard_coded/dir_test
>> passes.
>>
>> An hlc.gc bootcheck with 64-bit MSVC succeeds, but
>> tests/hard_coded/dir_test fails with one point of different.
>> (The relevant part of the test log is attached.)
>>
>> (I'm still to test MinGW64, but I don't expect it to be any different
>> to the MSVC case.)
>
> Thanks for that. Can I take it that there are no review comments, or are those
> still to come?
>
> Is the test failure on 64 bit MSVC new, or did the test fail before this diff?
>
> If it is new, can you please step through the logic of dir./ and see which
> part of the condition of the if-then-else that selects between
>
> - adding a / between the dirname and the filename, and
> - not adding a /
>
> fails on MSVC, and why? Making a copy of the logic outside the library for simpler debugging.


I believe the issue is the following. It fixes my simplified test
case, I'm just installing a library with the fix and will check the
dir_test test case again.

> diff --git a/library/dir.m b/library/dir.m
> index a73fb42b8..b71bf320e 100644
> --- a/library/dir.m
> +++ b/library/dir.m

...

> @@ -868,43 +916,32 @@ dotnet_path_name_is_absolute_2(_) :-
>  %---------------------------------------------------------------------------%
>
>  DirName0/FileName0 = PathName :-
> -    DirName = string.from_char_list(canonicalize_path_chars(
> -        string.to_char_list(DirName0))),
> -    FileName = string.from_char_list(canonicalize_path_chars(
> -        string.to_char_list(FileName0))),
> -    ( if
> -        dir.path_name_is_absolute(FileName)
> -    then
> +    ( if is_path_string_canonical(DirName0) then
> +        DirName = DirName0
> +    else
> +        DirName = string.from_char_list(canonicalize_path_chars(
> +            string.to_char_list(DirName0)))
> +    ),
> +    ( if is_path_string_canonical(FileName0) then
> +        FileName = FileName0
> +    else
> +        FileName = string.from_char_list(canonicalize_path_chars(
> +            string.to_char_list(FileName0)))
> +    ),
> +    ( if dir.path_name_is_absolute(FileName) then
>          unexpected($pred, "second argument is absolute")
> -    else if
> -        % Check that FileName is not a relative path of the form "C:foo".
> -        use_windows_paths,
> -        Length = length(FileName),
> -        ( if Length >= 2 then
> -            char.is_alpha(string.unsafe_index(FileName, 0)),
> -            string.unsafe_index(FileName, 1) = (':'),
> -            ( if Length > 2 then
> -                not is_directory_separator(string.unsafe_index(FileName, 2))
> -            else
> -                true
> -            )
> -        else
> -            fail
> -        )
> -    then
> +    else if dir.path_name_is_cur_drive_relative(FileName) then
>          unexpected($pred, "second argument is a current drive relative path")
>      else if
> -        DirNameLength = length(DirName),
>          (
>              % Check for construction of relative paths of the form "C:foo".
> -            use_windows_paths,
> -            DirNameLength = 2,
> -            char.is_alpha(string.unsafe_index(DirName, 0)),
> -            string.unsafe_index(DirName, 1) = (':')
> +            path_name_names_drive_letter(FileName, ThirdCharIndex),
> +            not string.index_next(FileName, ThirdCharIndex, _, _)

The issue I encountered with MSVC is due to testing that FileName is
the beginning of a current drive relative path here instead of DirName

Julien.