Some refactoring and cleanup (#1806)

* Use clang-tidy `misc-include-cleaner` for IWYU `#include` cleanup

* Use `std::optional<size_t>` instead of `ssize_t`

* Rename some functions in linkdefs.hpp

* Fix header order
This commit is contained in:
Rangi
2025-08-20 16:09:04 -04:00
committed by GitHub
parent 92ed6ece53
commit 3d155d5695
63 changed files with 271 additions and 118 deletions

View File

@@ -4,16 +4,17 @@
#include <deque>
#include <inttypes.h>
#include <optional>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <string>
#include <vector>
#include "diagnostics.hpp"
#include "helpers.hpp"
#include "itertools.hpp"
#include "linkdefs.hpp"
#include "platform.hpp"
#include "verbosity.hpp"
#include "link/main.hpp"
@@ -38,7 +39,7 @@ static std::vector<std::deque<FreeSpace>> memory[SECTTYPE_INVALID];
// Init the free space-modelling structs
static void initFreeSpace() {
for (SectionType type : EnumSeq(SECTTYPE_INVALID)) {
memory[type].resize(nbbanks(type));
memory[type].resize(sectTypeBanks(type));
for (std::deque<FreeSpace> &bankMem : memory[type]) {
bankMem.push_back({
.address = sectionTypeInfo[type].startAddr,
@@ -114,8 +115,8 @@ static MemoryLocation getStartLocation(Section const &section) {
}
// Returns a suitable free space index into `memory[section->type]` at which to place the given
// section, or -1 if none was found.
static ssize_t getPlacement(Section const &section, MemoryLocation &location) {
// section, or `std::nullopt` if none was found.
static std::optional<size_t> getPlacement(Section const &section, MemoryLocation &location) {
SectionTypeInfo const &typeInfo = sectionTypeInfo[section.type];
// Switch to the beginning of the next bank
@@ -169,7 +170,7 @@ static ssize_t getPlacement(Section const &section, MemoryLocation &location) {
// Try scrambled banks in descending order until no bank in the scrambled range is
// available. Otherwise, try in ascending order.
if (section.isBankFixed) {
return -1;
return std::nullopt;
} else if (options.scrambleROMX && section.type == SECTTYPE_ROMX
&& location.bank <= options.scrambleROMX) {
if (location.bank > typeInfo.firstBank) {
@@ -177,7 +178,7 @@ static ssize_t getPlacement(Section const &section, MemoryLocation &location) {
} else if (options.scrambleROMX < typeInfo.lastBank) {
location.bank = options.scrambleROMX + 1;
} else {
return -1;
return std::nullopt;
}
} else if (options.scrambleWRAMX && section.type == SECTTYPE_WRAMX
&& location.bank <= options.scrambleWRAMX) {
@@ -186,7 +187,7 @@ static ssize_t getPlacement(Section const &section, MemoryLocation &location) {
} else if (options.scrambleWRAMX < typeInfo.lastBank) {
location.bank = options.scrambleWRAMX + 1;
} else {
return -1;
return std::nullopt;
}
} else if (options.scrambleSRAM && section.type == SECTTYPE_SRAM
&& location.bank <= options.scrambleSRAM) {
@@ -195,12 +196,12 @@ static ssize_t getPlacement(Section const &section, MemoryLocation &location) {
} else if (options.scrambleSRAM < typeInfo.lastBank) {
location.bank = options.scrambleSRAM + 1;
} else {
return -1;
return std::nullopt;
}
} else if (location.bank < typeInfo.lastBank) {
++location.bank;
} else {
return -1;
return std::nullopt;
}
return getPlacement(section, location); // Tail recursion
@@ -210,7 +211,7 @@ static std::string getSectionDescription(Section const &section) {
std::string where;
char bank[8], addr[8], mask[8], offset[8];
if (section.isBankFixed && nbbanks(section.type) != 1) {
if (section.isBankFixed && sectTypeBanks(section.type) != 1) {
snprintf(bank, sizeof(bank), "%02" PRIx32, section.bank);
}
if (section.isAddressFixed) {
@@ -221,7 +222,7 @@ static std::string getSectionDescription(Section const &section) {
snprintf(offset, sizeof(offset), "%" PRIx16, section.alignOfs);
}
if (section.isBankFixed && nbbanks(section.type) != 1) {
if (section.isBankFixed && sectTypeBanks(section.type) != 1) {
if (section.isAddressFixed) {
where = "at $";
where += bank;
@@ -272,10 +273,10 @@ static void placeSection(Section &section) {
// Place section using first-fit decreasing algorithm
// https://en.wikipedia.org/wiki/Bin_packing_problem#First-fit_algorithm
MemoryLocation location = getStartLocation(section);
if (ssize_t spaceIdx = getPlacement(section, location); spaceIdx != -1) {
if (std::optional<size_t> spaceIdx = getPlacement(section, location); spaceIdx) {
std::deque<FreeSpace> &bankMem =
memory[section.type][location.bank - sectionTypeInfo[section.type].firstBank];
FreeSpace &freeSpace = bankMem[spaceIdx];
FreeSpace &freeSpace = bankMem[*spaceIdx];
assignSection(section, location);
@@ -285,17 +286,17 @@ static void placeSection(Section &section) {
bool noRightSpace = freeSpace.address + freeSpace.size == sectionEnd;
if (noLeftSpace && noRightSpace) {
// The free space is entirely deleted
bankMem.erase(bankMem.begin() + spaceIdx);
bankMem.erase(bankMem.begin() + *spaceIdx);
} else if (!noLeftSpace && !noRightSpace) {
// The free space is split in two
// Append the new space after the original one
uint16_t size = static_cast<uint16_t>(freeSpace.address + freeSpace.size - sectionEnd);
bankMem.insert(bankMem.begin() + spaceIdx + 1, {.address = sectionEnd, .size = size});
bankMem.insert(bankMem.begin() + *spaceIdx + 1, {.address = sectionEnd, .size = size});
// **`freeSpace` cannot be reused from this point on, because `bankMem.insert`
// invalidates all references to itself!**
// Resize the original space (address is unmodified)
bankMem[spaceIdx].size = section.org - bankMem[spaceIdx].address;
bankMem[*spaceIdx].size = section.org - bankMem[*spaceIdx].address;
} else {
// The amount of free spaces doesn't change: resize!
freeSpace.size -= section.size;
@@ -315,7 +316,7 @@ static void placeSection(Section &section) {
sectionTypeInfo[section.type].name.c_str(),
getSectionDescription(section).c_str()
);
} else if (section.org + section.size > endaddr(section.type) + 1) {
} else if (section.org + section.size > sectTypeEndAddr(section.type) + 1) {
// If the section just can't fit the bank, report that
fatal(
"Unable to place \"%s\" (%s section) %s: section runs past end of region ($%04x > "
@@ -324,7 +325,7 @@ static void placeSection(Section &section) {
sectionTypeInfo[section.type].name.c_str(),
getSectionDescription(section).c_str(),
section.org + section.size,
endaddr(section.type) + 1
sectTypeEndAddr(section.type) + 1
);
} else {
// Otherwise there is overlap with another section

View File

@@ -1,8 +1,14 @@
#include "link/fstack.hpp"
#include <inttypes.h>
#include <stdint.h>
#include <string>
#include <utility>
#include <variant>
#include <vector>
#include "backtrace.hpp"
#include "helpers.hpp"
#include "linkdefs.hpp"
#include "link/warning.hpp"

View File

@@ -5,10 +5,11 @@
#include <array>
#include <bit>
#include <inttypes.h>
#include <stdint.h>
#include <vector>
#include "helpers.hpp"
#include "util.hpp"
#include "linkdefs.hpp"
#include "link/section.hpp"
#include "link/warning.hpp"
@@ -30,7 +31,7 @@ static void setActiveTypeAndIdx(SectionType type, uint32_t idx) {
}
void layout_SetFloatingSectionType(SectionType type) {
if (nbbanks(type) == 1) {
if (sectTypeBanks(type) == 1) {
// There is only a single bank anyway, so just set the index to 0.
setActiveTypeAndIdx(type, 0);
} else {
@@ -45,7 +46,7 @@ void layout_SetFloatingSectionType(SectionType type) {
}
void layout_SetSectionType(SectionType type) {
if (nbbanks(type) != 1) {
if (sectTypeBanks(type) != 1) {
scriptError("A bank number must be specified for %s", sectionTypeInfo[type].name.c_str());
// Keep going with a default value for the bank index.
}
@@ -91,14 +92,14 @@ void layout_SetAddr(uint32_t addr) {
if (addr < pc) {
scriptError("Cannot decrease the current address (from $%04x to $%04x)", pc, addr);
} else if (addr > endaddr(activeType)) { // Allow "one past the end" sections.
} else if (addr > sectTypeEndAddr(activeType)) { // Allow "one past the end" sections.
scriptError(
"Cannot set the current address to $%04" PRIx32 ": %s ends at $%04" PRIx16,
addr,
typeInfo.name.c_str(),
endaddr(activeType)
sectTypeEndAddr(activeType)
);
pc = endaddr(activeType);
pc = sectTypeEndAddr(activeType);
} else {
pc = addr;
}
@@ -178,7 +179,7 @@ void layout_AlignTo(uint32_t alignment, uint32_t alignOfs) {
", past $%04" PRIx16,
pc,
static_cast<uint16_t>(pc + length),
static_cast<uint16_t>(endaddr(activeType) + 1)
static_cast<uint16_t>(sectTypeEndAddr(activeType) + 1)
);
return;
}
@@ -206,7 +207,7 @@ void layout_Pad(uint32_t length) {
"Cannot increase the current address by %u bytes: only %u bytes to $%04" PRIx16,
length,
typeInfo.size - offset,
static_cast<uint16_t>(endaddr(activeType) + 1)
static_cast<uint16_t>(sectTypeEndAddr(activeType) + 1)
);
} else {
pc += length;
@@ -232,13 +233,13 @@ void layout_PlaceSection(std::string const &name, bool isOptional) {
// Check that the linker script doesn't contradict what the code says.
if (section->type == SECTTYPE_INVALID) {
// A section that has data must get assigned a type that requires data.
if (!sect_HasData(activeType) && !section->data.empty()) {
if (!sectTypeHasData(activeType) && !section->data.empty()) {
scriptError(
"\"%s\" is specified to be a %s section, but it contains data",
name.c_str(),
typeInfo.name.c_str()
);
} else if (sect_HasData(activeType) && section->data.empty() && section->size != 0) {
} else if (sectTypeHasData(activeType) && section->data.empty() && section->size != 0) {
// A section that lacks data can only be assigned to a type that requires data
// if it's empty.
scriptError(

View File

@@ -2,16 +2,19 @@
#include "link/lexer.hpp"
#include <array>
#include <errno.h>
#include <fstream>
#include <inttypes.h>
#include <ios>
#include <stdint.h>
#include <stdio.h>
#include <string.h>
#include <string>
#include <utility>
#include <vector>
#include "backtrace.hpp"
#include "helpers.hpp"
#include "itertools.hpp"
#include "linkdefs.hpp"
#include "util.hpp"
#include "link/warning.hpp"

View File

@@ -1,11 +1,12 @@
// SPDX-License-Identifier: MIT
#include <sys/stat.h>
#include <sys/types.h>
#include "link/main.hpp"
#include <inttypes.h>
#include <limits.h>
#include <stdarg.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <utility>
@@ -13,9 +14,7 @@
#include "backtrace.hpp"
#include "diagnostics.hpp"
#include "extern/getopt.hpp"
#include "helpers.hpp" // assume
#include "itertools.hpp"
#include "platform.hpp"
#include "linkdefs.hpp"
#include "script.hpp" // Generated from script.y
#include "style.hpp"
#include "usage.hpp"
@@ -29,7 +28,6 @@
#include "link/output.hpp"
#include "link/patch.hpp"
#include "link/section.hpp"
#include "link/symbol.hpp"
#include "link/warning.hpp"
Options options;

View File

@@ -11,18 +11,18 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <string>
#include <utility>
#include <variant>
#include <vector>
#include "diagnostics.hpp"
#include "helpers.hpp"
#include "linkdefs.hpp"
#include "platform.hpp"
#include "verbosity.hpp"
#include "version.hpp"
#include "link/assign.hpp"
#include "link/fstack.hpp"
#include "link/main.hpp"
#include "link/patch.hpp"
#include "link/sdas_obj.hpp"
#include "link/section.hpp"
@@ -360,7 +360,7 @@ static void readSection(
}
section.alignOfs = tmp;
if (sect_HasData(section.type)) {
if (sectTypeHasData(section.type)) {
if (section.size) {
section.data.resize(section.size);
if (fread(section.data.data(), 1, section.size, file) != section.size) {
@@ -549,7 +549,7 @@ void obj_ReadFile(char const *fileName, unsigned int fileID) {
// Give patches' PC section pointers to their sections
for (uint32_t i = 0; i < nbSections; ++i) {
if (sect_HasData(fileSections[i]->type)) {
if (sectTypeHasData(fileSections[i]->type)) {
for (Patch &patch : fileSections[i]->patches) {
linkPatchToPCSect(patch, fileSections);
}

View File

@@ -6,9 +6,12 @@
#include <deque>
#include <errno.h>
#include <inttypes.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <tuple>
#include <variant>
#include <vector>
#include "diagnostics.hpp"

View File

@@ -5,14 +5,15 @@
#include <deque>
#include <inttypes.h>
#include <stdint.h>
#include <variant>
#include <vector>
#include "diagnostics.hpp"
#include "helpers.hpp" // assume
#include "linkdefs.hpp"
#include "opmath.hpp"
#include "verbosity.hpp"
#include "link/main.hpp"
#include "link/section.hpp"
#include "link/symbol.hpp"
#include "link/warning.hpp"
@@ -566,7 +567,7 @@ static void applyFilePatches(Section &section, Section &dataSection) {
// Applies all of a section's patches, iterating over "components" of unionized sections
static void applyPatches(Section &section) {
if (!sect_HasData(section.type)) {
if (!sectTypeHasData(section.type)) {
return;
}

View File

@@ -6,16 +6,17 @@
#include <inttypes.h>
#include <memory>
#include <stdint.h>
#include <stdio.h>
#include <string.h>
#include <utility>
#include <variant>
#include <vector>
#include "helpers.hpp" // assume, literal_strlen
#include "linkdefs.hpp"
#include "platform.hpp"
#include "link/assign.hpp"
#include "link/fstack.hpp"
#include "link/main.hpp"
#include "link/section.hpp"
#include "link/symbol.hpp"
#include "link/warning.hpp"
@@ -865,7 +866,7 @@ void sdobj_ReadFile(FileStackNode const &src, FILE *file, std::vector<Symbol> &f
// Otherwise, how would the type already be known at this point?
assume(section->isAddressFixed);
if (!sect_HasData(section->type)) {
if (!sectTypeHasData(section->type)) {
if (!section->data.empty()) {
fatalAt(
where,

View File

@@ -3,13 +3,19 @@
#include "link/section.hpp"
#include <inttypes.h>
#include <memory>
#include <stdint.h>
#include <stdlib.h>
#include <string.h>
#include <string>
#include <unordered_map>
#include <utility>
#include <vector>
#include "diagnostics.hpp"
#include "helpers.hpp"
#include "linkdefs.hpp"
#include "link/main.hpp"
#include "link/warning.hpp"
static std::vector<std::unique_ptr<Section>> sectionList;
@@ -167,7 +173,7 @@ static void mergeSections(Section &target, std::unique_ptr<Section> &&other) {
// Append `other` to `target`
other->offset = target.size;
target.size += other->size;
// Normally we'd check that `sect_HasData`, but SDCC areas may be `_INVALID` here
// Normally we'd check that `sectTypeHasData`, but SDCC areas may be `_INVALID` here
if (!other->data.empty()) {
target.data.insert(target.data.end(), RANGE(other->data));
// Adjust patches' PC offsets
@@ -195,7 +201,7 @@ void sect_AddSection(std::unique_ptr<Section> &&section) {
// Check if the section already exists
if (Section *target = sect_GetSection(section->name); target) {
mergeSections(*target, std::move(section));
} else if (section->modifier == SECTION_UNION && sect_HasData(section->type)) {
} else if (section->modifier == SECTION_UNION && sectTypeHasData(section->type)) {
fatal(
"Section \"%s\" is of type `%s`, which cannot be `UNION`ized",
section->name.c_str(),
@@ -315,23 +321,23 @@ static void doSanityChecks(Section &section) {
// Ensure the target address is valid
if (section.org < sectionTypeInfo[section.type].startAddr
|| section.org > endaddr(section.type)) {
|| section.org > sectTypeEndAddr(section.type)) {
error(
"Section \"%s\"'s fixed address $%04" PRIx16 " is outside of range [$%04" PRIx16
"; $%04" PRIx16 "]",
section.name.c_str(),
section.org,
sectionTypeInfo[section.type].startAddr,
endaddr(section.type)
sectTypeEndAddr(section.type)
);
}
if (section.org + section.size > endaddr(section.type) + 1) {
if (section.org + section.size > sectTypeEndAddr(section.type) + 1) {
error(
"Section \"%s\"'s end address $%04x is greater than last address $%04x",
section.name.c_str(),
section.org + section.size,
endaddr(section.type) + 1
sectTypeEndAddr(section.type) + 1
);
}
}

View File

@@ -2,12 +2,17 @@
#include "link/symbol.hpp"
#include <inttypes.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string>
#include <unordered_map>
#include <utility>
#include <variant>
#include <vector>
#include "helpers.hpp" // assume
#include "linkdefs.hpp"
#include "link/fstack.hpp"
#include "link/section.hpp"

View File

@@ -4,7 +4,11 @@
#include <inttypes.h>
#include <stdarg.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include "diagnostics.hpp"
#include "style.hpp"
#include "link/fstack.hpp"