Submissions may not declare functions that are defined nowhere
The server compiles units one by one and never links, so a call to a function declared only by its prototype (an invented helper, or a member declared in a header and never written) compiled and could merge; main would not link. A newly declared function at file or class scope must be defined in the submission, exist elsewhere in the repository, or be a known function of the original (by name or fn_ ID). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
347ce4ea24
commit
b55b592a82
3 changed files with 166 additions and 0 deletions
111
src/server/undefined.js
Normal file
111
src/server/undefined.js
Normal file
|
|
@ -0,0 +1,111 @@
|
|||
// Functions a submission declares but nobody defines. The server compiles units one by one and
|
||||
// compares their code; it never links. A call to a function declared only by its prototype
|
||||
// compiles fine (an external reference) and no comparison notices, so an agent could "port" a
|
||||
// function by calling helpers that exist nowhere (CAsyncObjectCancel, LocaleStringToIndex), and
|
||||
// main would no longer link. A newly declared function must be defined in the submission, exist
|
||||
// elsewhere in the repository, or be a known function of the original (the symbol database).
|
||||
|
||||
const NOT_A_TYPE = new Set(['return', 'delete', 'throw', 'else', 'case', 'goto', 'sizeof', 'new', 'do', 'if', 'while', 'for', 'switch', 'co_return']);
|
||||
|
||||
// A file-scope or class-scope prototype on one line: `ret name(params) [const];`
|
||||
const PROTOTYPE = /^\s*((?:[A-Za-z_][\w:<>,*&\s]*?)[\s*&])([A-Za-z_]\w*)\s*\(([^;{}()]*(?:\([^()]*\)[^;{}()]*)*)\)\s*(?:const\s*)?(?:=\s*0\s*)?;\s*(?:\/\/.*)?$/;
|
||||
|
||||
function lineSet(text) {
|
||||
const m = new Map();
|
||||
for (const l of String(text ?? '').replace(/\r\n/g, '\n').split('\n')) m.set(l, (m.get(l) ?? 0) + 1);
|
||||
return m;
|
||||
}
|
||||
|
||||
/**
|
||||
* For each line, whether it is at a scope where a prototype declares a function: file, namespace,
|
||||
* extern "C" or a class body. Inside a function body `Type name(args);` is a variable.
|
||||
*/
|
||||
function declarationScopes(lines) {
|
||||
const out = [];
|
||||
const stack = []; // true for scopes that hold declarations
|
||||
let pendingDecl = false; // a class/struct/union/namespace/extern head waits for its brace
|
||||
let block = false;
|
||||
for (const raw of lines) {
|
||||
let line = raw;
|
||||
if (block) {
|
||||
const e = line.indexOf('*/');
|
||||
line = e < 0 ? '' : line.slice(e + 2);
|
||||
if (e >= 0) block = false;
|
||||
}
|
||||
line = line.replace(/\/\*.*?\*\//g, ' ').replace(/\/\/.*$/, '').replace(/"(?:[^"\\]|\\.)*"/g, '""').replace(/'(?:[^'\\]|\\.)*'/g, "''");
|
||||
const open = line.indexOf('/*');
|
||||
if (open >= 0) {
|
||||
block = true;
|
||||
line = line.slice(0, open);
|
||||
}
|
||||
out.push(stack.every(Boolean));
|
||||
if (/\b(?:class|struct|union|namespace)\b[^;()]*$/.test(line.replace(/template\s*<[^>]*>/g, '')) || /\bextern\s+"C"/.test(raw)) pendingDecl = true;
|
||||
for (const c of line) {
|
||||
if (c === '{') {
|
||||
stack.push(pendingDecl);
|
||||
pendingDecl = false;
|
||||
} else if (c === '}') stack.pop();
|
||||
else if (c === ';') pendingDecl = false;
|
||||
}
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
/** Names of functions declared on lines the submission added (not in the base version). */
|
||||
export function newPrototypes(after, before) {
|
||||
const old = lineSet(before);
|
||||
const seen = new Map();
|
||||
const out = new Set();
|
||||
const lines = String(after ?? '').replace(/\r\n/g, '\n').split('\n');
|
||||
const declScope = declarationScopes(lines);
|
||||
for (const [i, line] of lines.entries()) {
|
||||
seen.set(line, (seen.get(line) ?? 0) + 1);
|
||||
if (seen.get(line) <= (old.get(line) ?? 0) || !declScope[i]) continue;
|
||||
const m = PROTOTYPE.exec(line);
|
||||
if (!m) continue;
|
||||
const first = m[1].trim().split(/\s+/)[0];
|
||||
if (NOT_A_TYPE.has(first) || /=/.test(m[1])) continue;
|
||||
// A literal in the parentheses makes it an object constructed with arguments: CImVector c(0xFF, ...).
|
||||
if (/(?:^|[(,\s])(?:-?\d|'|")/.test(m[3])) continue;
|
||||
out.add(m[2]);
|
||||
}
|
||||
return [...out];
|
||||
}
|
||||
|
||||
/** Whether `text` has a definition (a body) of a function called `name`. */
|
||||
export function definesFunction(text, name) {
|
||||
const s = String(text ?? '');
|
||||
const re = new RegExp(`\\b${name}\\s*\\(`, 'g');
|
||||
let m;
|
||||
while ((m = re.exec(s))) {
|
||||
let depth = 0;
|
||||
let i = m.index + m[0].length - 1;
|
||||
for (; i < s.length; i++) {
|
||||
if (s[i] === '(') depth++;
|
||||
else if (s[i] === ')' && --depth === 0) break;
|
||||
}
|
||||
const rest = s.slice(i + 1, i + 200);
|
||||
if (/^\s*(?:const\s*)?(?:noexcept\s*)?(?::[^;{]*)?\{/.test(rest)) return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Functions the submission newly declares that are defined nowhere. `existsElsewhere(name)`:
|
||||
* the name occurs in the repository outside the submitted files; `known(name)`: a function of
|
||||
* the original has that name.
|
||||
*/
|
||||
export function undefinedDeclarations(files, { before, existsElsewhere, known }) {
|
||||
const declared = new Set();
|
||||
for (const [file, text] of Object.entries(files)) {
|
||||
if (typeof text !== 'string') continue;
|
||||
for (const n of newPrototypes(text, before(file))) declared.add(n);
|
||||
}
|
||||
const out = [];
|
||||
for (const name of declared) {
|
||||
if (Object.values(files).some((t) => typeof t === 'string' && definesFunction(t, name))) continue;
|
||||
if (known(name) || existsElsewhere(name)) continue;
|
||||
out.push(name);
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
|
@ -10,6 +10,7 @@ import { reconcile } from '../shared/reconcile.js';
|
|||
import { STATUS } from '../shared/compare.js';
|
||||
import { listSources, isUnit } from '../shared/project.js';
|
||||
import { changedNames, unitsMentioning } from './affected.js';
|
||||
import { undefinedDeclarations } from './undefined.js';
|
||||
import { className } from './grouping.js';
|
||||
import { now } from './db.js';
|
||||
import { returnBatch, batchAttempts, settleBatch } from './batches.js';
|
||||
|
|
@ -220,6 +221,21 @@ export class Verifier {
|
|||
return errors;
|
||||
}
|
||||
|
||||
/** Functions the submission declares that nothing defines (undefined.js). */
|
||||
undefinedGate(files, baseCommit) {
|
||||
const submitted = Object.keys(files);
|
||||
// A function of the original: by name, or by its stable ID (agents name unnamed ones fn_01234).
|
||||
const known = (name) => !!this.store.get('SELECT 1 FROM symbols WHERE id = ? OR name = ? OR name LIKE ? LIMIT 1', name, name, `%::${name}`);
|
||||
const existsElsewhere = (name) => {
|
||||
try {
|
||||
return this.repo.git(['grep', '-l', '-w', '-F', '-e', name, 'HEAD', '--', '.', ...submitted.map((f) => `:(exclude)${f}`)]).trim().length > 0;
|
||||
} catch {
|
||||
return false; // git grep exits 1 when nothing matches
|
||||
}
|
||||
};
|
||||
return undefinedDeclarations(files, { before: (f) => this.repo.show(baseCommit, f), existsElsewhere, known });
|
||||
}
|
||||
|
||||
// What a merge needs from the verification's check (without listings and byte diffs).
|
||||
rememberCheck(batchId, { check, rec, changedHeaders }) {
|
||||
const compact = {
|
||||
|
|
@ -396,6 +412,13 @@ export class Verifier {
|
|||
if (!this.repo.isAncestor(batch.base_commit)) return finishAll('rejected', { summary: `base commit ${batch.base_commit} is not part of main; sync and resubmit` });
|
||||
const textErrors = this.freeTextGate(submitted, batch.base_commit);
|
||||
if (textErrors.length) return finishAll('rejected', { summary: 'rejected: the submission adds comments or strings that are not allowed (see lintErrors)', lintErrors: textErrors });
|
||||
const undefinedNames = this.undefinedGate(submitted, batch.base_commit);
|
||||
if (undefinedNames.length) {
|
||||
return finishAll('needs_cleanup', {
|
||||
summary: `declares functions that are defined nowhere: ${undefinedNames.join(', ')}. Main is linked as a whole: call the real function of the original (td_lookup_symbol; td_new_symbol for one not in the database yet) and port it too if it has no code yet, or write the logic inline`,
|
||||
undefinedFunctions: undefinedNames,
|
||||
});
|
||||
}
|
||||
const merged = this.repo.mergeOntoHead(batch.base_commit, submitted);
|
||||
if (merged.conflicts.length) return finishAll('needs_cleanup', { summary: 'your change conflicts with newer work on main; sync, merge and resubmit', conflicts: merged.conflicts });
|
||||
|
||||
|
|
|
|||
32
test/undefined.test.js
Normal file
32
test/undefined.test.js
Normal file
|
|
@ -0,0 +1,32 @@
|
|||
import { test } from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import { newPrototypes, definesFunction, undefinedDeclarations } from '../src/server/undefined.js';
|
||||
|
||||
test('functions a submission declares but nobody defines are found', () => {
|
||||
const before = 'static void LoadAddOnMetadata(UIADDON* addon, const char* line) {\n}\n';
|
||||
const after = [
|
||||
'int32_t LocaleStringToIndex(const char* locale);',
|
||||
'int32_t LocaleIndexToClientLocale(int32_t index);',
|
||||
'void Helper(int a);',
|
||||
'void Known(int a);',
|
||||
'void Elsewhere(int a);',
|
||||
'static void LoadAddOnMetadata(UIADDON* addon, const char* line) {',
|
||||
' return Compute(addon);',
|
||||
' x = Other(1);',
|
||||
' ScopedGuard resultGuard(addon);',
|
||||
' std::vector<int> timeslots(16);',
|
||||
'}',
|
||||
'static const CImVector s_shipMarkerColor(0xFF, 0xFF, 0xFF, 0xFF);',
|
||||
'class Widget {',
|
||||
' void Member(int a);',
|
||||
'};',
|
||||
'void Helper(int a) {',
|
||||
'}',
|
||||
].join('\n');
|
||||
assert.deepEqual(newPrototypes(after, before).sort(), ['Elsewhere', 'Helper', 'Known', 'LocaleIndexToClientLocale', 'LocaleStringToIndex', 'Member'], 'locals constructed with arguments are not prototypes');
|
||||
assert.ok(definesFunction(after, 'Helper'));
|
||||
assert.ok(!definesFunction(after, 'LocaleStringToIndex'));
|
||||
const missing = undefinedDeclarations({ 'a.cpp': after }, { before: () => before, known: (n) => n === 'Known', existsElsewhere: (n) => n === 'Elsewhere' });
|
||||
assert.deepEqual(missing.sort(), ['LocaleIndexToClientLocale', 'LocaleStringToIndex', 'Member']);
|
||||
assert.deepEqual(undefinedDeclarations({ 'a.cpp': before }, { before: () => before, known: () => false, existsElsewhere: () => false }), [], 'nothing new, nothing to report');
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue