Skip to content

Commit

Permalink
Refactor to avoid redundant obj_CheckAssertions function
Browse files Browse the repository at this point in the history
  • Loading branch information
Rangi42 committed Mar 27, 2024
1 parent dcb4e40 commit 506911d
Show file tree
Hide file tree
Showing 7 changed files with 38 additions and 54 deletions.
5 changes: 0 additions & 5 deletions include/link/object.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,6 @@
*/
void obj_ReadFile(char const *fileName, unsigned int i);

/*
* Evaluate all assertions
*/
void obj_CheckAssertions();

/*
* Sets up object file reading
* @param nbFiles The number of object files that will be read
Expand Down
17 changes: 1 addition & 16 deletions include/link/patch.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,11 @@
#ifndef RGBDS_LINK_PATCH_H
#define RGBDS_LINK_PATCH_H

#include <deque>
#include <stdint.h>
#include <vector>

#include "linkdefs.hpp"

#include "link/section.hpp"

struct Assertion {
Patch patch; // Also used for its `.type`
std::string message;
// This would be redundant with `.section->fileSymbols`, but `section` is sometimes `nullptr`!
std::vector<Symbol> *fileSymbols;
};

/*
* Checks all assertions
* @return true if assertion failed
*/
void patch_CheckAssertions(std::deque<Assertion> &assertions);
void patch_CheckAssertions();

