Skip to content

Commit ed2d2ca

Browse files
committed
evaluate conditional breakpoints, hit counts and log points in Macro.Env obtained from metadata
1 parent f0c4a81 commit ed2d2ca

File tree

4 files changed

+296
-149
lines changed

4 files changed

+296
-149
lines changed

apps/debug_adapter/lib/debug_adapter/breakpoint_condition.ex

Lines changed: 65 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -16,27 +16,27 @@ defmodule ElixirLS.DebugAdapter.BreakpointCondition do
1616
@spec register_condition(
1717
module,
1818
module,
19-
[non_neg_integer],
19+
non_neg_integer,
2020
String.t(),
2121
String.t() | nil,
22-
non_neg_integer
22+
String.t()
2323
) ::
2424
{:ok, {module, atom}} | {:error, :limit_reached}
25-
def register_condition(name \\ __MODULE__, module, lines, condition, log_message, hit_count) do
25+
def register_condition(name \\ __MODULE__, module, line, env, condition, log_message, hit_count) do
2626
GenServer.call(
2727
name,
28-
{:register_condition, {module, lines}, condition, log_message, hit_count}
28+
{:register_condition, {module, line}, env, condition, log_message, hit_count}
2929
)
3030
end
3131

32-
@spec unregister_condition(module, module, [non_neg_integer]) :: :ok
33-
def unregister_condition(name \\ __MODULE__, module, lines) do
34-
GenServer.cast(name, {:unregister_condition, {module, lines}})
32+
@spec unregister_condition(module, module, non_neg_integer) :: :ok
33+
def unregister_condition(name \\ __MODULE__, module, line) do
34+
GenServer.cast(name, {:unregister_condition, {module, line}})
3535
end
3636

37-
@spec has_condition?(module, module, [non_neg_integer]) :: boolean
38-
def has_condition?(name \\ __MODULE__, module, lines) do
39-
GenServer.call(name, {:has_condition?, {module, lines}})
37+
@spec has_condition?(module, module, non_neg_integer) :: boolean
38+
def has_condition?(name \\ __MODULE__, module, line) do
39+
GenServer.call(name, {:has_condition?, {module, line}})
4040
end
4141

4242
@spec get_condition(module, non_neg_integer) ::
@@ -96,7 +96,7 @@ defmodule ElixirLS.DebugAdapter.BreakpointCondition do
9696

9797
@impl GenServer
9898
def handle_call(
99-
{:register_condition, key, condition, log_message, hit_count},
99+
{:register_condition, key, env, condition, log_message, hit_count},
100100
_from,
101101
%{free: free, conditions: conditions} = state
102102
) do
@@ -111,7 +111,7 @@ defmodule ElixirLS.DebugAdapter.BreakpointCondition do
111111
state
112112
| free: rest,
113113
conditions:
114-
conditions |> Map.put(key, {number, {condition, log_message, hit_count}})
114+
conditions |> Map.put(key, {number, {env, condition, log_message, hit_count}})
115115
}
116116

117117
{:reply, {:ok, {__MODULE__, :"check_#{number}"}}, state}
@@ -120,7 +120,8 @@ defmodule ElixirLS.DebugAdapter.BreakpointCondition do
120120
{number, _old_condition} ->
121121
state = %{
122122
state
123-
| conditions: conditions |> Map.put(key, {number, {condition, log_message, hit_count}})
123+
| conditions:
124+
conditions |> Map.put(key, {number, {env, condition, log_message, hit_count}})
124125
}
125126

126127
{:reply, {:ok, {__MODULE__, :"check_#{number}"}}, state}
@@ -132,11 +133,11 @@ defmodule ElixirLS.DebugAdapter.BreakpointCondition do
132133
end
133134

134135
def handle_call({:get_condition, number}, _from, %{conditions: conditions, hits: hits} = state) do
135-
{condition, log_message, hit_count} =
136+
{env, condition, log_message, hit_count} =
136137
conditions |> Map.values() |> Enum.find(fn {n, _c} -> n == number end) |> elem(1)
137138

138139
hits = hits |> Map.get(number, 0)
139-
{:reply, {condition, log_message, hit_count, hits}, state}
140+
{:reply, {env, condition, log_message, hit_count, hits}, state}
140141
end
141142

