jake/SplitBit-Emulator
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Name the rules, say when there is nothing to say, and hold a baseline

Browse Source 
Four things SplitLint wanted, and they build on each other.

EVERY WARNING NAMES ITS RULE, in brackets at the end the way a compiler
names the flag that produced it. Twelve rules, listed by --help. That makes
the other three possible: suppressions can name one rule and leave the line
honest about the others, the harness can assert on a rule's identity rather
than on the wording of its message, and --machine can print one tab
separated line per warning - file, line, rule, message, help - so nothing
downstream reads prose. This file's own output was parsed with regular
expressions three times in one day before it had a shape to rely on.

A CLEAN RUN SAYS SO:

  No style warnings: 121 files checked against 12 rules.

It used to exit in silence, which does not tell you it found nothing - it
tells you nothing at all, and from outside the two are identical.

A MARKER THAT SILENCES NOTHING IS ITSELF REPORTED, as dead-suppression. An
exception that outlived whatever made it necessary is the thing the
required reason exists to prevent, and naming the wrong rule now gets you
both the warning you meant to silence and a note that your suppression is
doing nothing.

AND THE CORPUS IS HELD TO A BASELINE. Sixty one warnings are left in it
deliberately and nothing stopped a sixty second. Tests/lint-baseline.txt
records how many of each rule each file should produce, so a new one fails
make test while the sixty one stay quiet; confirmed by adding an INIA 0d0
to Say.asm and watching it name the file, the rule and the count. It counts
per file and rule rather than recording line numbers, because line numbers
would churn the whole baseline whenever anything was inserted above a
warning - the same reason cycle counts are stripped from recorded output
here. ./Tests/lint.sh --bless records it again.

