Skip to content

Commit c51e5cb

Browse files
ahundtahundt
committed
fix(extract,mcp): retain pointer returns and exact signature status
Both merge parents be89d49 and 97ce23f stored only the base type node for C/C++ functions and methods, so const char * and char ** were published as char. The feature parent also marked requested get_code signature output as truncated source and emitted repair-oriented clipping metadata. Add c_declarator_return_type_text at internal/cbm/extract_defs.c:3154 to retain leading qualifiers and every pointer_declarator in O(D) traversal time with O(return-type bytes) arena storage and checked size arithmetic. Route free-function and method return metadata through it. In src/mcp/mcp.c:13543, treat signature mode as a complete requested representation: retain total_lines, omit truncated/source_clipped/clipped_at_lines, and reuse one signature_mode predicate. Tests require const char *, char **, unchanged scalar int, C++ method parity, total_lines=300, and absence of false clipping fields. Verified extraction 329/329, C LSP 752/752, token_reduction 56/56, MCP 314/314 under ASan/UBSan; test-leak 1194/1194 with 0 leaks; explicit four-file test-syntax and source safety pass. Clang analyzer reports no diagnostics in changed production ranges. Signed-off-by: Andrew Hundt <[email protected]>
1 parent 111d426 commit c51e5cb

4 files changed

Lines changed: 199 additions & 8 deletions

File tree

‎internal/cbm/extract_defs.c‎

Lines changed: 118 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3151,6 +3151,122 @@ static TSNode find_c_params(TSNode func_node) {
31513151
return null_node;
31523152
}
31533153