142143
def handle_call(:clear, _from, _state) do
@@ -177,14 +178,20 @@ defmodule ElixirLS.DebugAdapter.BreakpointCondition do
177178
for i <- @range do
178179
@spec unquote(:"check_#{i}")(term) :: boolean
179180
def unquote(:"check_#{i}")(binding) do
180-
{condition, log_message, hit_count, hits} = get_condition(unquote(i))
181+
{env, condition, log_message, hit_count_condition, hits} = get_condition(unquote(i))
181182
elixir_binding = binding |> ElixirLS.DebugAdapter.Binding.to_elixir_variable_names()
182-
result = eval_condition(condition, elixir_binding)
183+
result = eval_condition(condition, elixir_binding, env)
183184

184185
result =
185186
if result do
186187
register_hit(unquote(i))
187188
# do not break if hit count not reached
189+
# the spec requires:
190+
# If both this property and `condition` are specified, `hitCondition` should
191+
# be evaluated only if the `condition` is met, and the debug adapter should
192+
# stop only if both conditions are met.
193+
194+
hit_count = eval_hit_condition(hit_count_condition, elixir_binding, env)
188195
hits + 1 > hit_count
189196
else
190197
result
@@ -194,20 +201,22 @@ defmodule ElixirLS.DebugAdapter.BreakpointCondition do
194201
# Debug Adapter Protocol:
195202
# If this attribute exists and is non-empty, the backend must not 'break' (stop)
196203
# but log the message instead. Expressions within {} are interpolated.
197-
Output.debugger_console(interpolate(log_message, elixir_binding))
204+
# If either `hitCondition` or `condition` is specified, then the message
205+
# should only be logged if those conditions are met.
206+
Output.debugger_console(interpolate(log_message, elixir_binding, env))
198207
false
199208
else
200209
result
201210
end
202211
end
203212
end
204213

205-
@spec eval_condition(String.t(), keyword) :: boolean
206-
def eval_condition("true", _binding), do: true
214+
@spec eval_condition(String.t(), keyword, Macro.Env.t()) :: boolean
215+
def eval_condition("true", _binding, _env), do: true
207216

208-
def eval_condition(condition, elixir_binding) do
217+
def eval_condition(condition, elixir_binding, env) do
209218
try do
210-
{term, _bindings} = Code.eval_string(condition, elixir_binding)
219+
{term, _bindings} = Code.eval_string(condition, elixir_binding, env)
211220
if term, do: true, else: false
212221
catch
213222
kind, error ->
@@ -219,9 +228,29 @@ defmodule ElixirLS.DebugAdapter.BreakpointCondition do
219228
end
220229
end
221230

222-
def eval_string(expression, elixir_binding) do
231+
@spec eval_hit_condition(String.t(), keyword, Macro.Env.t()) :: number
232+
def eval_hit_condition("0", _binding, _env), do: 0
233+
234+
def eval_hit_condition(condition, elixir_binding, env) do
235+
try do
236+
{term, _bindings} = Code.eval_string(condition, elixir_binding, env)
237+
238+
if is_number(term) do
239+
term
240+
else
241+
raise "Hit count evaluated to non number #{inspect(term)}"
242+
end
243+
catch
244+
kind, error ->
245+
Output.debugger_important("Error in hit count: " <> Exception.format_banner(kind, error))
246+
247+
0
248+
end
249+
end
250+
251+
def eval_string(expression, elixir_binding, env) do
223252
try do
224-
{term, _bindings} = Code.eval_string(expression, elixir_binding)
253+
{term, _bindings} = Code.eval_string(expression, elixir_binding, env)
225254
to_string(term)
226255
catch
227256
kind, error ->
@@ -233,39 +262,39 @@ defmodule ElixirLS.DebugAdapter.BreakpointCondition do
233262
end
234263
end
235264

236-
def interpolate(format_string, elixir_binding) do
237-
interpolate(format_string, [], elixir_binding)
265+
def interpolate(format_string, elixir_binding, env) do
266+
interpolate(format_string, [], elixir_binding, env)
238267
|> Enum.reverse()
239268
|> IO.iodata_to_binary()
240269
end
241270

242-
def interpolate(<<>>, acc, _elixir_binding), do: acc
271+
def interpolate(<<>>, acc, _elixir_binding, _env), do: acc
243272

244-
def interpolate(<<"\\{", rest::binary>>, acc, elixir_binding),
245-
do: interpolate(rest, ["{" | acc], elixir_binding)
273+
def interpolate(<<"\\{", rest::binary>>, acc, elixir_binding, env),
274+
do: interpolate(rest, ["{" | acc], elixir_binding, env)
246275

