Skip to content

Commit 8e19d02

Browse files
varungandhi-srcvarungandhi-src
authored
fix: More accurate source locations for method names. (#92)
Support various different rewrites by making sure to propagate locations correctly.
1 parent b96af00 commit 8e19d02

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

79 files changed

+1986
-559
lines changed

‎ast/Helpers.h

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -244,36 +244,37 @@ class MK {
244244
core::make_type<core::NamedLiteralType>(core::Symbols::String(), value));
245245
}
246246

247-
static ExpressionPtr Method(core::LocOffsets loc, core::LocOffsets declLoc, core::NameRef name,
248-
MethodDef::ARGS_store args, ExpressionPtr rhs,
247+
static ExpressionPtr Method(core::LocOffsets loc, core::LocOffsets declLoc, core::LocOffsets nameLoc,
248+
core::NameRef name, MethodDef::ARGS_store args, ExpressionPtr rhs,
249249
MethodDef::Flags flags = MethodDef::Flags()) {
250250
if (args.empty() || (!isa_tree<ast::Local>(args.back()) && !isa_tree<ast::BlockArg>(args.back()))) {
251251
auto blkLoc = core::LocOffsets::none();
252252
args.emplace_back(make_expression<ast::BlockArg>(blkLoc, MK::Local(blkLoc, core::Names::blkArg())));
253253
}
254-
return make_expression<MethodDef>(loc, declLoc, core::Symbols::todoMethod(), name, std::move(args),
254+
return make_expression<MethodDef>(loc, declLoc, nameLoc, core::Symbols::todoMethod(), name, std::move(args),
255255
std::move(rhs), flags);
256256
}
257257

258-
static ExpressionPtr SyntheticMethod(core::LocOffsets loc, core::LocOffsets declLoc, core::NameRef name,
259-
MethodDef::ARGS_store args, ExpressionPtr rhs,
258+
static ExpressionPtr SyntheticMethod(core::LocOffsets loc, core::LocOffsets declLoc, core::LocOffsets nameLoc,
259+
core::NameRef name, MethodDef::ARGS_store args, ExpressionPtr rhs,
260260
MethodDef::Flags flags = MethodDef::Flags()) {
261261
flags.isRewriterSynthesized = true;
262-
return Method(loc, declLoc, name, std::move(args), std::move(rhs), flags);
262+
return Method(loc, declLoc, nameLoc, name, std::move(args), std::move(rhs), flags);
263263
}
264264

265-
static ExpressionPtr SyntheticMethod0(core::LocOffsets loc, core::LocOffsets declLoc, core::NameRef name,
266-
ExpressionPtr rhs, MethodDef::Flags flags = MethodDef::Flags()) {
265+
static ExpressionPtr SyntheticMethod0(core::LocOffsets loc, core::LocOffsets declLoc, core::LocOffsets nameLoc,
266+
core::NameRef name, ExpressionPtr rhs,
267+
MethodDef::Flags flags = MethodDef::Flags()) {
267268
MethodDef::ARGS_store args;
268-
return SyntheticMethod(loc, declLoc, name, std::move(args), std::move(rhs), flags);
269+
return SyntheticMethod(loc, declLoc, nameLoc, name, std::move(args), std::move(rhs), flags);
269270
}
270271

271-
static ExpressionPtr SyntheticMethod1(core::LocOffsets loc, core::LocOffsets declLoc, core::NameRef name,
272-
ExpressionPtr arg0, ExpressionPtr rhs,
272+
static ExpressionPtr SyntheticMethod1(core::LocOffsets loc, core::LocOffsets declLoc, core::LocOffsets nameLoc,
273+
core::NameRef name, ExpressionPtr arg0, ExpressionPtr rhs,
273274
MethodDef::Flags flags = MethodDef::Flags()) {
274275
MethodDef::ARGS_store args;
275276
args.emplace_back(std::move(arg0));
276-
return SyntheticMethod(loc, declLoc, name, std::move(args), std::move(rhs), flags);
277+
return SyntheticMethod(loc, declLoc, nameLoc, name, std::move(args), std::move(rhs), flags);
277278
}
278279

279280
static ExpressionPtr ClassOrModule(core::LocOffsets loc, core::LocOffsets declLoc, ExpressionPtr name,

‎ast/TreeCopying.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ ExpressionPtr deepCopy(const void *avoid, const Tag tag, const void *tree, bool
4444

4545
case Tag::MethodDef: {
4646
auto *exp = reinterpret_cast<const MethodDef *>(tree);
47-
return make_expression<MethodDef>(exp->loc, exp->declLoc, exp->symbol, exp->name,
47+
return make_expression<MethodDef>(exp->loc, exp->declLoc, exp->nameLoc, exp->symbol, exp->name,
4848
deepCopyVec(avoid, exp->args), deepCopy(avoid, exp->rhs), exp->flags);
4949
}
5050

‎ast/Trees.cc

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -164,9 +164,10 @@ ClassDef::ClassDef(core::LocOffsets loc, core::LocOffsets declLoc, core::ClassOr
164164
_sanityCheck();
165165
}
166166

167-
MethodDef::MethodDef(core::LocOffsets loc, core::LocOffsets declLoc, core::MethodRef symbol, core::NameRef name,
168-
ARGS_store args, ExpressionPtr rhs, Flags flags)
169-
: loc(loc), declLoc(declLoc), symbol(symbol), rhs(std::move(rhs)), args(std::move(args)), name(name), flags(flags) {
167+
MethodDef::MethodDef(core::LocOffsets loc, core::LocOffsets declLoc, core::LocOffsets nameLoc, core::MethodRef symbol,
168+
core::NameRef name, ARGS_store args, ExpressionPtr rhs, Flags flags)
169+
: loc(loc), declLoc(declLoc), nameLoc(nameLoc), symbol(symbol), rhs(std::move(rhs)), args(std::move(args)),
170+
name(name), flags(flags) {
170171
categoryCounterInc("trees", "methoddef");
171172
histogramInc("trees.methodDef.args", this->args.size());
172173
_sanityCheck();

‎ast/Trees.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,9 @@ EXPRESSION(MethodDef) {
388388
/// The range for the method declaration, including 'def' and ending
389389
/// after the closing ')' after the parameter list.
390390
core::LocOffsets declLoc;
391+
/// The range for the name of the method itself in the method declaration,
392+
/// excluding 'def' and the parameter list '(...)'.
393+
core::LocOffsets nameLoc;
391394
/// Reference to the method data.
392395
core::MethodRef symbol;
393396

@@ -401,8 +404,8 @@ EXPRESSION(MethodDef) {
401404
using Flags = core::FoundMethod::Flags;
402405
Flags flags;
403406

404-
MethodDef(core::LocOffsets loc, core::LocOffsets declLoc, core::MethodRef symbol, core::NameRef name,
405-
ARGS_store args, ExpressionPtr rhs, Flags flags);
407+
MethodDef(core::LocOffsets loc, core::LocOffsets declLoc, core::LocOffsets nameLoc, core::MethodRef symbol,
408+
core::NameRef name, ARGS_store args, ExpressionPtr rhs, Flags flags);
406409

407410
ExpressionPtr deepCopy() const;
408411

@@ -412,7 +415,7 @@ EXPRESSION(MethodDef) {
412415

413416
void _sanityCheck();
414417
};
415-
CheckSize(MethodDef, 64, 8);
418+
CheckSize(MethodDef, 72, 8);
416419

417420
EXPRESSION(If) {
418421
public:

‎ast/desugar/Desugar.cc

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -300,8 +300,9 @@ ExpressionPtr validateRBIBody(DesugarContext dctx, ExpressionPtr body) {
300300
return body;
301301
}
302302

303-
ExpressionPtr buildMethod(DesugarContext dctx, core::LocOffsets loc, core::LocOffsets declLoc, core::NameRef name,
304-
unique_ptr<parser::Node> &argnode, unique_ptr<parser::Node> &body, bool isSelf) {
303+
ExpressionPtr buildMethod(DesugarContext dctx, core::LocOffsets loc, core::LocOffsets declLoc, core::LocOffsets nameLoc,
304+
core::NameRef name, unique_ptr<parser::Node> &argnode, unique_ptr<parser::Node> &body,
305+
bool isSelf) {
305306
// Reset uniqueCounter within this scope (to keep numbers small)
306307
uint32_t uniqueCounter = 1;
307308
DesugarContext dctx1(dctx.ctx, uniqueCounter, dctx.enclosingBlockArg, declLoc, name);
@@ -320,7 +321,7 @@ ExpressionPtr buildMethod(DesugarContext dctx, core::LocOffsets loc, core::LocOf
320321
ExpressionPtr desugaredBody = desugarBody(dctx2, loc, body, std::move(destructures));
321322
desugaredBody = validateRBIBody(dctx2, move(desugaredBody));
322323

323-
auto mdef = MK::Method(loc, declLoc, name, std::move(args), std::move(desugaredBody));
324+
auto mdef = MK::Method(loc, declLoc, nameLoc, name, std::move(args), std::move(desugaredBody));
324325
cast_tree<MethodDef>(mdef)->flags.isSelfMethod = isSelf;
325326
return mdef;
326327
}
@@ -1515,8 +1516,8 @@ ExpressionPtr node2TreeImpl(DesugarContext dctx, unique_ptr<parser::Node> what)
15151516
},
15161517
[&](parser::DefMethod *method) {
15171518
bool isSelf = false;
1518-
ExpressionPtr res =
1519-
buildMethod(dctx, method->loc, method->declLoc, method->name, method->args, method->body, isSelf);
1519+
ExpressionPtr res = buildMethod(dctx, method->loc, method->declLoc, method->nameLoc, method->name,
1520+
method->args, method->body, isSelf);
15201521
result = std::move(res);
15211522
},
15221523
[&](parser::DefS *method) {
@@ -1528,8 +1529,8 @@ ExpressionPtr node2TreeImpl(DesugarContext dctx, unique_ptr<parser::Node> what)
15281529
}
15291530
}
15301531
bool isSelf = true;
1531-
ExpressionPtr res =
1532-
buildMethod(dctx, method->loc, method->declLoc, method->name, method->args, method->body, isSelf);
1532+
ExpressionPtr res = buildMethod(dctx, method->loc, method->declLoc, method->nameLoc, method->name,
1533+
method->args, method->body, isSelf);
15331534
result = std::move(res);
15341535
},
15351536
[&](parser::SClass *sclass) {

‎cfg/CFG.cc

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,18 @@ string CFG::toString(const core::GlobalState &gs) const {
241241
string CFG::toTextualString(const core::GlobalState &gs, optional<core::FileRef> file) const {
242242
fmt::memory_buffer buf;
243243
string symbolName = this->symbol.showFullName(gs);
244-
fmt::format_to(std::back_inserter(buf), "method {} {{\n\n", symbolName);
244+
if (file) {
245+
auto method = this->symbol.data(gs);
246+
if (method->nameLoc.exists() && !method->nameLoc.empty()) {
247+
fmt::format_to(std::back_inserter(buf), "method @ {} {} {{\n\n",
248+
core::Loc(file.value(), method->nameLoc).showRawLineColumn(gs), symbolName);
249+
} else {
250+
fmt::format_to(std::back_inserter(buf), "method @ {} (full) {} {{\n\n", method->loc().showRawLineColumn(gs),
251+
symbolName);
252+
}
253+
} else {
254+
fmt::format_to(std::back_inserter(buf), "method {} {{\n\n", symbolName);
255+
}
245256
for (auto &basicBlock : this->basicBlocks) {
246257
if (!basicBlock->backEdges.empty()) {
247258
fmt::format_to(std::back_inserter(buf), "# backedges\n");
@@ -379,16 +390,7 @@ string BasicBlock::toTextualString(const core::GlobalState &gs, optional<core::F
379390
for (const Binding &exp : this->exprs) {
380391
string positionText = "";
381392
if (file) {
382-
if (exp.loc.exists() && !exp.loc.empty()) {
383-
auto lineCol = core::Loc(file.value(), exp.loc).position(gs);
384-
positionText =
385-
lineCol.first.line == lineCol.second.line
386-
? fmt::format(" @ {}:{}-{}", lineCol.first.line, lineCol.first.column, lineCol.second.column)
387-
: fmt::format(" @ {}:{}-{}:{}", lineCol.first.line, lineCol.first.column, lineCol.second.line,
388-
lineCol.second.column);
389-
} else {
390-
positionText = " @ <>";
391-
}
393+
positionText = fmt::format(" @ {}", core::Loc(file.value(), exp.loc).showRawLineColumn(gs));
392394
}
393395

394396
fmt::format_to(std::back_inserter(buf), " {}{} = {}\n", exp.bind.toString(gs, cfg), positionText,

‎class_flatten/class_flatten.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,9 +111,9 @@ class ClassFlattenWalk {
111111
ast::MethodDef::ARGS_store args;
112112
args.emplace_back(ast::make_expression<ast::Local>(blkLoc, blkLocalVar));
113113

114-
auto init =
115-
ast::make_expression<ast::MethodDef>(classDef->declLoc, classDef->declLoc, sym, core::Names::staticInit(),
116-
std::move(args), std::move(inits), ast::MethodDef::Flags());
114+
auto init = ast::make_expression<ast::MethodDef>(classDef->declLoc, classDef->declLoc, core::LocOffsets::none(),
115+
sym, core::Names::staticInit(), std::move(args),
116+
std::move(inits), ast::MethodDef::Flags());
117117
ast::cast_tree_nonnull<ast::MethodDef>(init).flags.isRewriterSynthesized = false;
118118
ast::cast_tree_nonnull<ast::MethodDef>(init).flags.isSelfMethod = true;
119119

‎core/FoundDefinitions.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ struct FoundMethod final {
142142
core::NameRef name;
143143
core::LocOffsets loc;
144144
core::LocOffsets declLoc;
145+
core::LocOffsets nameLoc;
145146
std::vector<core::ParsedArg> parsedArgs;
146147
core::ArityHash arityHash;
147148
struct Flags {
@@ -161,7 +162,7 @@ struct FoundMethod final {
161162

162163
std::string toString(const core::GlobalState &gs, const FoundDefinitions &foundDefs, uint32_t id) const;
163164
};
164-
CheckSize(FoundMethod, 56, 8);
165+
CheckSize(FoundMethod, 64, 8);
165166

166167
struct FoundModifier {
167168
enum class Kind : uint8_t {

‎core/GlobalState.cc

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ struct MethodBuilder {
127127
};
128128

129129
MethodBuilder enterMethod(GlobalState &gs, ClassOrModuleRef klass, NameRef name) {
130-
return MethodBuilder{gs, gs.enterMethodSymbol(Loc::none(), klass, name)};
130+
return MethodBuilder{gs, gs.enterMethodSymbol(Loc::none(), klass, name, LocOffsets::none())};
131131
}
132132

133133
struct ParentLinearizationInformation {
@@ -303,7 +303,8 @@ void GlobalState::initEmpty() {
303303
ClassOrModuleRef klass;
304304
klass = synthesizeClass(core::Names::Constants::NoSymbol(), 0);
305305
ENFORCE(klass == Symbols::noClassOrModule());
306-
MethodRef method = enterMethodSymbol(Loc::none(), Symbols::noClassOrModule(), Names::noMethod());
306+
MethodRef method =
307+
enterMethodSymbol(Loc::none(), Symbols::noClassOrModule(), Names::noMethod(), LocOffsets::none());
307308
ENFORCE(method == Symbols::noMethod());
308309
FieldRef field = enterFieldSymbol(Loc::none(), Symbols::noClassOrModule(), Names::noFieldOrStaticField());
309310
ENFORCE(field == Symbols::noField());
@@ -581,7 +582,7 @@ void GlobalState::initEmpty() {
581582
method = enterMethod(*this, Symbols::Class(), Names::new_()).repeatedArg(Names::args()).build();
582583
ENFORCE(method == Symbols::Class_new());
583584

584-
method = enterMethodSymbol(Loc::none(), Symbols::noClassOrModule(), Names::TodoMethod());
585+
method = enterMethodSymbol(Loc::none(), Symbols::noClassOrModule(), Names::TodoMethod(), LocOffsets::none());
585586
enterMethodArgumentSymbol(Loc::none(), method, Names::args());
586587
ENFORCE(method == Symbols::todoMethod());
587588

@@ -887,7 +888,7 @@ void GlobalState::installIntrinsics() {
887888
break;
888889
}
889890
auto countBefore = methodsUsed();
890-
auto method = enterMethodSymbol(Loc::none(), symbol, entry.method);
891+
auto method = enterMethodSymbol(Loc::none(), symbol, entry.method, LocOffsets::none());
891892
method.data(*this)->intrinsicOffset = offset + Method::FIRST_VALID_INTRINSIC_OFFSET;
892893
if (countBefore != methodsUsed()) {
893894
auto &blkArg = enterMethodArgumentSymbol(Loc::none(), method, Names::blkArg());
@@ -1192,7 +1193,7 @@ TypeArgumentRef GlobalState::enterTypeArgument(Loc loc, MethodRef owner, NameRef
11921193
return result;
11931194
}
11941195

1195-
MethodRef GlobalState::enterMethodSymbol(Loc loc, ClassOrModuleRef owner, NameRef name) {
1196+
MethodRef GlobalState::enterMethodSymbol(Loc loc, ClassOrModuleRef owner, NameRef name, LocOffsets nameLoc) {
11961197
ClassOrModuleData ownerScope = owner.dataAllowingNone(*this);
11971198
histogramInc("symbol_enter_by_name", ownerScope->members().size());
11981199

@@ -1211,6 +1212,7 @@ MethodRef GlobalState::enterMethodSymbol(Loc loc, ClassOrModuleRef owner, NameRe
12111212

12121213
MethodData data = result.dataAllowingNone(*this);
12131214
data->name = name;
1215+
data->nameLoc = nameLoc;
12141216
data->owner = owner;
12151217
data->addLoc(*this, loc);
12161218
DEBUG_ONLY(categoryCounterInc("symbols", "method"));
@@ -1225,7 +1227,7 @@ MethodRef GlobalState::enterNewMethodOverload(Loc sigLoc, MethodRef original, co
12251227
core::Loc loc = num == 0 ? original.data(*this)->loc()
12261228
: sigLoc; // use original Loc for main overload so that we get right jump-to-def for it.
12271229
auto owner = original.data(*this)->owner;
1228-
auto res = enterMethodSymbol(loc, owner, name);
1230+
auto res = enterMethodSymbol(loc, owner, name, original.data(*this)->nameLoc);
12291231
bool newMethod = res != original;
12301232
const auto &resArguments = res.data(*this)->arguments;
12311233
ENFORCE(newMethod || !resArguments.empty(), "must be at least the block arg");
@@ -2342,7 +2344,8 @@ const vector<shared_ptr<File>> &GlobalState::getFiles() const {
23422344

23432345
MethodRef GlobalState::staticInitForClass(ClassOrModuleRef klass, Loc loc) {
23442346
auto prevCount = methodsUsed();
2345-
auto sym = enterMethodSymbol(loc, klass.data(*this)->singletonClass(*this), core::Names::staticInit());
2347+
auto sym = enterMethodSymbol(loc, klass.data(*this)->singletonClass(*this), core::Names::staticInit(),
2348+
klass.data(*this)->loc().offsets());
23462349
if (prevCount != methodsUsed()) {
23472350
auto blkLoc = core::Loc::none(loc.file());
23482351
auto &blkSym = enterMethodArgumentSymbol(blkLoc, sym, core::Names::blkArg());
@@ -2361,7 +2364,7 @@ MethodRef GlobalState::lookupStaticInitForClass(ClassOrModuleRef klass, bool all
23612364
MethodRef GlobalState::staticInitForFile(Loc loc) {
23622365
auto nm = freshNameUnique(core::UniqueNameKind::Namer, core::Names::staticInit(), loc.file().id());
23632366
auto prevCount = this->methodsUsed();
2364-
auto sym = enterMethodSymbol(loc, core::Symbols::rootSingleton(), nm);
2367+
auto sym = enterMethodSymbol(loc, core::Symbols::rootSingleton(), nm, LocOffsets::none());
23652368
if (prevCount != this->methodsUsed()) {
23662369
auto blkLoc = core::Loc::none(loc.file());
23672370
auto &blkSym = this->enterMethodArgumentSymbol(blkLoc, sym, core::Names::blkArg());

‎core/GlobalState.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ class GlobalState final {
102102
ClassOrModuleRef enterClassSymbol(Loc loc, ClassOrModuleRef owner, NameRef name);
103103
TypeMemberRef enterTypeMember(Loc loc, ClassOrModuleRef owner, NameRef name, Variance variance);
104104
TypeArgumentRef enterTypeArgument(Loc loc, MethodRef owner, NameRef name, Variance variance);
105-
MethodRef enterMethodSymbol(Loc loc, ClassOrModuleRef owner, NameRef name);
105+
MethodRef enterMethodSymbol(Loc loc, ClassOrModuleRef owner, NameRef name, LocOffsets nameLoc);
106106
MethodRef enterNewMethodOverload(Loc loc, MethodRef original, core::NameRef originalName, uint32_t num,
107107
const std::vector<bool> &argsToKeep);
108108
FieldRef enterFieldSymbol(Loc loc, ClassOrModuleRef owner, NameRef name);

0 commit comments

Comments
 (0)