3154+
static bool c_return_type_size_add(size_t *total, size_t addition) {
3155+
if (addition > SIZE_MAX - *total) {
3156+
return false;
3157+
}
3158+
*total += addition;
3159+
return true;
3160+
}
3161+
3162+
/* C-family grammars split a function's declared return type across sibling
3163+
* type/type_qualifier nodes and the declarator chain. Preserve the exact base
3164+
* spelling, leading qualifiers, and every pointer layer without imposing a
3165+
* depth cap: the walk follows one strict child chain, so runtime is O(D) and
3166+
* temporary memory is O(return-type bytes), where D is declarator depth. */
3167+
static char *c_declarator_return_type_text(CBMExtractCtx *ctx, TSNode func_node, TSNode type_node) {
3168+
uint32_t base_start = ts_node_start_byte(type_node);
3169+
uint32_t base_end = ts_node_end_byte(type_node);
3170+
if (base_end <= base_start) {
3171+
return cbm_node_text(ctx->arena, type_node, ctx->source);
3172+
}
3173+
3174+
TSNode declarator = ts_node_child_by_field_name(func_node, TS_FIELD("declarator"));
3175+
if (ts_node_is_null(declarator)) {
3176+
return cbm_node_text(ctx->arena, type_node, ctx->source);
3177+
}
3178+
3179+
size_t pointer_count = 0;
3180+
for (TSNode node = declarator; !ts_node_is_null(node);
3181+
node = ts_node_child_by_field_name(node, TS_FIELD("declarator"))) {
3182+
if (strcmp(ts_node_type(node), "pointer_declarator") == 0) {
3183+
if (pointer_count == SIZE_MAX) {
3184+
ctx->result->has_error = true;
3185+
ctx->result->error_msg =
3186+
cbm_arena_strdup(ctx->arena, "C return-type pointer depth exceeds size limit");
3187+
return cbm_node_text(ctx->arena, type_node, ctx->source);
3188+
}
3189+
pointer_count++;
3190+
}
3191+
}
3192+
3193+
uint32_t declarator_start = ts_node_start_byte(declarator);
3194+
size_t qualifier_bytes = 0;
3195+
size_t qualifier_count = 0;
3196+
uint32_t child_count = ts_node_named_child_count(func_node);
3197+
for (uint32_t i = 0; i < child_count; i++) {
3198+
TSNode child = ts_node_named_child(func_node, i);
3199+
if (strcmp(ts_node_type(child), "type_qualifier") != 0 ||
3200+
ts_node_end_byte(child) > declarator_start) {
3201+
continue;
3202+
}
3203+
uint32_t start = ts_node_start_byte(child);
3204+
uint32_t end = ts_node_end_byte(child);
3205+
if (end > start) {
3206+
size_t len = (size_t)(end - start);
3207+
if (len > SIZE_MAX - qualifier_bytes || qualifier_count == SIZE_MAX) {
3208+
ctx->result->has_error = true;
3209+
ctx->result->error_msg =
3210+
cbm_arena_strdup(ctx->arena, "C return-type qualifier size exceeds limit");
3211+
return cbm_node_text(ctx->arena, type_node, ctx->source);
3212+
}
3213+
qualifier_bytes += len;
3214+
qualifier_count++;
3215+
}
3216+
}
3217+
3218+
if (pointer_count == 0 && qualifier_count == 0) {
3219+
return cbm_node_text(ctx->arena, type_node, ctx->source);
3220+
}
3221+
3222+
size_t base_len = (size_t)(base_end - base_start);
3223+
size_t result_len = base_len;
3224+
if (!c_return_type_size_add(&result_len, qualifier_bytes) ||
3225+
!c_return_type_size_add(&result_len, qualifier_count) ||
3226+
(pointer_count > 0 && !c_return_type_size_add(&result_len, SKIP_ONE)) ||
3227+
!c_return_type_size_add(&result_len, pointer_count) ||
3228+
!c_return_type_size_add(&result_len, NULL_TERM)) {
3229+
ctx->result->has_error = true;
3230+
ctx->result->error_msg =
3231+
cbm_arena_strdup(ctx->arena, "C return-type metadata exceeds addressable size");
3232+
return cbm_node_text(ctx->arena, type_node, ctx->source);
3233+
}
3234+
char *result = cbm_arena_alloc(ctx->arena, result_len);
3235+
if (!result) {
3236+
ctx->result->has_error = true;
3237+
ctx->result->error_msg =
3238+
cbm_arena_strdup(ctx->arena, "C return-type metadata allocation failed");
3239+
return cbm_node_text(ctx->arena, type_node, ctx->source);
3240+
}
3241+
3242+
size_t pos = 0;
3243+
for (uint32_t i = 0; i < child_count; i++) {
3244+
TSNode child = ts_node_named_child(func_node, i);
3245+
if (strcmp(ts_node_type(child), "type_qualifier") != 0 ||
3246+
ts_node_end_byte(child) > declarator_start) {
3247+
continue;
3248+
}
3249+
uint32_t start = ts_node_start_byte(child);
3250+
uint32_t end = ts_node_end_byte(child);
3251+
if (end <= start) {
3252+
continue;
3253+
}
3254+
size_t len = (size_t)(end - start);
3255+
memcpy(result + pos, ctx->source + start, len);
3256+
pos += len;
3257+
result[pos++] = ' ';
3258+
}
3259+
memcpy(result + pos, ctx->source + base_start, base_len);
3260+
pos += base_len;
3261+
if (pointer_count > 0) {
3262+
result[pos++] = ' ';
3263+
memset(result + pos, '*', pointer_count);
3264+
pos += pointer_count;
3265+
}
3266+
result[pos] = '\0';
3267+
return result;
3268+
}
3269+
31543270
// C++: resolve trailing return type (auto f() -> Type) on a declarator node.
31553271
// Updates def->return_type and def->return_types if trailing type found.
31563272
static void resolve_cpp_trailing_return(CBMArena *a, TSNode func_node, const char *source,
@@ -3304,7 +3420,7 @@ static void extract_func_def(CBMExtractCtx *ctx, TSNode node, const CBMLangSpec
33043420
for (const char **f = rt_fields; *f; f++) {
33053421
TSNode rt = ts_node_child_by_field_name(func_node, *f, (uint32_t)strlen(*f));
33063422
if (!ts_node_is_null(rt)) {
3307-
def.return_type = cbm_node_text(a, rt, ctx->source);
3423+
def.return_type = c_declarator_return_type_text(ctx, func_node, rt);
33083424
def.return_types = extract_return_types(ctx, rt);
33093425
break;
33103426
}
@@ -4311,7 +4427,7 @@ static void push_method_def(CBMExtractCtx *ctx, TSNode child, TSNode class_node,
43114427
for (const char **f = rt_fields; *f; f++) {
43124428
TSNode rt = ts_node_child_by_field_name(child, *f, (uint32_t)strlen(*f));
43134429
if (!ts_node_is_null(rt)) {
4314-
def.return_type = cbm_node_text(a, rt, ctx->source);
4430+
def.return_type = c_declarator_return_type_text(ctx, child, rt);
43154431
break;
43164432
}
43174433
}

‎src/mcp/mcp.c‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13540,6 +13540,7 @@ static char *build_snippet_response(cbm_mcp_server_t *srv, cbm_node_t *node,
1354013540
int end = node->end_line > start ? node->end_line : start + SNIPPET_DEFAULT_LINES;
1354113541
int total_lines = end - start + 1;
1354213542
bool truncated = false;
13543+
bool signature_mode = mode && strcmp(mode, "signature") == 0;
1354313544
char *source = NULL;
1354413545
char *source_tail = NULL;
1354513546

@@ -13555,8 +13556,8 @@ static char *build_snippet_response(cbm_mcp_server_t *srv, cbm_node_t *node,
1355513556
bool path_ok = path_len >= 0 && (size_t)path_len < apsz &&
1355613557
cbm_path_within_root(root_path, abs_path);
1355713558
if (path_ok) {
13558-
if (mode && strcmp(mode, "signature") == 0) {
13559-
truncated = true;
13559+
if (signature_mode) {
13560+
/* Source omission is the requested representation, not truncation. */
1356013561
} else if (mode && strcmp(mode, "head_tail") == 0 && max_lines > 0 &&
1356113562
total_lines > max_lines) {
1356213563
int head_count = (max_lines * CBM_SNIPPET_HEAD_PERCENT) /
@@ -13600,7 +13601,7 @@ static char *build_snippet_response(cbm_mcp_server_t *srv, cbm_node_t *node,
1360013601
yyjson_mut_obj_add_int(doc, root_obj, "start_line", start);
1360113602
yyjson_mut_obj_add_int(doc, root_obj, "end_line", end);
1360213603

13603-
if (mode && strcmp(mode, "signature") == 0) {
13604+
if (signature_mode) {
1360413605
/* Signature mode: source omitted; signature comes from properties below */
1360513606
} else if (mode && strcmp(mode, "head_tail") == 0 && source && source_tail) {
1360613607
/* Combine head + marker + tail */
@@ -13641,6 +13642,10 @@ static char *build_snippet_response(cbm_mcp_server_t *srv, cbm_node_t *node,
1364113642
yyjson_mut_obj_add_bool(doc, root_obj, "source_clipped", true);
1364213643
yyjson_mut_obj_add_int(doc, root_obj, "clipped_at_lines", max_lines);
1364313644
yyjson_mut_obj_add_int(doc, root_obj, "total_lines", total_lines);
13645+
} else if (signature_mode) {
13646+
/* Preserve useful size context without claiming requested source was
13647+
* clipped or suggesting max_lines=0 is needed to repair the result. */
13648+
yyjson_mut_obj_add_int(doc, root_obj, "total_lines", total_lines);
1364413649
}
1364513650

1364613651
/* match_method — omitted for exact matches */
@@ -13652,7 +13657,7 @@ static char *build_snippet_response(cbm_mcp_server_t *srv, cbm_node_t *node,
1365213657
* props_doc is freed AFTER serialization since yyjson_mut_obj_add_str
1365313658
* stores pointers into it (zero-copy). */
1365413659
yyjson_doc *props_doc = NULL;
13655-
bool include_properties = !compact || (mode && strcmp(mode, "signature") == 0);
13660+
bool include_properties = !compact || signature_mode;
1365613661
if (include_properties && node->properties_json && node->properties_json[0] != '\0') {
1365713662
props_doc = yyjson_read(node->properties_json, strlen(node->properties_json), 0);
1365813663
if (props_doc) {

‎tests/test_extraction.c‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -908,6 +908,40 @@ TEST(c_function) {
908908
PASS();
909909
}
910910

911+
TEST(c_function_return_type_preserves_pointer_and_qualifier) {
912+
CBMFileResult *r =
913+
extract("static const char *text_end(const char *text) { return text; }\n"
914+
"char **table(void) { return 0; }\n"
915+
"int scalar(void) { return 0; }\n",
916+
CBM_LANG_C, "t", "returns.c");
917+
ASSERT_NOT_NULL(r);
918+
ASSERT_FALSE(r->has_error);
919+
920+
const CBMDefinition *text_end = NULL;
921+
const CBMDefinition *table = NULL;
922+
const CBMDefinition *scalar = NULL;
923+
for (int i = 0; i < r->defs.count; i++) {
924+
const CBMDefinition *def = &r->defs.items[i];
925+
if (strcmp(def->name, "text_end") == 0) {
926+
text_end = def;
927+
} else if (strcmp(def->name, "table") == 0) {
928+
table = def;
929+
} else if (strcmp(def->name, "scalar") == 0) {
930+
scalar = def;
931+
}
932+
}
933+
934+
ASSERT_NOT_NULL(text_end);
935+
ASSERT_NOT_NULL(table);
936+
ASSERT_NOT_NULL(scalar);
937+
ASSERT_STR_EQ(text_end->return_type, "const char *");
938+
ASSERT_STR_EQ(table->return_type, "char **");
939+
ASSERT_STR_EQ(scalar->return_type, "int");
940+
941+
cbm_free_result(r);
942+
PASS();
943+
}
944+
911945
TEST(c_struct) {
912946
CBMFileResult *r =
913947
extract("struct Point { int x; int y; };\nvoid init_point(struct Point *p) { p->x = 0; }\n",
@@ -932,6 +966,36 @@ TEST(cpp_class) {
932966
PASS();
933967
}
934968

969+
TEST(cpp_method_return_type_preserves_pointer_and_qualifier) {
970+
CBMFileResult *r =
971+
extract("class Text {\n"
972+
"public:\n"
973+
" const char *end() { return nullptr; }\n"
974+
" char **table() { return nullptr; }\n"
975+
"};\n",
976+
CBM_LANG_CPP, "t", "text.cpp");
977+
ASSERT_NOT_NULL(r);
978+
ASSERT_FALSE(r->has_error);
979+
980+
const CBMDefinition *end = NULL;
981+
const CBMDefinition *table = NULL;
982+
for (int i = 0; i < r->defs.count; i++) {
983+
const CBMDefinition *def = &r->defs.items[i];
984+
if (strcmp(def->name, "end") == 0) {
985+
end = def;
986+
} else if (strcmp(def->name, "table") == 0) {
987+
table = def;
988+
}
989+
}
990+
ASSERT_NOT_NULL(end);
991+
ASSERT_NOT_NULL(table);
992+
ASSERT_STR_EQ(end->return_type, "const char *");
993+
ASSERT_STR_EQ(table->return_type, "char **");
994+
995+
cbm_free_result(r);
996+
PASS();
997+
}
998+
935999
/* ═══════════════════════════════════════════════════════════════════
9361000
* Group C: Scripting / Dynamic Languages
9371001
* ═══════════════════════════════════════════════════════════════════ */
@@ -5643,8 +5707,10 @@ SUITE(extraction) {
56435707
RUN_TEST(dlang_struct);
56445708
RUN_TEST(zig_function);
56455709
RUN_TEST(c_function);
5710+
RUN_TEST(c_function_return_type_preserves_pointer_and_qualifier);
56465711
RUN_TEST(c_struct);
56475712
RUN_TEST(cpp_class);
5713+
RUN_TEST(cpp_method_return_type_preserves_pointer_and_qualifier);
56485714

56495715
/* Scripting */
56505716
RUN_TEST(python_function);

‎tests/test_token_reduction.c‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -387,8 +387,12 @@ TEST(snippet_signature_mode) {
387387
ASSERT_NOT_NULL(strstr(resp, "def large_function(arg1, arg2, arg3)"));
388388
/* Should NOT contain full source body */
389389
ASSERT_NULL(strstr(resp, "step_050"));
390-
/* Should indicate total size */
391-
ASSERT_NOT_NULL(strstr(resp, "\"total_lines\""));
390+
/* Signature is a complete requested representation, not clipped source.
391+
* Retain size context without telling callers to repair a non-problem. */
392+
ASSERT_NOT_NULL(strstr(resp, "\"total_lines\":300"));
393+
ASSERT_NULL(strstr(resp, "\"truncated\":true"));
394+
ASSERT_NULL(strstr(resp, "\"source_clipped\":true"));
395+
ASSERT_NULL(strstr(resp, "\"clipped_at_lines\""));
392396

393397
free(resp);
394398
cbm_mcp_server_free(srv);

0 commit comments

Comments
 (0)