[m-rev.] for review: Fix --track-flags.

Peter Wang novalazy at gmail.com
Thu Jun 3 15:28:39 AEST 2021


Commit 181ada0dbf2899b388ae9155083a3050dd027cc1 changed many options
from normal options to special options. This revealed a bug in the
implementation of --track-flags. We previously computed the hash of a
list of (option, option_data) pairs to detect if options differ from a
previous run. However, for special options, the option_data is always
bool_special, int_special, etc. so whatever value those options
were set to had no effect on the hash value.

compiler/make.m:
    Include the opt_tuple table when computing the hash for
    `.track_flags' files.

diff --git a/compiler/make.m b/compiler/make.m
index 0ea0bd6e1..b30890067 100644
--- a/compiler/make.m
+++ b/compiler/make.m
@@ -2,6 +2,7 @@
 % vim: ft=mercury ts=4 sw=4 et
 %-----------------------------------------------------------------------------%
 % Copyright (C) 2002-2012 The University of Melbourne.
+% Copyright (C) 2013-2021 The Mercury team.
 % This file may only be copied under the terms of the GNU General
 % Public License - see the file COPYING in the Mercury distribution.
 %-----------------------------------------------------------------------------%
@@ -696,15 +697,38 @@ option_table_hash(AllOptionArgs, Hash, !IO) :-
     globals.get_options(AllOptionArgsGlobals, OptionTable),
     map.to_sorted_assoc_list(OptionTable, OptionList),
     inconsequential_options(InconsequentialOptions),
-    list.filter(is_consequential_option(InconsequentialOptions),
-        OptionList, ConsequentialOptionList),
-    Hash = md4sum(string(ConsequentialOptionList)).
+    list.filter(include_option_in_hash(InconsequentialOptions),
+        OptionList, HashOptionList),
+    globals.get_opt_tuple(AllOptionArgsGlobals, OptTuple),
+    Hash = md4sum(string({HashOptionList, OptTuple})).
 
-:- pred is_consequential_option(set(option)::in,
+:- pred include_option_in_hash(set(option)::in,
     pair(option, option_data)::in) is semidet.
 
-is_consequential_option(InconsequentialOptions, Option - _) :-
-    not set.contains(InconsequentialOptions, Option).
+include_option_in_hash(InconsequentialOptions, Option - OptionData) :-
+    require_complete_switch [OptionData]
+    (
+        ( OptionData = bool(_)
+        ; OptionData = int(_)
+        ; OptionData = string(_)
+        ; OptionData = maybe_int(_)
+        ; OptionData = maybe_string(_)
+        ; OptionData = accumulating(_)
+        ),
+        % XXX reconsider if a lot of these options really should be ignored
+        not set.contains(InconsequentialOptions, Option)
+    ;
+        ( OptionData = special
+        ; OptionData = bool_special
+        ; OptionData = int_special
+        ; OptionData = string_special
+        ; OptionData = maybe_string_special
+        ; OptionData = file_special
+        ),
+        % There is no point hashing special options as the option_data is
+        % always the same.
+        fail
+    ).
 
 :- pred compare_hash_file(globals::in, string::in, string::in, bool::out,
     io::di, io::uo) is det.
-- 
2.31.1



More information about the reviews mailing list