Eliminate some redundant target->filename conversions.

compiler/make.util.m:
    Many operations in this module that operate on filenames did not take
    those filenames as arguments; instead, they took an argument such as
    a target_file from which they *computed* the filename. This meant that
    any predicate that called more than one of these operations implicitly
    computed the filename more than once. This could be a problem, because

    - there are several predicates one can use to compute the filename, but
    - there is no guarantee that different operations use the same predicate.

    As a first step in fixing this, change the predicates that print
    filenames in progress or error messages to take those filenames
    as parameters. Delete one of them, target_file_error, because
    after this change, it would have become identical to the existing
    file_error predicate.

compiler/make.module_target.m:
    Require the callers of record_made_target to supply the filename
    as well as the target_file from which it is derived.

compiler/make.dependencies.m:
compiler/make.module_dep_file.m:
compiler/make.program_target.m:
    Compute the filename before calling the updated operations in make.util.m
    and/or make.module_target.m.

    Add "XXX MAKE_STREAM" to places in the code that operate on either
    implicit or badly-chosed explicit streams.
This commit is contained in:
Zoltan Somogyi
2023-06-01 18:17:40 +02:00
parent 422bef837b
commit 083d376e65
5 changed files with 66 additions and 98 deletions

View File

@@ -1407,6 +1407,7 @@ dependency_status(Globals, Dep, Status, !Info, !IO) :-
;
Dep = dep_target(Target),
Target = target_file(ModuleName, FileType),
get_make_target_file_name(Globals, $pred, Target, TargetFileName, !IO),
( if
( FileType = module_target_source
; FileType = module_target_track_flags
@@ -1416,8 +1417,9 @@ dependency_status(Globals, Dep, Status, !Info, !IO) :-
% .track_flags should already have been made, if required,
% so are also up-to-date.
ModuleTarget = module_target(module_target_source),
maybe_warn_up_to_date_target(Globals, $pred,
top_target_file(ModuleName, ModuleTarget), !Info, !IO),
TopTargetFile = top_target_file(ModuleName, ModuleTarget),
maybe_warn_up_to_date_target(Globals, TopTargetFile,
TargetFileName, !Info, !IO),
Status = deps_status_up_to_date
else if
DepStatusMap0 = !.Info ^ mki_dependency_status,
@@ -1446,10 +1448,9 @@ dependency_status(Globals, Dep, Status, !Info, !IO) :-
;
MaybeTimestamp = error(Error),
Status = deps_status_error,
get_make_target_file_name(Globals, $pred, Target,
TargetFileName, !IO),
string.format("** Error: file `%s' not found: %s\n",
[s(TargetFileName), s(Error)], ErrorMsg),
% XXX MAKE_STREAM
% Try to write this with one call to avoid
% interleaved output when doing parallel builds.
io.write_string(ErrorMsg, !IO)

View File

@@ -935,6 +935,7 @@ make_module_dependencies_no_fatal_error(Globals, OldOutputStream, ErrorStream,
% XXX Why are we ignoring all previously reported errors?
io.set_exit_status(0, !IO),
write_error_specs(ErrorStream, Globals, Specs, !IO),
% XXX MAKE_STREAM
% XXX Now that we pass ErrorStream to write_error_specs instead of
% setting the output stream to ErrorStream, this may now be redundant.
io.set_output_stream(OldOutputStream, _, !IO),
@@ -942,6 +943,10 @@ make_module_dependencies_no_fatal_error(Globals, OldOutputStream, ErrorStream,
list.foldl(make_info_add_module_and_imports_as_dep,
BurdenedModules, !Info),
MadeTarget = target_file(ModuleName, module_target_int3),
get_make_target_file_name(Globals, $pred,
MadeTarget, MadeTargetFileName, !IO),
% If there were no errors, write out the `.int3' file
% while we have the contents of the module. The `int3' file
% does not depend on anything else.
@@ -949,9 +954,8 @@ make_module_dependencies_no_fatal_error(Globals, OldOutputStream, ErrorStream,
% We already know FatalReadError is empty.
NonFatalErrors = ReadModuleErrors ^ rm_nonfatal_errors,
( if set.is_empty(NonFatalErrors) then
Target = target_file(ModuleName, module_target_int3),
maybe_make_target_message_to_stream(Globals, $pred, OldOutputStream,
Target, !IO),
maybe_make_target_message_to_stream(Globals, OldOutputStream,
MadeTargetFileName, !IO),
setup_checking_for_interrupt(CookieMSI, !IO),
DetectedGradeFlags = !.Info ^ mki_detected_grade_flags,
@@ -969,6 +973,7 @@ make_module_dependencies_no_fatal_error(Globals, OldOutputStream, ErrorStream,
MayBuild = may_build(_AllOptions, BuildGlobals),
% Printing progress to the current output stream
% preserves old behavior.
% XXX MAKE_STREAM
% XXX Our caller should pass us ProgressStream.
io.output_stream(ProgressStream, !IO),
make_int3_files(ProgressStream, ErrorStream, BuildGlobals,
@@ -988,8 +993,7 @@ make_module_dependencies_no_fatal_error(Globals, OldOutputStream, ErrorStream,
teardown_checking_for_interrupt(VeryVerbose, CookieWMDF,
CleanupWMDF, succeeded, _Succeeded, !Info, !IO),
MadeTarget = target_file(ModuleName, module_target_int3),
record_made_target(Globals, MadeTarget,
record_made_target(Globals, MadeTarget, MadeTargetFileName,
process_module(task_make_int3), Succeeded, !Info, !IO),
unredirect_output(Globals, ModuleName, ErrorStream, !Info, !IO).

View File

@@ -45,14 +45,15 @@
dependency_file::in, maybe_succeeded::out,
make_info::in, make_info::out, io::di, io::uo) is det.
% record_made_target(Globals, Target, Task, MakeSucceeded, !Info, !IO):
% record_made_target(Globals, TargetFile, TargetFileName, Task,
% MakeSucceeded, !Info, !IO):
%
% Record whether building a target succeeded or not.
% Makes sure any timestamps for files which may have changed
% in building the target are recomputed next time they are needed.
% Exported for use by make.module_dep_file.write_module_dep_file.
%
:- pred record_made_target(globals::in, target_file::in,
:- pred record_made_target(globals::in, target_file::in, file_name::in,
compilation_task_type::in, maybe_succeeded::in,
make_info::in, make_info::out, io::di, io::uo) is det.
@@ -188,6 +189,7 @@ make_module_target(ExtraOptions, Globals, Dep, Succeeded, !Info, !IO) :-
make_module_target_file_main_path(ExtraOptions, Globals, TargetFile,
CompilationTaskAndOptions, ModuleDepInfo, Succeeded, !Info, !IO) :-
TargetFile = target_file(ModuleName, TargetType),
get_make_target_file_name(Globals, $pred, TargetFile, TargetFileName, !IO),
CompilationTaskAndOptions = task_and_options(CompilationTaskType, _),
find_files_maybe_touched_by_task(Globals, TargetFile,
CompilationTaskType, TouchedTargetFiles, TouchedFiles, !Info, !IO),
@@ -229,8 +231,6 @@ make_module_target_file_main_path(ExtraOptions, Globals, TargetFile,
debug_make_msg(Globals,
( pred(!.IO::di, !:IO::uo) is det :-
get_make_target_file_name(Globals, $pred,
TargetFile, TargetFileName, !IO),
dependency_file_index_set_to_plain_set(!.Info,
DepFiles0, DepFilesPlainSet),
list.map_foldl(dependency_file_to_file_name(Globals),
@@ -277,8 +277,8 @@ make_module_target_file_main_path(ExtraOptions, Globals, TargetFile,
ExtraOptions, Succeeded, !Info, !IO)
;
DepsResult = deps_up_to_date,
maybe_warn_up_to_date_target(Globals, $pred,
top_target_file(ModuleName, module_target(TargetType)),
TopTargetFile = top_target_file(ModuleName, module_target(TargetType)),
maybe_warn_up_to_date_target(Globals, TopTargetFile, TargetFileName,
!Info, !IO),
debug_file_msg(Globals, TargetFile, "up to date", !IO),
Succeeded = succeeded,
@@ -382,7 +382,8 @@ build_target(Globals, CompilationTask, TargetFile, ModuleDepInfo,
!Info, !IO) :-
% XXX MAKE_FILENAME Either our caller should be able to give us
% TargetFileName, or we could compute it here, and give it to code below.
maybe_make_target_message(Globals, $pred, TargetFile, !IO),
get_make_target_file_name(Globals, $pred, TargetFile, TargetFileName, !IO),
maybe_make_target_message(Globals, TargetFileName, !IO),
TargetFile = target_file(ModuleName, _TargetType),
CompilationTask = task_and_options(Task, TaskOptions),
ExtraAndTaskOptions = ExtraOptions ++ TaskOptions,
@@ -453,7 +454,8 @@ build_target(Globals, CompilationTask, TargetFile, ModuleDepInfo,
teardown_checking_for_interrupt(VeryVerbose, Cookie, Cleanup,
Succeeded0, Succeeded, !Info, !IO),
record_made_target_given_maybe_touched_files(Globals, Succeeded,
TargetFile, TouchedTargetFiles, TouchedFiles, !Info, !IO),
TargetFile, TargetFileName, TouchedTargetFiles, TouchedFiles,
!Info, !IO),
get_real_milliseconds(Time, !IO),
globals.lookup_bool_option(Globals, show_make_times, ShowMakeTimes),
@@ -462,10 +464,6 @@ build_target(Globals, CompilationTask, TargetFile, ModuleDepInfo,
DiffSecs = float(Time - Time0) / 1000.0,
% Avoid cluttering the screen with short running times.
( if DiffSecs >= 0.5 then
% XXX MAKE_FILENAME The code above should be able to give us
% TargetFileName.
get_make_target_file_name(Globals, $pred,
TargetFile, TargetFileName, !IO),
io.format("Making %s took %.2fs\n",
[s(TargetFileName), f(DiffSecs)], !IO)
else
@@ -476,6 +474,7 @@ build_target(Globals, CompilationTask, TargetFile, ModuleDepInfo,
)
;
ArgFileNameRes = error(ArgFileError),
% XXX MAKE_STREAM
io.format(stderr_stream, "Could not create temporary file: %s\n",
[s(error_message(ArgFileError))], !IO),
Succeeded = did_not_succeed
@@ -506,7 +505,7 @@ cleanup_files(Globals, MaybeArgFileName, TouchedTargetFiles, TouchedFiles,
build_target_2(ModuleName, Task, ArgFileName, ModuleDepInfo, Globals,
AllOptionArgs, ErrorStream, Succeeded, !Info, !IO) :-
% XXX STREAM Printing progress messages to the current output stream
% XXX MAKE_STREAM Printing progress messages to the current output stream
% is an attempt to preserve old behavior.
io.output_stream(ProgressStream, !IO),
(
@@ -520,6 +519,7 @@ build_target_2(ModuleName, Task, ArgFileName, ModuleDepInfo, Globals,
% XXX Don't write the default options.
AllArgStrs = list.map(quote_shell_cmd_arg, AllArgs),
AllArgsStr = string.join_list(" ", AllArgStrs),
% XXX MAKE_STREAM
io.format("Invoking self `mmc %s'\n", [s(AllArgsStr)], !IO)
;
Verbose = no
@@ -778,26 +778,29 @@ target_is_java :-
%---------------------------------------------------------------------------%
record_made_target(Globals, TargetFile, CompilationTask, Succeeded,
!Info, !IO) :-
record_made_target(Globals, TargetFile, TargetFileName,
CompilationTask, Succeeded, !Info, !IO) :-
find_files_maybe_touched_by_task(Globals, TargetFile, CompilationTask,
TouchedTargetFiles, TouchedFiles, !Info, !IO),
record_made_target_given_maybe_touched_files(Globals, Succeeded,
TargetFile, TouchedTargetFiles, TouchedFiles, !Info, !IO).
TargetFile, TargetFileName, TouchedTargetFiles, TouchedFiles,
!Info, !IO).
:- pred record_made_target_given_maybe_touched_files(globals::in,
maybe_succeeded::in, target_file::in, list(target_file)::in,
list(file_name)::in, make_info::in, make_info::out, io::di, io::uo) is det.
maybe_succeeded::in, target_file::in, string::in,
list(target_file)::in, list(file_name)::in,
make_info::in, make_info::out, io::di, io::uo) is det.
record_made_target_given_maybe_touched_files(Globals, Succeeded, TargetFile,
TouchedTargetFiles, OtherTouchedFiles, !Info, !IO) :-
record_made_target_given_maybe_touched_files(Globals, Succeeded,
TargetFile, TargetFileName, TouchedTargetFiles, OtherTouchedFiles,
!Info, !IO) :-
(
Succeeded = succeeded,
TargetStatus = deps_status_up_to_date
;
Succeeded = did_not_succeed,
TargetStatus = deps_status_error,
target_file_error(!.Info, Globals, TargetFile, !IO)
file_error(!.Info, TargetFileName, !IO)
),
list.foldl(update_target_status(TargetStatus), TouchedTargetFiles, !Info),

View File

@@ -594,7 +594,10 @@ build_linked_target_2(Globals, MainModuleName, FileType, OutputFileName,
Succeeded = did_not_succeed
;
DepsResult = deps_up_to_date,
MsgTarget = top_target_file(MainModuleName, linked_target(FileType)),
MainModuleLinkedTarget =
top_target_file(MainModuleName, linked_target(FileType)),
linked_target_file_name(Globals, MainModuleName, FileType,
MainModuleLinkedFileName, !IO),
globals.lookup_bool_option(NoLinkObjsGlobals, use_grade_subdirs,
UseGradeSubdirs),
(
@@ -605,16 +608,17 @@ build_linked_target_2(Globals, MainModuleName, FileType, OutputFileName,
(
MadeSymlinkOrCopy = yes,
maybe_symlink_or_copy_linked_target_message(NoLinkObjsGlobals,
$pred, MsgTarget, !IO)
MainModuleLinkedFileName, !IO)
;
MadeSymlinkOrCopy = no,
maybe_warn_up_to_date_target(NoLinkObjsGlobals, $pred,
MsgTarget, !Info, !IO)
maybe_warn_up_to_date_target(NoLinkObjsGlobals,
MainModuleLinkedTarget, MainModuleLinkedFileName,
!Info, !IO)
)
;
UseGradeSubdirs = no,
maybe_warn_up_to_date_target(NoLinkObjsGlobals, $pred,
MsgTarget, !Info, !IO),
maybe_warn_up_to_date_target(NoLinkObjsGlobals,
MainModuleLinkedTarget, MainModuleLinkedFileName, !Info, !IO),
Succeeded = succeeded
)
;

View File

@@ -198,22 +198,17 @@
% Write a message "Making <filename>" if `--verbose-make' is set.
%
:- pred maybe_make_target_message(globals::in, string::in, target_file::in,
:- pred maybe_make_target_message(globals::in, file_name::in,
io::di, io::uo) is det.
:- pred maybe_make_target_message_to_stream(globals::in, string::in,
io.text_output_stream::in, target_file::in, io::di, io::uo) is det.
:- pred maybe_make_target_message_to_stream(globals::in,
io.text_output_stream::in, file_name::in, io::di, io::uo) is det.
% Write a message "Reanalysing invalid/suboptimal modules" if
% `--verbose-make' is set.
%
:- pred maybe_reanalyse_modules_message(globals::in, io::di, io::uo) is det.
% Write a message "** Error making <filename>".
%
:- pred target_file_error(make_info::in, globals::in, target_file::in,
io::di, io::uo) is det.
% Write a message "** Error making <filename>".
%
:- pred file_error(make_info::in, file_name::in, io::di, io::uo) is det.
@@ -221,15 +216,14 @@
% If the given target was specified on the command line, warn that it
% was already up to date.
%
:- pred maybe_warn_up_to_date_target(globals::in, string::in,
top_target_file::in, make_info::in, make_info::out,
io::di, io::uo) is det.
:- pred maybe_warn_up_to_date_target(globals::in, top_target_file::in,
file_name::in, make_info::in, make_info::out, io::di, io::uo) is det.
% Write a message "Made symlink/copy of <filename>"
% if `--verbose-make' is set.
%
:- pred maybe_symlink_or_copy_linked_target_message(globals::in, string::in,
top_target_file::in, io::di, io::uo) is det.
:- pred maybe_symlink_or_copy_linked_target_message(globals::in, file_name::in,
io::di, io::uo) is det.
%---------------------------------------------------------------------------%
%
@@ -270,7 +264,6 @@
:- import_module int.
:- import_module io.file.
:- import_module map.
:- import_module require.
:- import_module set.
:- import_module string.
:- import_module uint.
@@ -991,17 +984,14 @@ maybe_make_linked_target_message(Globals, FileName, !IO) :-
io.write_string(Msg, !IO)
), !IO).
maybe_make_target_message(Globals, From, TargetFile, !IO) :-
maybe_make_target_message(Globals, FileName, !IO) :-
io.output_stream(OutputStream, !IO),
maybe_make_target_message_to_stream(Globals, From, OutputStream,
TargetFile, !IO).
maybe_make_target_message_to_stream(Globals, OutputStream, FileName, !IO).
maybe_make_target_message_to_stream(Globals, From, OutputStream,
TargetFile, !IO) :-
maybe_make_target_message_to_stream(Globals, OutputStream, FileName, !IO) :-
verbose_make_msg(Globals,
( pred(!.IO::di, !:IO::uo) is det :-
get_make_target_file_name(Globals, From, TargetFile,
FileName, !IO),
% XXX MAKE_STREAM
% Try to write this with one call to avoid interleaved output
% when doing parallel builds.
string.format("Making %s\n", [s(FileName)], Msg),
@@ -1011,36 +1001,23 @@ maybe_make_target_message_to_stream(Globals, From, OutputStream,
maybe_reanalyse_modules_message(Globals, !IO) :-
verbose_make_msg(Globals,
( pred(!.IO::di, !:IO::uo) is det :-
% XXX MAKE_STREAM
io.output_stream(OutputStream, !IO),
io.write_string(OutputStream,
"Reanalysing invalid/suboptimal modules\n", !IO)
), !IO).
target_file_error(Info, Globals, TargetFile, !IO) :-
with_locked_stdout(Info,
write_make_target_file_wrapped(Globals,
"** Error making `", TargetFile, "'.\n"), !IO).
file_error(Info, FileName, !IO) :-
string.format("** Error making `%s'.\n", [s(FileName)], Msg),
% XXX MAKE_STREAM
with_locked_stdout(Info, io.write_string(Msg), !IO).
:- pred write_make_target_file_wrapped(globals::in,
string::in, target_file::in, string::in, io::di, io::uo) is det.
write_make_target_file_wrapped(Globals, Prefix, TargetFile, Suffix, !IO) :-
get_make_target_file_name(Globals, $pred, TargetFile, FileName, !IO),
% Our one caller just above never passes empty Prefix or Suffix.
io.write_string(Prefix ++ FileName ++ Suffix, !IO).
file_error(Info, TargetFile, !IO) :-
with_locked_stdout(Info,
io.write_string("** Error making `" ++ TargetFile ++ "'.\n"), !IO).
maybe_warn_up_to_date_target(Globals, From, Target, !Info, !IO) :-
maybe_warn_up_to_date_target(Globals, Target, FileName, !Info, !IO) :-
globals.lookup_bool_option(Globals, warn_up_to_date, Warn),
CmdLineTargets0 = !.Info ^ mki_command_line_targets,
(
Warn = yes,
( if set.member(Target, CmdLineTargets0) then
module_or_linked_target_file_name(Globals, From,
Target, FileName, !IO),
io.format("** Nothing to be done for `%s'.\n", [s(FileName)], !IO)
else
true
@@ -1051,33 +1028,12 @@ maybe_warn_up_to_date_target(Globals, From, Target, !Info, !IO) :-
set.delete(Target, CmdLineTargets0, CmdLineTargets),
!Info ^ mki_command_line_targets := CmdLineTargets.
maybe_symlink_or_copy_linked_target_message(Globals, From, Target, !IO) :-
maybe_symlink_or_copy_linked_target_message(Globals, FileName, !IO) :-
verbose_make_msg(Globals,
( pred(!.IO::di, !:IO::uo) is det :-
module_or_linked_target_file_name(Globals, From, Target,
FileName, !IO),
io.format("Made symlink/copy of %s\n", [s(FileName)], !IO)
), !IO).
:- pred module_or_linked_target_file_name(globals::in, string::in,
top_target_file::in, string::out, io::di, io::uo) is det.
module_or_linked_target_file_name(Globals, From, TopTargetFile,
FileName, !IO) :-
TopTargetFile = top_target_file(ModuleName, TargetType),
(
TargetType = module_target(ModuleTargetType),
TargetFile = target_file(ModuleName, ModuleTargetType),
get_make_target_file_name(Globals, From, TargetFile, FileName, !IO)
;
TargetType = linked_target(LinkedTargetType),
linked_target_file_name(Globals, ModuleName, LinkedTargetType,
FileName, !IO)
;
TargetType = misc_target(_),
unexpected($pred, "misc_target")
).
%---------------------------------------------------------------------------%
%
% Timing.