<div dir="ltr"><div dir="ltr"> </div><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 13 Nov 2024 at 12:11, Zoltan Somogyi <<a href="mailto:zoltan.somogyi@runbox.com">zoltan.somogyi@runbox.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"> On 2024-11-12 15:54 +11:00 AEDT, "Zoltan Somogyi" <<a href="mailto:zoltan.somogyi@runbox.com" target="_blank">zoltan.somogyi@runbox.com</a>> wrote: > For review by anyone. Does anyone intend to review this, or answer its questions? </blockquote><div> </div><div>> 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.</div><div> </div><div>Julien. </div></div></div>