247-
def interpolate(<<"\\}", rest::binary>>, acc, elixir_binding),
248-
do: interpolate(rest, ["}" | acc], elixir_binding)
276+
def interpolate(<<"\\}", rest::binary>>, acc, elixir_binding, env),
277+
do: interpolate(rest, ["}" | acc], elixir_binding, env)
249278

250-
def interpolate(<<"{", rest::binary>>, acc, elixir_binding) do
279+
def interpolate(<<"{", rest::binary>>, acc, elixir_binding, env) do
251280
case parse_expression(rest, []) do
252281
{:ok, expression_iolist, expression_rest} ->
253282
expression =
254283
expression_iolist
255284
|> Enum.reverse()
256285
|> IO.iodata_to_binary()
257286

258-
eval_result = eval_string(expression, elixir_binding)
259-
interpolate(expression_rest, [eval_result | acc], elixir_binding)
287+
eval_result = eval_string(expression, elixir_binding, env)
288+
interpolate(expression_rest, [eval_result | acc], elixir_binding, env)
260289

261290
:error ->
262291
Output.debugger_important("Log message has unpaired or nested `{}`")
263292
acc
264293
end
265294
end
266295

267-
def interpolate(<<char::binary-size(1), rest::binary>>, acc, elixir_binding),
268-
do: interpolate(rest, [char | acc], elixir_binding)
296+
def interpolate(<<char::binary-size(1), rest::binary>>, acc, elixir_binding, env),
297+
do: interpolate(rest, [char | acc], elixir_binding, env)
269298

270299
def parse_expression(<<>>, _acc), do: :error
271300
def parse_expression(<<"\\{", rest::binary>>, acc), do: parse_expression(rest, ["{" | acc])

apps/debug_adapter/lib/debug_adapter/server.ex

Lines changed: 52 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -973,7 +973,9 @@ defmodule ElixirLS.DebugAdapter.Server do
973973

974974
for {{m, f, a}, lines} <- state.function_breakpoints,
975975
not Map.has_key?(parsed_mfas_conditions, {m, f, a}) do
976-
BreakpointCondition.unregister_condition(m, lines)
976+
for line <- lines do
977+
BreakpointCondition.unregister_condition(m, line)
978+
end
977979