/*
* Applies all SECTIONs' patches to them
Expand Down
11 changes: 11 additions & 0 deletions include/link/section.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

// GUIDELINE: external code MUST NOT BE AWARE of the data structure used!

#include <deque>
#include <memory>
#include <stdint.h>
#include <string>
Expand All @@ -13,6 +14,7 @@
#include "linkdefs.hpp"

#include "link/main.hpp"
#include "link/patch.hpp"

struct FileStackNode;
struct Section;
Expand Down Expand Up @@ -53,6 +55,15 @@ struct Section {
std::unique_ptr<Section> nextu; // The next "component" of this unionized sect
};

struct Assertion {
Patch patch; // Also used for its `.type`
std::string message;
// This would be redundant with `.section->fileSymbols`, but `section` is sometimes `nullptr`!
std::vector<Symbol> *fileSymbols;
};

extern std::deque<Assertion> assertions;

/*
* Execute a callback for each section currently registered.
* This is to avoid exposing the data structure in which sections are stored.
Expand Down
2 changes: 1 addition & 1 deletion src/link/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ int main(int argc, char *argv[]) {
if (nbErrors != 0)
reportErrors();
assign_AssignSections();
obj_CheckAssertions();
patch_CheckAssertions();

// and finally output the result.
patch_ApplyPatches();
Expand Down
5 changes: 0 additions & 5 deletions src/link/object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@

static std::deque<std::vector<Symbol>> symbolLists;
static std::vector<std::vector<FileStackNode>> nodes;
static std::deque<Assertion> assertions;

// Helper functions for reading object files

Expand Down Expand Up @@ -632,10 +631,6 @@ void obj_ReadFile(char const *fileName, unsigned int fileID) {
}
}

void obj_CheckAssertions() {
patch_CheckAssertions(assertions);
}

void obj_Setup(unsigned int nbFiles) {
nodes.resize(nbFiles);
}
51 changes: 25 additions & 26 deletions src/link/patch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,29 @@
#include "link/patch.hpp"

#include <assert.h>
#include <deque>
#include <inttypes.h>
#include <limits.h>
#include <stdint.h>
#include <stdlib.h>
#include <string.h>
#include <variant>
#include <vector>

#include "error.hpp"
#include "helpers.hpp"
#include "opmath.hpp"
#include "platform.hpp"

#include "linkdefs.hpp"

#include "link/main.hpp"
#include "link/object.hpp"
#include "link/section.hpp"
#include "link/symbol.hpp"

std::deque<Assertion> assertions;

struct RPNStackEntry {
int32_t value;
bool errorFlag; // Whether the value is a placeholder inserted for error recovery
Expand Down Expand Up @@ -88,10 +97,6 @@ static int32_t computeRPNExpr(Patch const &patch, std::vector<Symbol> const &fil
// So, if there are two `popRPN` in the same expression, make
// sure the operation is commutative.
switch (command) {
Symbol const *symbol;
char const *name;
Section const *sect;

case RPN_ADD:
value = popRPN(patch) + popRPN(patch);
break;
Expand Down Expand Up @@ -207,9 +212,8 @@ static int32_t computeRPNExpr(Patch const &patch, std::vector<Symbol> const &fil
value = 0;
for (uint8_t shift = 0; shift < 32; shift += 8)
value |= getRPNByte(expression, size, patch) << shift;
symbol = getSymbol(fileSymbols, value);

if (!symbol) {
if (Symbol const *symbol = getSymbol(fileSymbols, value); !symbol) {
error(
patch.src,
patch.lineNo,
Expand All @@ -232,16 +236,14 @@ static int32_t computeRPNExpr(Patch const &patch, std::vector<Symbol> const &fil
}
break;

case RPN_BANK_SECT:
case RPN_BANK_SECT: {
// `expression` is not guaranteed to be '\0'-terminated. If it is not,
// `getRPNByte` will have a fatal internal error.
name = (char const *)expression;
char const *name = (char const *)expression;
while (getRPNByte(expression, size, patch))
;

sect = sect_GetSection(name);

if (!sect) {
if (Section const *sect = sect_GetSection(name); !sect) {
error(
patch.src,
patch.lineNo,
Expand All @@ -254,6 +256,7 @@ static int32_t computeRPNExpr(Patch const &patch, std::vector<Symbol> const &fil
value = sect->bank;
}
break;
}

case RPN_BANK_SELF:
if (!patch.pcSection) {
Expand All @@ -265,15 +268,13 @@ static int32_t computeRPNExpr(Patch const &patch, std::vector<Symbol> const &fil
}
break;

case RPN_SIZEOF_SECT:
case RPN_SIZEOF_SECT: {
// This has assumptions commented in the `RPN_BANK_SECT` case above.
name = (char const *)expression;
char const *name = (char const *)expression;
while (getRPNByte(expression, size, patch))
;

sect = sect_GetSection(name);

if (!sect) {
if (Section const *sect = sect_GetSection(name); !sect) {
error(
patch.src,
patch.lineNo,
Expand All @@ -286,17 +287,15 @@ static int32_t computeRPNExpr(Patch const &patch, std::vector<Symbol> const &fil
value = sect->size;
}
break;
}

case RPN_STARTOF_SECT:
case RPN_STARTOF_SECT: {
// This has assumptions commented in the `RPN_BANK_SECT` case above.
name = (char const *)expression;
char const *name = (char const *)expression;
while (getRPNByte(expression, size, patch))
;

sect = sect_GetSection(name);
assert(sect->offset == 0);

if (!sect) {
if (Section const *sect = sect_GetSection(name); !sect) {
error(
patch.src,
patch.lineNo,
Expand All @@ -306,9 +305,11 @@ static int32_t computeRPNExpr(Patch const &patch, std::vector<Symbol> const &fil
isError = true;
value = 1;
} else {
assert(sect->offset == 0);
value = sect->org;
}
break;
}

case RPN_SIZEOF_SECTTYPE:
value = getRPNByte(expression, size, patch);
Expand Down Expand Up @@ -373,9 +374,7 @@ static int32_t computeRPNExpr(Patch const &patch, std::vector<Symbol> const &fil
value = patch.pcOffset + patch.pcSection->org;
}
} else {
symbol = getSymbol(fileSymbols, value);

if (!symbol) {
if (Symbol const *symbol = getSymbol(fileSymbols, value); !symbol) {
error(
patch.src,
patch.lineNo,
Expand Down Expand Up @@ -403,7 +402,7 @@ static int32_t computeRPNExpr(Patch const &patch, std::vector<Symbol> const &fil
return popRPN(patch);
}

void patch_CheckAssertions(std::deque<Assertion> &assertions) {
void patch_CheckAssertions() {
verbosePrint("Checking assertions...\n");

for (Assertion &assert : assertions) {
Expand Down
1 change: 0 additions & 1 deletion src/link/section.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
#include "link/section.hpp"

#include <assert.h>
#include <deque>
#include <inttypes.h>
#include <stdlib.h>
#include <string.h>
Expand Down

0 comments on commit 506911d

Please sign in to comment.