Skip to content

Commit d1076e4

Browse files
committed
Do not run fixpoint computations on runtime deps
If A depends on B and B depends on C, all as runtime dependencies, and C changes, there is no change that can happen in C that will change the result of running the checker on A. This could change in the future in two ways: 1. If we introduce inference of return types, then we need to infer and check the types for A again. This may be an argument for not introducing inference of return types (only of patterns and guards) 2. We will need to track "type exports" once we introduce type aliases
1 parent b9e4477 commit d1076e4

File tree

2 files changed

+48
-38
lines changed

2 files changed

+48
-38
lines changed

Diff for: lib/mix/lib/mix/compilers/elixir.ex

+43-28
Original file line numberDiff line numberDiff line change
@@ -609,8 +609,8 @@ defmodule Mix.Compilers.Elixir do
609609
# within the dependency, they will be recompiled. However, export
610610
# and runtime dependencies won't have recompiled so we need to
611611
# propagate them to the parent app.
612-
{dep_modules, _, _} =
613-
fixpoint_runtime_modules(manifest_sources, Map.from_keys(dep_modules, true))
612+
dep_modules =
613+
fixpoint_non_compile_modules(manifest_sources, Map.from_keys(dep_modules, true))
614614

615615
old_exports = Map.get(deps_exports, app, %{})
616616

@@ -654,46 +654,39 @@ defmodule Mix.Compilers.Elixir do
654654
end
655655
end
656656

657-
defp fixpoint_runtime_modules(sources, modules) when modules != %{} do
658-
fixpoint_runtime_modules(Map.to_list(sources), modules, false, [], [], sources)
657+
defp fixpoint_non_compile_modules(sources, modules) when modules != %{} do
658+
fixpoint_non_compile_modules(Map.to_list(sources), modules, false, [])
659659
end
660660

661-
defp fixpoint_runtime_modules(sources, modules) do
662-
{modules, [], sources}
661+
defp fixpoint_non_compile_modules(_sources, modules) do
662+
modules
663663
end
664664

