Skip to content

Commit a23e952

Browse files
praihanfacebook-github-bot
authored andcommitted
Implement source backtrace support for partial blocks
Summary: With partial applications being added, one source file can have multiple stack frames in it. So showing *only* the source file name in the back trace is not enough anymore. The format of the source backtrace now include the partial name, if available. Reviewed By: createdbysk Differential Revision: D68689301 fbshipit-source-id: 3201a51fa35322df8e3296816ec8e1bec1dd6737
1 parent 75913c9 commit a23e952

File tree

2 files changed

+169
-65
lines changed

2 files changed

+169
-65
lines changed

thrift/compiler/whisker/render.cc

+83-31
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include <exception>
2424
#include <functional>
2525
#include <iterator>
26+
#include <list>
2627
#include <ostream>
2728
#include <set>
2829
#include <string>
@@ -199,12 +200,22 @@ class source_stack {
199200
* each partial application's source location.
200201
*/
201202
struct frame {
203+
/**
204+
* The frame from which the current frame was jumped to. This is
205+
* nullptr for the root frame.
206+
*/
207+
frame* prev;
202208
/**
203209
* The evaluation context of the source frame. When a new frame is pushed:
204210
* - macros — retain the context from the previous frame.
205211
* - partials — derive a new context from the previous frame.
206212
*/
207213
eval_context context;
214+
/**
215+
* The currently active partial application, or nullptr if the frame
216+
* originated from the root, or is a macro.
217+
*/
218+
partial_definition::ptr partial;
208219
/**
209220
* For all elements except the top of the stack, this is the location of the
210221
* partial application within that source that led to the current stack.
@@ -215,44 +226,59 @@ class source_stack {
215226
* saved before the new source is pushed to the stack. After the partial
216227
* application completes, the saved location is dropped.
217228
*/
218-
source_location apply_location;
229+
source_location jumped_from;
230+
/**
231+
* For `{{#pragma ignore-newlines}}`.
232+
*/
219233
bool ignore_newlines = false;
220234

221-
explicit frame(eval_context ctx) : context(std::move(ctx)) {}
235+
std::optional<std::string> name() const {
236+
return partial == nullptr ? std::nullopt : std::optional{partial->name};
237+
}
238+
239+
explicit frame(
240+
frame* prev, eval_context ctx, partial_definition::ptr partial)
241+
: prev(prev), context(std::move(ctx)), partial(std::move(partial)) {}
222242
};
223243

224244
/**
225245
* Returns the current source stack frame, or nullptr if the stack is empty.
226246
*/
227-
frame* top() {
228-
if (frames_.empty()) {
229-
return nullptr;
230-
}
231-
return &frames_.back();
247+
frame* top() { return frames_.empty() ? nullptr : &frames_.back(); }
248+
const frame* top() const {
249+
return frames_.empty() ? nullptr : &frames_.back();
232250
}
233251

234252
/**
235253
* An RAII guard that pushes and pops sources from the stack of partial
236254
* applications.
237255
*/
238-
auto make_frame_guard(eval_context eval_ctx, source_location apply_location) {
256+
auto make_frame_guard(
257+
eval_context eval_ctx,
258+
partial_definition::ptr partial,
259+
source_location jumped_from) {
239260
class frame_guard {
240261
public:
241262
explicit frame_guard(
242263
source_stack& stack,
243264
eval_context eval_ctx,
244-
source_location apply_location)
265+
partial_definition::ptr&& partial,
266+
source_location jumped_from)
245267
: stack_(stack) {
246268
if (auto* frame = stack_.top()) {
247-
frame->apply_location = std::move(apply_location);
269+
// Save the jump location since we're jumping to a new frame. This
270+
// allows collecting a backtrace.
271+
frame->jumped_from = jumped_from;
248272
}
249-
stack.frames_.emplace_back(std::move(eval_ctx));
273+
stack.frames_.emplace_back(
274+
stack_.top(), std::move(eval_ctx), std::move(partial));
250275
}
251276
~frame_guard() noexcept {
252277
assert(!stack_.frames_.empty());
253278
stack_.frames_.pop_back();
254279
if (auto* source = stack_.top()) {
255-
source->apply_location = source_location();
280+
// Reset the jump location since we've return to its origin.
281+
source->jumped_from = source_location();
256282
}
257283
}
258284
frame_guard(frame_guard&& other) = delete;
@@ -263,10 +289,23 @@ class source_stack {
263289
private:
264290
source_stack& stack_;
265291
};
266-
return frame_guard{*this, std::move(eval_ctx), std::move(apply_location)};
292+
return frame_guard{
293+
*this, std::move(eval_ctx), std::move(partial), jumped_from};
267294
}
268295

269-
using backtrace = std::vector<resolved_location>;
296+
struct backtrace_frame {
297+
/**
298+
* The resolved source location where a jump happened, or the origin of
299+
* the backtrace for the top-most frame.
300+
*/
301+
resolved_location location;
302+
/**
303+
* The name of the partial application (if present) from which the jump
304+
* happened.
305+
*/
306+
std::optional<std::string> name;
307+
};
308+
using backtrace = std::vector<backtrace_frame>;
270309
/**
271310
* Creates a back trace for debugging that contains the chain of partial
272311
* applications within the source.
@@ -275,25 +314,31 @@ class source_stack {
275314
* caller.
276315
*/
277316
backtrace make_backtrace_at(const source_location& current) const {
278-
assert(!frames_.empty());
279317
assert(current != source_location());
318+
const frame* frame = top();
319+
assert(frame != nullptr);
280320

281-
std::vector<resolved_location> result;
282-
result.emplace_back(resolved_location(current, diags_.source_mgr()));
283-
for (auto frame = std::next(frames_.rbegin()); frame != frames_.rend();
284-
++frame) {
285-
assert(frame->apply_location != source_location());
286-
result.emplace_back(
287-
resolved_location(frame->apply_location, diags_.source_mgr()));
321+
std::vector<backtrace_frame> result;
322+
result.emplace_back(backtrace_frame{
323+
resolved_location(current, diags_.source_mgr()),
324+
frame->name(),
325+
});
326+
frame = frame->prev;
327+
for (; frame != nullptr; frame = frame->prev) {
328+
assert(frame->jumped_from != source_location());
329+
result.emplace_back(backtrace_frame{
330+
resolved_location(frame->jumped_from, diags_.source_mgr()),
331+
frame->name(),
332+
});
288333
}
289334
return result;
290335
}
291336

292337
explicit source_stack(diagnostics_engine& diags) : diags_(diags) {}
293338

294339
private:
295-
// Using std::vector as a stack so we can iterate over it
296-
std::vector<frame> frames_;
340+
// Doubly linked list provides stable iterators / pointers for backtraces.
341+
std::list<frame> frames_;
297342
diagnostics_engine& diags_;
298343
};
299344

@@ -371,7 +416,9 @@ class render_engine {
371416
auto eval_ctx = eval_context::with_root_scope(
372417
std::move(root_context_), std::exchange(opts_.globals, {}));
373418
auto source_frame_guard = source_stack_.make_frame_guard(
374-
std::move(eval_ctx), source_location());
419+
std::move(eval_ctx),
420+
nullptr /* the root node is not a partial */,
421+
source_location());
375422
visit(root.body_elements);
376423
return true;
377424
} catch (const render_error& err) {
@@ -385,14 +432,17 @@ class render_engine {
385432
auto source_trace = [&]() -> std::string {
386433
std::string result;
387434
for (std::size_t i = 0; i < backtrace.size(); ++i) {
388-
const auto& frame = backtrace[i];
435+
const source_stack::backtrace_frame& frame = backtrace[i];
436+
const std::string location = frame.name.has_value()
437+
? fmt::format("{} @ {}", *frame.name, frame.location.file_name())
438+
: fmt::format("{}", frame.location.file_name());
389439
fmt::format_to(
390440
std::back_inserter(result),
391441
"#{} {} <line:{}, col:{}>\n",
392442
i,
393-
frame.file_name(),
394-
frame.line(),
395-
frame.column());
443+
location,
444+
frame.location.line(),
445+
frame.location.column());
396446
}
397447
return result;
398448
}();
@@ -1071,7 +1121,7 @@ class render_engine {
10711121
}
10721122

10731123
auto source_frame_guard = source_stack_.make_frame_guard(
1074-
std::move(derived_ctx), partial_statement.loc.begin);
1124+
std::move(derived_ctx), partial, partial_statement.loc.begin);
10751125
auto indent_guard =
10761126
out_.make_indent_guard(partial_statement.standalone_offset_within_line);
10771127
visit(partial->bodies);
@@ -1104,7 +1154,9 @@ class render_engine {
11041154
// Macros are "inlined" into their invocation site. In other words, they
11051155
// execute within the scope where they are invoked.
11061156
auto source_frame_guard = source_stack_.make_frame_guard(
1107-
current_frame().context, macro.loc.begin);
1157+
current_frame().context,
1158+
nullptr /* no partial definition because macros are at root scope */,
1159+
macro.loc.begin);
11081160
auto indent_guard =
11091161
out_.make_indent_guard(macro.standalone_offset_within_line);
11101162
visit(resolved_macro->body_elements);

0 commit comments

Comments
 (0)