One thing to know for next time: the rule name was inserted before the line
number at all twenty one call sites, and the signature was changed to match
rather than the twenty one call sites being fixed. (path, rule, line) reads
no worse than (path, line, rule) and one edit has fewer ways to go wrong
than twenty one.
This commit is contained in:
Anachronaut
2026-08-26 21:00:21 -04:00
parent 9c144469b4
commit 0a2965bc63
4 changed files with 259 additions and 65 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Source/Linter/Linter.c
+168 -62
View File
@@ -16,6 +16,16 @@
#define LINE_CAPACITY 1024
#define TOKEN_CAPACITY 64
#define RULE_CAPACITY 32
// Every rule this knows, named once. The count in the clean run message comes from here,
// so a rule added without a name here is a rule the summary does not know about.
static const char *const ruleNames[] = {
"redundant-setd", "redundant-assignment", "pointer-offset", "redundant-ccf",
"known-branch", "zero-load", "dead-assignment", "q-through-stack", "self-push-pop",
"branch-to-next", "unreachable", "dead-suppression",
};
#define RULE_COUNT ((int)(sizeof(ruleNames) / sizeof(ruleNames[0])))
typedef struct {
char mnemonic[TOKEN_CAPACITY];
@@ -53,13 +63,25 @@ typedef enum {
} KnownCarry;
static void usage(const char *name) {
printf("Usage: %s [--fatal-warnings] <sourcefile> [sourcefile ...]\n", name);
printf("Usage: %s [options] <sourcefile> [sourcefile ...]\n", name);
printf("\n");
printf("Checks SplitBit assembly source for correct but needlessly long forms.\n");
printf("Warnings do not fail the command unless --fatal-warnings is given.\n");
printf("\n");
printf("A line whose comment says \"splitlint: <reason>\" is not reported on. The\n");
printf(" --fatal-warnings Exit non-zero if anything is reported.\n");
printf(" --machine One tab separated line per warning and nothing else:\n");
printf(" file, line, rule, message, help.\n");
printf("\n");
printf("A line whose comment says \"splitlint: <reason>\" is not reported on, and the\n");
printf("reason is required, so that a deliberate exception says what makes it one.\n");
printf("\"splitlint[rule]: <reason>\" silences one rule and leaves the line honest\n");
printf("about the others. A marker that silences nothing is itself reported.\n");
printf("\n");
printf("Rules:");
for (int i = 0; i < RULE_COUNT; i++) {
printf("%s %s", i && i % 4 == 0 ? "\n " : "", ruleNames[i]);
}
printf("\n");
}
static void uppercase(char *text) {
@@ -149,75 +171,135 @@ static SourceLine parseInstruction(char *line, int lineNumber, SourceInstruction
// ---- Saying that something is deliberate ----
//
// A line whose comment carries "splitlint: <reason>" is not reported on. The reason is
// REQUIRED, and that is the whole design: a suppression with no explanation is a way to
// make a tool quiet rather than a way to say something, and this file already has three
// real ones - branchTest exists to check that a branch whose outcome is known behaves
// correctly, so the diagnostic saying the outcome is known is exactly right and exactly
// unwanted.
// A line whose comment carries "splitlint: <reason>" is not reported on, and the reason is
// REQUIRED: a suppression with no explanation is a way to make a tool quiet rather than a
// way to say something. "splitlint[rule]: <reason>" silences one rule and leaves the line
// honest about every other, which matters once a line can trip more than one.
//
// Warnings are reported against a line that has already been read, sometimes an earlier
// one than the line in hand, so what is kept is the set of lines seen so far.
static int *suppressedLines = NULL;
typedef struct {
int line;
char rule[RULE_CAPACITY]; // Empty for "every rule on this line".
int used;
} Suppression;
static Suppression *suppressions = NULL;
static int suppressedCount = 0;
static int suppressedRoom = 0;
static int suppressionsUsed = 0;
static int machineReadable = 0;
static void forgetSuppressions(void) {
free(suppressedLines);
suppressedLines = NULL;
free(suppressions);
suppressions = NULL;
suppressedCount = 0;
suppressedRoom = 0;
}
static int rememberSuppression(int line) {
static int rememberSuppression(int line, const char *rule) {
if (suppressedCount == suppressedRoom) {
int room = suppressedRoom ? suppressedRoom * 2 : 16;
int *grown = realloc(suppressedLines, (size_t)room * sizeof(int));
Suppression *grown = realloc(suppressions, (size_t)room * sizeof(Suppression));
if (!grown) {
return 0;
}
suppressedLines = grown;
suppressions = grown;
suppressedRoom = room;
}
suppressedLines[suppressedCount++] = line;
suppressions[suppressedCount].line = line;
suppressions[suppressedCount].used = 0;
snprintf(suppressions[suppressedCount].rule, RULE_CAPACITY, "%s", rule ? rule : "");
suppressedCount++;
return 1;
}
static int isSuppressed(int line) {
static int isSuppressed(int line, const char *rule) {
for (int i = 0; i < suppressedCount; i++) {
if (suppressedLines[i] == line) {
return 1;
if (suppressions[i].line != line) {
continue;
}
if (suppressions[i].rule[0] != '\0'
&& strcmp(suppressions[i].rule, rule) != 0) {
continue; // Names a different rule, so this one still applies.
}
suppressions[i].used = 1;
suppressionsUsed++;
return 1;
}
return 0;
}
// A suppression that no longer suppresses anything is the thing the required reason was
// meant to prevent: an exception that outlived whatever made it necessary. Saying so is
// what keeps the count of them meaningful.
static int reportDeadSuppressions(const char *path) {
int dead = 0;
for (int i = 0; i < suppressedCount; i++) {
if (suppressions[i].used) {
continue;
}
if (machineReadable) {
printf("%s\t%d\t%s\t%s\t%s\n", path, suppressions[i].line,
"dead-suppression", "this line no longer has a warning to suppress",
"remove the splitlint comment");
} else {
printf("%s:%d: style: this line no longer has a warning to suppress"
" [dead-suppression]\n", path, suppressions[i].line);
printf(" help: remove the splitlint comment\n");
}
dead++;
}
return dead;
}
// The reason has to be there and has to say something. A bare marker is refused rather
// than honoured, because a suppression nobody explained is the one that outlives whatever
// made it necessary.
//
// Returns 1 if the line is marked, 0 if it is not, and -1 if it is marked with nothing.
static int suppressionOn(const char *rawLine) {
const char *at = strstr(rawLine, "splitlint:");
// A rule name in brackets, if there is one, is copied into `rule`.
static int suppressionOn(const char *rawLine, char *rule) {
rule[0] = '\0';
const char *at = strstr(rawLine, "splitlint");
if (!at) {
return 0;
}
at += strlen("splitlint:");
at += strlen("splitlint");
if (*at == '[') {
const char *close = strchr(at, ']');
if (!close || close == at + 1 || (size_t)(close - at - 1) >= RULE_CAPACITY) {
return -1;
}
memcpy(rule, at + 1, (size_t)(close - at - 1));
rule[close - at - 1] = '\0';
at = close + 1;
}
if (*at != ':') {
return 0; // "splitlint" inside ordinary prose is not a marker.
}
at++;
while (*at == ' ' || *at == '\t') {
at++;
}
return (*at == '\0' || *at == '\n' || *at == '\r') ? -1 : 1;
}
// Returns 1 if the warning was reported and 0 if the line said it was deliberate, so that
// a suppressed warning is not counted and --fatal-warnings does not fail on one.
static int warning(const char *path, int line, const char *message, const char *help) {
if (isSuppressed(line)) {
suppressionsUsed++;
static int warning(const char *path, const char *rule, int line, const char *message,
const char *help) {
if (isSuppressed(line, rule)) {
return 0;
}
printf("%s:%d: style: %s\n", path, line, message);
if (machineReadable) {
// One line, tab separated, so nothing downstream has to read prose. This file's
// own diagnostics were parsed with regular expressions three times in a day
// before it had a shape anything could rely on.
printf("%s\t%d\t%s\t%s\t%s\n", path, line, rule, message, help);
return 1;
}
// The rule goes in brackets at the end, the way a compiler names the flag that
// produced a warning, so that the sentence still reads as a sentence.
printf("%s:%d: style: %s [%s]\n", path, line, message, rule);
printf(" help: %s\n", help);
return 1;
}
@@ -302,7 +384,7 @@ static int lintKnownPointers(const char *path, const SourceInstruction *instruct
char message[180];
snprintf(message, sizeof(message), "DP%d is already known to hold %s",
selected, instruction->operand);
return warning(path, instruction->line, message, "remove the redundant SETD");
return warning(path, "redundant-setd", instruction->line, message, "remove the redundant SETD");
}
static void moveKnownPointer(KnownPointer *pointer, uint16_t amount, int downward) {
@@ -391,7 +473,7 @@ static int lintKnownRegisters(const char *path, const SourceInstruction *instruc
&& instruction->hasLiteral && known->a == instruction->literal))) {
snprintf(message, sizeof(message), "%s leaves A at its known value of %u",
instruction->spelling, known->a);
warnings += warning(path, instruction->line, message, "remove the redundant assignment");
warnings += warning(path, "redundant-assignment", instruction->line, message, "remove the redundant assignment");
}
if (known->bKnown && previousAssignment != 'B'
&& ((strcmp(instruction->mnemonic, "RSTB") == 0 && known->b == 0)
@@ -399,19 +481,19 @@ static int lintKnownRegisters(const char *path, const SourceInstruction *instruc
&& instruction->hasLiteral && known->b == instruction->literal))) {
snprintf(message, sizeof(message), "%s leaves B at its known value of %u",
instruction->spelling, known->b);
warnings += warning(path, instruction->line, message, "remove the redundant assignment");
warnings += warning(path, "redundant-assignment", instruction->line, message, "remove the redundant assignment");
}
if (known->aKnown && (strcmp(instruction->mnemonic, "DPUA") == 0
|| strcmp(instruction->mnemonic, "DPDA") == 0)) {
if (known->a == 0) {
warnings += warning(path, instruction->line, "pointer offset is known to be zero",
warnings += warning(path, "pointer-offset", instruction->line, "pointer offset is known to be zero",
"remove the pointer instruction");
} else if (known->a == 1) {
snprintf(help, sizeof(help), "use %s%s",
strcmp(instruction->mnemonic, "DPUA") == 0 ? "INCD" : "DECD",
selectorSuffix(instruction));
warnings += warning(path, instruction->line, "pointer offset is known to be one", help);
warnings += warning(path, "pointer-offset", instruction->line, "pointer offset is known to be one", help);
}
}
@@ -420,13 +502,13 @@ static int lintKnownRegisters(const char *path, const SourceInstruction *instruc
|| strcmp(instruction->mnemonic, "DPDW") == 0)) {
uint16_t word = ((uint16_t)known->a << 8) | known->b;
if (word == 0) {
warnings += warning(path, instruction->line, "word-sized pointer offset is known to be zero",
warnings += warning(path, "pointer-offset", instruction->line, "word-sized pointer offset is known to be zero",
"remove the pointer instruction");
} else if (word == 1) {
snprintf(help, sizeof(help), "use %s%s",
strcmp(instruction->mnemonic, "DPUW") == 0 ? "INCD" : "DECD",
selectorSuffix(instruction));
warnings += warning(path, instruction->line, "word-sized pointer offset is known to be one", help);
warnings += warning(path, "pointer-offset", instruction->line, "word-sized pointer offset is known to be one", help);
}
}
return warnings;
@@ -503,23 +585,23 @@ static int lintKnownCarry(const char *path, const SourceInstruction *instruction
return 0;
}
if (strcmp(instruction->mnemonic, "CCF") == 0 && carry == CARRY_CLEAR) {
return warning(path, instruction->line, "carry is already known to be clear",
return warning(path, "redundant-ccf", instruction->line, "carry is already known to be clear",
"remove the redundant CCF");
}
if (strcmp(instruction->mnemonic, "BRC") == 0) {
if (carry == CARRY_SET) {
return warning(path, instruction->line, "BRC is always taken because carry is known set",
return warning(path, "known-branch", instruction->line, "BRC is always taken because carry is known set",
"use BRI");
}
return warning(path, instruction->line, "BRC is never taken because carry is known clear",
return warning(path, "known-branch", instruction->line, "BRC is never taken because carry is known clear",
"remove the branch");
}
if (strcmp(instruction->mnemonic, "BNC") == 0) {
if (carry == CARRY_CLEAR) {
return warning(path, instruction->line, "BNC is always taken because carry is known clear",
return warning(path, "known-branch", instruction->line, "BNC is always taken because carry is known clear",
"use BRI");
}
return warning(path, instruction->line, "BNC is never taken because carry is known set",
return warning(path, "known-branch", instruction->line, "BNC is never taken because carry is known set",
"remove the branch");
}
return 0;
@@ -571,10 +653,10 @@ static int lintInstruction(const char *path, const SourceInstruction *previous,
if (current->hasLiteral && current->literal == 0
&& strcmp(current->mnemonic, "INIA") == 0) {
warnings += warning(path, current->line, "loading zero into A takes two bytes", "use RSTA");
warnings += warning(path, "zero-load", current->line, "loading zero into A takes two bytes", "use RSTA");
} else if (current->hasLiteral && current->literal == 0
&& strcmp(current->mnemonic, "INIB") == 0) {
warnings += warning(path, current->line, "loading zero into B takes two bytes", "use RSTB");
warnings += warning(path, "zero-load", current->line, "loading zero into B takes two bytes", "use RSTB");
}
char previousAssignment = assignedRegister(previous);
@@ -582,7 +664,7 @@ static int lintInstruction(const char *path, const SourceInstruction *previous,
char message[180];
snprintf(message, sizeof(message), "%s assigns %c, but %s replaces it immediately",
previous->spelling, previousAssignment, current->spelling);
warnings += warning(path, previous->line, message, "remove the first assignment");
warnings += warning(path, "dead-assignment", previous->line, message, "remove the first assignment");
}
// Q already has direct moves to both operand registers. The older stack transfer
@@ -591,19 +673,19 @@ static int lintInstruction(const char *path, const SourceInstruction *previous,
// the newer instruction was available.
if (strcmp(previous->mnemonic, "PSHQ") == 0
&& strcmp(current->mnemonic, "POPA") == 0) {
warnings += warning(path, previous->line, "moving Q to A through the stack takes two instructions",
warnings += warning(path, "q-through-stack", previous->line, "moving Q to A through the stack takes two instructions",
"use MVQA");
} else if (strcmp(previous->mnemonic, "PSHQ") == 0
&& strcmp(current->mnemonic, "POPB") == 0) {
warnings += warning(path, previous->line, "moving Q to B through the stack takes two instructions",
warnings += warning(path, "q-through-stack", previous->line, "moving Q to B through the stack takes two instructions",
"use MVQB");
} else if (strcmp(previous->mnemonic, "PSHA") == 0
&& strcmp(current->mnemonic, "POPA") == 0) {
warnings += warning(path, previous->line, "pushing A and immediately restoring it leaves A unchanged",
warnings += warning(path, "self-push-pop", previous->line, "pushing A and immediately restoring it leaves A unchanged",
"remove both instructions if the stack write is not intentional");
} else if (strcmp(previous->mnemonic, "PSHB") == 0
&& strcmp(current->mnemonic, "POPB") == 0) {
warnings += warning(path, previous->line, "pushing B and immediately restoring it leaves B unchanged",
warnings += warning(path, "self-push-pop", previous->line, "pushing B and immediately restoring it leaves B unchanged",
"remove both instructions if the stack write is not intentional");
}
@@ -641,7 +723,8 @@ static int lintFile(const char *path) {
// BEFORE THE COMMENT IS STRIPPED, because the marker lives in one. Recorded for
// the line rather than acted on here: a warning is reported against a line that
// has already been read, and not always the one in hand.
int marked = suppressionOn(line);
char suppressedRule[RULE_CAPACITY];
int marked = suppressionOn(line, suppressedRule);
if (marked < 0) {
fprintf(stderr, "%s:%d: error: splitlint: needs a reason after it\n",
path, lineNumber);
@@ -649,7 +732,7 @@ static int lintFile(const char *path) {
forgetSuppressions();
return -1;
}
if (marked && !rememberSuppression(lineNumber)) {
if (marked && !rememberSuppression(lineNumber, suppressedRule)) {
fprintf(stderr, "%s: error: out of memory recording a suppression\n", path);
fclose(file);
forgetSuppressions();
@@ -663,7 +746,7 @@ static int lintFile(const char *path) {
if (sourceLine == SOURCE_LABEL) {
if (havePendingBranch && current.spelling[0]
&& strcmp(pendingBranch.operand, current.spelling) == 0) {
warnings += warning(path, pendingBranch.line,
warnings += warning(path, "branch-to-next", pendingBranch.line,
"branch target is the next labeled address",
"remove the branch");
havePendingBranch = 0;
@@ -685,7 +768,7 @@ static int lintFile(const char *path) {
// other than a branch to the next address.
havePendingBranch = 0;
if (fallthroughStopped) {
warnings += warning(path, current.line,
warnings += warning(path, "unreachable", current.line,
"no ordinary fallthrough reaches this instruction",
"remove it, or give an intentional indirect entry point a label");
}
@@ -715,6 +798,7 @@ static int lintFile(const char *path) {
return -1;
}
fclose(file);
warnings += reportDeadSuppressions(path);
forgetSuppressions();
return warnings;
}
@@ -722,32 +806,54 @@ static int lintFile(const char *path) {
int main(int argc, char **argv) {
int fatalWarnings = 0;
int firstFile = 1;
if (argc > 1 && strcmp(argv[1], "--fatal-warnings") == 0) {
fatalWarnings = 1;
while (firstFile < argc && argv[firstFile][0] == '-' && argv[firstFile][1] == '-') {
if (strcmp(argv[firstFile], "--fatal-warnings") == 0) {
fatalWarnings = 1;
} else if (strcmp(argv[firstFile], "--machine") == 0) {
machineReadable = 1;
} else if (strcmp(argv[firstFile], "--help") == 0) {
usage(argv[0]);
return 0;
} else {
fprintf(stderr, "%s: error: unknown option \"%s\"\n", argv[0], argv[firstFile]);
return 2;
}
firstFile++;
}
if (argc <= firstFile || strcmp(argv[firstFile], "--help") == 0
|| strcmp(argv[firstFile], "-h") == 0) {
if (argc <= firstFile || strcmp(argv[firstFile], "-h") == 0) {
usage(argv[0]);
return argc <= firstFile ? 1 : 0;
}
int warnings = 0;
int files = 0;
for (int i = firstFile; i < argc; i++) {
int found = lintFile(argv[i]);
if (found < 0) {
return 2;
}
warnings += found;
files++;
}
if (warnings) {
printf("%d style warning%s found.\n", warnings, warnings == 1 ? "" : "s");
}
// Said out loud rather than left implicit. A suppression is a claim that something is
// deliberate, and a count of them is how anybody notices the claim has spread.
if (suppressionsUsed) {
printf("%d warning%s suppressed by splitlint comments.\n",
suppressionsUsed, suppressionsUsed == 1 ? "" : "s");
// Machine readable output is warnings and nothing else, so that a reader can take
// every line the same way rather than having to know which ones are prose.
if (!machineReadable) {
if (warnings) {
printf("%d style warning%s found.\n", warnings, warnings == 1 ? "" : "s");
} else {
// SAYING SO IS THE POINT. A tool that exits silently has not told you it
// found nothing, it has told you nothing at all - and the two look identical
// from outside. This says what it looked at and what it looked for.
printf("No style warnings: %d file%s checked against %d rule%s.\n",
files, files == 1 ? "" : "s",
RULE_COUNT, RULE_COUNT == 1 ? "" : "s");
}
// Said out loud rather than left implicit. A suppression is a claim that something
// is deliberate, and a count of them is how anybody notices the claim has spread.
if (suppressionsUsed) {
printf("%d warning%s suppressed by splitlint comments.\n",
suppressionsUsed, suppressionsUsed == 1 ? "" : "s");
}
}
return fatalWarnings && warnings ? 1 : 0;
}