665-
defp fixpoint_runtime_modules(
666-
[{source_path, source_entry} = pair | sources],
665+
defp fixpoint_non_compile_modules(
666+
[{_source_path, source_entry} = pair | sources],
667667
modules,
668668
new?,
669-
pending_sources,
670-
acc_modules,
671-
acc_sources
669+
pending_sources
672670
) do
673671
source(export_references: export_refs, runtime_references: runtime_refs) = source_entry
674672

675673
if has_any_key?(modules, export_refs) or has_any_key?(modules, runtime_refs) do
676674
new_modules = Enum.reject(source(source_entry, :modules), &Map.has_key?(modules, &1))
677675
modules = Enum.reduce(new_modules, modules, &Map.put(&2, &1, true))
678676
new? = new? or new_modules != []
679-
acc_modules = new_modules ++ acc_modules
680-
681-
acc_sources =
682-
Map.replace!(acc_sources, source_path, source(source_entry, runtime_warnings: []))
683-
684-
fixpoint_runtime_modules(sources, modules, new?, pending_sources, acc_modules, acc_sources)
677+
fixpoint_non_compile_modules(sources, modules, new?, pending_sources)
685678
else
686679
pending_sources = [pair | pending_sources]
687-
fixpoint_runtime_modules(sources, modules, new?, pending_sources, acc_modules, acc_sources)
680+
fixpoint_non_compile_modules(sources, modules, new?, pending_sources)
688681
end
689682
end
690683

691-
defp fixpoint_runtime_modules([], modules, new?, pending_sources, acc_modules, acc_sources)
684+
defp fixpoint_non_compile_modules([], modules, new?, pending_sources)
692685
when new? == false or pending_sources == [],
693-
do: {modules, acc_modules, acc_sources}
686+
do: modules
694687

695-
defp fixpoint_runtime_modules([], modules, true, pending_sources, acc_modules, acc_sources),
696-
do: fixpoint_runtime_modules(pending_sources, modules, false, [], acc_modules, acc_sources)
688+
defp fixpoint_non_compile_modules([], modules, true, pending_sources),
689+
do: fixpoint_non_compile_modules(pending_sources, modules, false, [])
697690

698691
defp exports_md5(module, use_attributes?) do
699692
cond do
@@ -1068,7 +1061,7 @@ defmodule Mix.Compilers.Elixir do
10681061
end
10691062
end
10701063

1071-
defp each_cycle(runtime_modules, compile_path, timestamp, state) do
1064+
defp each_cycle(stale_modules, compile_path, timestamp, state) do
10721065
{modules, _exports, sources, changed, pending_modules, stale_exports} = state
10731066

10741067
{pending_modules, exports, changed} =
@@ -1081,11 +1074,33 @@ defmodule Mix.Compilers.Elixir do
10811074
end
10821075

10831076
if changed == [] do
1084-
# We merge runtime_modules (which is a map of %{module => true}) into
1085-
# a map of modules (which is a map of %{module => record}). This is fine
1086-
# since fixpoint_runtime_modules only cares about map keys.
1087-
{_, runtime_modules, sources} =
1088-
fixpoint_runtime_modules(sources, Map.merge(modules, runtime_modules))
1077+
# We merge stale_modules (which is a map of %{module => true} that the user changed)
1078+
# into a map of modules we compiled (which is a map of %{module => record}). This is
1079+
# fine because we only care about the keys.
1080+
runtime_modules = Map.merge(modules, stale_modules)
1081+
1082+
# Now we do a simple pass finding anything that directly depends on the modules that
1083+
# changed. We don't need to compute a fixpoint, because now only the directly affected
1084+
# matter.
1085+
{sources, runtime_modules} =
1086+
Enum.reduce(sources, {sources, Map.keys(runtime_modules)}, fn
1087+
{source_path, source_entry}, {acc_sources, acc_modules} ->
1088+
source(export_references: export_refs, runtime_references: runtime_refs) =
1089+
source_entry
1090+
1091+
if has_any_key?(runtime_modules, export_refs) or
1092+
has_any_key?(runtime_modules, runtime_refs) do
1093+
acc_sources =
1094+
Map.replace!(acc_sources, source_path, source(source_entry, runtime_warnings: []))
1095+
1096+
new_modules =
1097+
Enum.reject(source(source_entry, :modules), &Map.has_key?(runtime_modules, &1))
1098+
1099+
{acc_sources, new_modules ++ acc_modules}
1100+
else
1101+
{acc_sources, acc_modules}
1102+
end
1103+
end)
10891104

10901105
runtime_paths =
10911106
Enum.map(runtime_modules, &{&1, Path.join(compile_path, Atom.to_string(&1) <> ".beam")})

Diff for: lib/mix/test/mix/tasks/compile.elixir_test.exs

+5-10
Original file line numberDiff line numberDiff line change
@@ -1597,11 +1597,7 @@ defmodule Mix.Tasks.Compile.ElixirTest do
15971597
def foo(), do: B.foo()
15981598
def bar(), do: B.bar()
15991599
def __after_verify__(__MODULE__) do
1600-
if Code.ensure_loaded?(B) and not function_exported?(B, :foo, 0) do
1601-
:ok
1602-
else
1603-
IO.warn("AFTER_VERIFY", __ENV__)
1604-
end
1600+
IO.warn("AFTER_VERIFY", __ENV__)
16051601
end
16061602
end
16071603
""")
@@ -1630,10 +1626,9 @@ defmodule Mix.Tasks.Compile.ElixirTest do
16301626
end)
16311627

16321628
# Check B due to direct dependency on A
1633-
# Check C due to transient dependency on A
16341629
assert output =~ "A.foo/0 is undefined or private"
1635-
assert output =~ "B.bar/0 is undefined or private"
1636-
assert output =~ "AFTER_VERIFY"
1630+
refute output =~ "B.bar/0 is undefined or private"
1631+
refute output =~ "AFTER_VERIFY"
16371632

16381633
# Ensure only A was recompiled
16391634
assert_received {:mix_shell, :info, ["Compiled lib/a.ex"]}
@@ -1650,7 +1645,7 @@ defmodule Mix.Tasks.Compile.ElixirTest do
16501645
assert output =~ "B.bar/0 is undefined or private"
16511646
assert output =~ "AFTER_VERIFY"
16521647

1653-
# Now we change B and it must no longer emit an AFTER_VERIFY warning
1648+
# Now we change B and it must emit an AFTER_VERIFY warning
16541649
File.write!("lib/b.ex", """
16551650
defmodule B do
16561651
end
@@ -1663,7 +1658,7 @@ defmodule Mix.Tasks.Compile.ElixirTest do
16631658

16641659
assert output =~ "B.foo/0 is undefined or private"
16651660
assert output =~ "B.bar/0 is undefined or private"
1666-
refute output =~ "AFTER_VERIFY"
1661+
assert output =~ "AFTER_VERIFY"
16671662
end)
16681663
end
16691664

0 commit comments

Comments
 (0)