[m-rev.] for review: start on replacing --intermod-directories and related options

[Julien Fischer]

On Wed, 13 Nov 2024 at 12:11, Zoltan Somogyi <zoltan.somogyi at runbox.com>
wrote:

>
> On 2024-11-12 15:54 +11:00 AEDT, "Zoltan Somogyi" <
> zoltan.somogyi at runbox.com> wrote:
> > For review by anyone.
>
> Does anyone intend to review this, or answer its questions?
>

> This diff does not include user-facing documentation of the
> new options, but it does describe how they are intended to operate
> in the code. Once there is agreement on the approach, I will
>
> - implement the approach to replace the related options, such as
>  --search-directories, and
>
> - add user-facing documentation of the new options, both in
>   compiler/options.m and doc/user_guide.texi.
>
> The questions I am seeking particular feedback on are these.
>
> First, this diff classifies directories in search paths into three
categories:
> (a) workspace directories that share the same subdir setting, i.e they use
> the same values of --use-subdirs and --use-grade-subdirs, (b) workspace
> directories that may use a different subdir setting, and (c) installed
library
> directories. The current code assumes that all directories in category (a)
> come before all directories in categories (b) and (c), and all directories
> in category (b) come before all directories in category (c). This is
always
> true in the compiler and in all the other Mercury programs I have worked
on,
> but you guys work on a wider variety of programs. Is this assumption
> justifiable, or should I switch to an approach that does not make any
> such assumptions?

Every program I work on, other than the Mercury implementation itself, uses
either (1) installed libraries or (2) just uses source file mapping to build
all of the program from a single build directory.  The main use case I would
have for using non-installed libraries like the above is for testing
libraries
I have written in situ. For that case the above assumptions are justifiable.

> Second, can anyone think of better names for the new options?
> Is the use of "same", "indep" and "install" to describe the above three
> categories understandable (once documented), or does anyone
> know a better naming scheme?

Nothing springs to mind.

> Third, the code of the get_ext_opt_deps and get_plain_trans_opt_deps
> predicates in write_deps_file search for source files in directories named
> by the --intermodule-directories option, when their caller passes them
> look_for_src. I do not see any documentation of what the intention behind
> this arrangement is. If and when the code finds the source file, it does
nothing
> with it; it just stops looking for that module's .opt or .trans_opt file.
> This behavior was introduced by Simon in a commit on 6 nov 1996,
> but the log message is not informative, and we don't even have mailing
list
> archives from that far back. So my question is: does anyone have any idea
> of what that code is doing or why it is doing it, and what would be the
> impact of deleting that search?

Conceivably, there may have been some notion of finding clauses for
intermodule
inlining in the source files installed in those directories (i.e. rather
than
putting them directly in .opt and .trans_opt files).  Regardless, it does
not
seem to be a useful behaviour *now*.


> Start on search paths for PROPOSED dir structures.
>
> The new code added by this diff is not yet used.

...


> @@ -2962,7 +2964,95 @@ handle_directory_options(OpMode, !Globals) :-
>              accumulating(SubdirCIncludeDirs), !Globals)

...

> +:- pred make_proposed_search_path_gs(subdir_setting::in, dir_name::in,
> +    dir_name::in,
> +    list(dir_name)::in, list(dir_name)::in, list(dir_name)::in,
> +    list(dir_name)::out) is det.
> +
> +make_proposed_search_path_gs(SubdirSetting, Grade, ExtSubDir,
> +        SearchDirsSame, SearchDirsIndep, SearchDirsInstall, Dirs) :-
> +    % First, we search SearchDirsSame, which should be the workspace
> +    % directories that are guaranteed to share the same subdir_setting
> +    % as the currrent workspace directory. In each of these directories,

s/currrent/current/


> +    % we use SubdirSetting to select the one right subdir.
> +    list.map(
> +        make_selected_proposed_dir_name_gs(SubdirSetting, Grade,
ExtSubDir),
> +        SearchDirsSame, DirsSame),
> +    % Second, we search SearchDirsIndept, which should be workspace
directories
> +    % whose subdir_setting is independent of the subdir_setting of this
> +    % workspace directory. In these places, we have to be search all
three
> +    % of the subdis where that independent subdir_setting can put the
values

s/subdis/subdirs/

> +    % of this extension, searching them in the order grade-specific,
> +    % non-grade-specific, and then the current directory ("current" in
this
> +    % case meaning one of the directories in SearchDirsIndep).
> +    list.map(
> +        make_all_proposed_dir_names_gs(Grade, ExtSubDir),
> +        SearchDirsIndep, DirsListIndep),
> +    % Third, we search SearchDirsInstall, which should be a list of
library
> +    % install directories. In install directories, we look only in the
> +    % grade-specific subdir.
> +    list.map(
> +        make_selected_proposed_dir_name_gs(use_cur_ngs_gs_subdir, Grade,
> +            ExtSubDir),
> +        SearchDirsInstall, DirsInstall),
> +    list.condense(DirsListIndep, DirsIndep),
> +    Dirs = DirsSame ++ DirsIndep ++ DirsInstall.

That looks fine otherwise.

Julien.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20241113/dd22c749/attachment-0001.html>