978980
case :int.del_break_in(m, f, a) do
979981
:ok ->
@@ -993,6 +995,14 @@ defmodule ElixirLS.DebugAdapter.Server do
993995
into: %{},
994996
do:
995997
(
998+
path =
999+
try do
1000+
module_info = ModuleInfoCache.get(m) || m.module_info()
1001+
Path.expand(to_string(module_info[:compile][:source]))
1002+
rescue
1003+
_ -> "nofile"
1004+
end
1005+
9961006
result =
9971007
case current[{m, f, a}] do
9981008
nil ->
@@ -1011,6 +1021,7 @@ defmodule ElixirLS.DebugAdapter.Server do
10111021

10121022
# pass nil as log_message - not supported on function breakpoints as of DAP 1.63
10131023
update_break_condition(
1024+
path,
10141025
m,
10151026
lines,
10161027
condition,
@@ -1038,7 +1049,15 @@ defmodule ElixirLS.DebugAdapter.Server do
10381049

10391050
lines ->
10401051
# pass nil as log_message - not supported on function breakpoints as of DAP 1.51
1041-
update_break_condition(m, lines, condition, nil, hit_count, state.config)
1052+
update_break_condition(
1053+
path,
1054+
m,
1055+
lines,
1056+
condition,
1057+
nil,
1058+
hit_count,
1059+
state.config
1060+
)
10421061

10431062
{:ok, lines}
10441063
end
@@ -2461,7 +2480,7 @@ defmodule ElixirLS.DebugAdapter.Server do
24612480
Output.debugger_console("Setting breakpoint in #{inspect(module)} #{path}:#{line}")
24622481
# no need to handle errors here, it can fail only with {:error, :break_exists}
24632482
:int.break(module, line)
2464-
update_break_condition(module, line, condition, log_message, hit_count, config)
2483+
update_break_condition(path, module, line, condition, log_message, hit_count, config)
24652484

24662485
[module | added]
24672486

@@ -2524,38 +2543,49 @@ defmodule ElixirLS.DebugAdapter.Server do
25242543
end
25252544
end
25262545

2527-
def update_break_condition(module, lines, condition, log_message, hit_count, config) do
2546+
def update_break_condition(path, module, lines, condition, log_message, hit_count, config) do
25282547
lines = List.wrap(lines)
25292548

2530-
condition = parse_condition(condition)
2549+
condition = parse_condition(condition, "true")
25312550

2532-
hit_count = eval_hit_count(hit_count)
2551+
hit_count = parse_condition(hit_count, "0")
25332552

25342553
log_message = if log_message not in ["", nil], do: log_message
25352554

2536-
register_break_condition(module, lines, condition, log_message, hit_count, config)
2555+
register_break_condition(path, module, lines, condition, log_message, hit_count, config)
25372556
end
25382557

2539-
defp register_break_condition(module, lines, condition, log_message, hit_count, %{
2558+
defp register_break_condition(file, module, lines, condition, log_message, hit_count, %{
25402559
"request" => "launch"
25412560
}) do
2542-
case BreakpointCondition.register_condition(module, lines, condition, log_message, hit_count) do
2543-
{:ok, mf} ->
2544-
for line <- lines do
2561+
for line <- lines do
2562+
{_metadata, _env, macro_env_or_opts} = parse_file(file, line)
2563+
# TODO use Code.env_for_eval when we require elixir 1.14
2564+
env = ElixirLS.DebugAdapter.Code.env_for_eval(macro_env_or_opts)
2565+
2566+
case BreakpointCondition.register_condition(
2567+
module,
2568+
line,
2569+
env,
2570+
condition,
2571+
log_message,
2572+
hit_count
2573+
) do
2574+
{:ok, mf} ->
25452575
:int.test_at_break(module, line, mf)
2546-
end
25472576

2548-
{:error, reason} ->
2549-
Output.debugger_important(
2550-
"Unable to set condition on a breakpoint in #{module}:#{inspect(lines)}: #{inspect(reason)}"
2551-
)
2577+
{:error, reason} ->
2578+
Output.debugger_important(
2579+
"Unable to set condition on a breakpoint in #{module}:#{inspect(line)}: #{inspect(reason)}"
2580+
)
2581+
end
25522582
end
25532583
end
25542584

2555-
defp register_break_condition(_module, _lines, condition, log_message, hit_count, %{
2585+
defp register_break_condition(_file, _module, _lines, condition, log_message, hit_count, %{
25562586
"request" => "attach"
25572587
}) do
2558-
if condition != "true" || log_message || hit_count != 0 do
2588+
if condition != "true" || log_message || hit_count != "0" do
25592589
# Module passed to :int.test_at_break has to be available on remote nodes. Otherwise break condition will
25602590
# always evaluate to false. We cannot easily distribute BreakpointCondition to remote nodes.
25612591
Output.debugger_important(
@@ -2564,39 +2594,16 @@ defmodule ElixirLS.DebugAdapter.Server do
25642594
end
25652595
end
25662596

2567-
defp parse_condition(condition) when condition in [nil, ""], do: "true"
2597+
defp parse_condition(condition, default) when condition in [nil, ""], do: default
25682598

2569-
defp parse_condition(condition) do
2599+
defp parse_condition(condition, default) do
25702600
case Code.string_to_quoted(condition) do
25712601
{:ok, _} ->
25722602
condition
25732603

25742604
{:error, reason} ->
25752605
Output.debugger_important("Cannot parse breakpoint condition: #{inspect(reason)}")
2576-
"true"
2577-
end
2578-
end
2579-
2580-
defp eval_hit_count(hit_count) when hit_count in [nil, ""], do: 0
2581-
2582-
defp eval_hit_count(hit_count) do
2583-
try do
2584-
# TODO binding?
2585-
{term, _bindings} = Code.eval_string(hit_count, [])
2586-
2587-
if is_integer(term) do
2588-
term
2589-
else
2590-
Output.debugger_important("Hit condition must evaluate to integer")
2591-
0
2592-
end
2593-
catch
2594-
kind, error ->
2595-
Output.debugger_important(
2596-
"Error while evaluating hit condition: " <> Exception.format_banner(kind, error)
2597-
)
2598-
2599-
0
2606+
default
26002607
end
26012608
end
26022609

@@ -2920,8 +2927,7 @@ defmodule ElixirLS.DebugAdapter.Server do
29202927
buffer_file_metadata = ElixirSense.Core.Parser.parse_string(code, false, true, {line, 1})
29212928

29222929
env = ElixirSense.Core.Metadata.get_env(buffer_file_metadata, {line, 1})
2923-
# TODO env_for_eval?
2924-
# should we clear versioned_vars?
2930+
29252931
{buffer_file_metadata, env, ElixirSense.Core.State.Env.to_macro_env(env, file, line)}
29262932
else
29272933
# do not try to parse non elixir files

0 commit comments

Comments
 (0)