Dismantle Decl a little bit more #20424
Conversation
We need special logic for updating line numbers anyway, so it's fine to just use absolute numbers here. This eliminates a field from `Decl`.
This is essentially just a rename. I also changed the representation of `AnalSubject` to use a `packed struct` rather than a non-exhaustive enum, but that change is relatively trivial.
|
@kcbanner, would you be able to confirm that this branch fixes your more complicated repro? |
|
@mlugg yep, this change resolves the issue! Thanks for the fix! |
|
AnalSubject? Not a good name. |
|
Do you have a better suggestion? |
|
AnalysisSubject? |
|
Too wordy. (I actually meant to call this |
|
a difference of 4 characters makes it too wordy? |
|
With all due respect, I'm not particularly interested in the opinions of general Zig users on the naming of an internal compiler datastructure which has been discussed and agreed upon elsewhere. |
|
With all due respect, your defense of "elsewhere" holds no water unless you actually show where and with who this was discussed and agreed upon. I have no opinion on this personally, but your cavalier attitude makes me question the accuracy and honesty of the response. |
From what I remember the abbreviation has been joked about between compiler developers when discussing planned changes on the Discord (channel
I personally think |
|
Thank you for the channel references, I will read through. I also do not have enough knowledge nor opinion to weigh in (I've certainly seen it used before). I simply found the proposed rationale strangely articulated and thought to express it. |
|
This is hilarious, with all due respect to naming conventions |
|
Just donated for your next pint at the pub. Thank you for keeping programming fun 🫶 |
This was going to be a bigger branch, but I accidentally regressed stack traces in my previous PR (sorry folks!) and this fixes it. Should be a pretty simple merge. The second commit here is a nop, just a rename I already had done.
The
return 0; // TODOinDecl.navSrcLinemight look sus, but rest assured the old logic was screwed, and no contemporary debugger uses the source line in this context as far as I'm aware, so making it screwed in a different way is fine for now.Line numbers for container-level declarations are no longer relative, because there is no advantage to this. Previously, they were relative, and
Declstored the computed absolute number -- this was objectively worse, as we still had the problem of having to update all those absolute numbers, but we were using an extra 4 bytes perDecl. With this system, we can track changes in line numbers more directly, avoidingDeclas a middle man. The only relative line numbers in ZIR now are indbg_stmtinstructions.This incidentally resolves #20407 -- I'm guessing that a line number was being treated as relative to the wrong thing somewhere. Whatever the issue was, this seems to have solved it!