Repository navigation
Comment lines placed after the last 'struct' field (in schema file) appear outside generated C language 'struct' definition #336
Description
Recently I've noticed that comments started with /// and placed in the end of struct definition (right after the last field of the struct and before closing } brace) appear outside C language struct definition after the schema file's been processed by flatcc. Please check following code snippets with reference example mymonster (file src/monster.fbs):
...
struct Vec3 {
/// Comment A
x:float;
/// Comment B
y:float;
/// Comment C
z:float;
/// Comment D
/// Comment D1
}
...
So, when flatcc processes mentioned schema file the following generated header file looses Comment D as well as Comment D1 in the struct definition. Lost comments can be found far outside the struct definition unfortunately:
...
struct MyGame_Sample_Vec3 {
/** Comment A */
alignas(4) float x;
/** Comment B */
alignas(4) float y;
/** Comment C */
alignas(4) float z;
};
static_assert(sizeof(MyGame_Sample_Vec3_t) == 12, "struct size mismatch");
...
...
/** Comment D
* Comment D1 */
struct MyGame_Sample_Monster_table { uint8_t unused__; };
...
Please help to fix this issue if it is not expected behavior.
Activity
mikkelfj commentedon Apr 11, 2026
Sorry for not getting back on this. This is not ideal. It likely has impact on comments stored in the binary schema (bfbs) as well, but I'm not sure where to attach such a doc comment that does not follow any fields. There might not be an obvious place to track this info internally, so maybe a fix could be to detect and ignore such doc comments.
This is fairly low priority, but a PR would be considered.
khushi-infinity commentedon Oct 9, 2026
Hi @mikkelfj , I would like to fix this bug, can you assign it to me or may I open a PR?
mikkelfj commentedon Oct 9, 2026
@khushi-infinity yes you can open a PR and you did open a PR.
As for the PR, I have not checked it for correctness, but as per my earlier comment, there is also the question of how to handle this in binary schema files. The main issue is misplacing comments rather than placing them correctly or removing them entirely in generated source. I think your PR attempts to place them correctly, which is fine.
But we need to establish what we actually want to achieve:
And wrt. the PR #399 specifically - I don't want any actual program logic to reside in build files (notably considering there is a parallel Meson build PR) so the test should be moved to actual test code, and likely be built through an existing test case, though that depends. Also, if bfbs (binary schema) is not affected, that may suffice, but if we also affect those, we need a way to test the binary output either from memory, or from a file, or decide it is not worth bothering testing.
khushi-infinity commentedon Oct 9, 2026
Thanks @mikkelfj , this helped clarify the scope. I checked FlatCC’s BFBS exporter: although the reflection schema has documentation fields, FlatCC does not currently populate them. I’ve left BFBS unchanged and kept this fix limited to generated C; adding documentation to BFBS would be a separate feature and would need its own test.
I also surveyed flatc. Its parser associates doc comments with the following declaration or member; a comment left before a closing brace is not reassigned to the next declaration. In FlatCC, the trailing table comment is now discarded rather than misplaced, and the union parser already clears unattached comments. The regression test covers the struct placement, the following declaration’s comment, and the table/union behavior.
I moved the assertions into the existing cgen_test executable and removed the CMake-script test logic. The existing generated-C compile target still works. The focused cgen_test and reflection_test pass.
mikkelfj commentedon Oct 9, 2026
OK, I'm fine with this direction. BFBS probably didn't have comment fields originally. It would be nice to support, but is not required here.
I only see you moved the test, but anything else in the PR doesn't need seem to reflect your stated intent. There is a trailing comment field introduced.
Can you provide more detail on the PR comment:
I don't understand what could be wrong with that line, other than potentially overwriting a stack allocated msgbuf[100], but it isn't.
Also, the top comment of the cgen test, with a few typos, state that content it is written to stdout, which is no longer the case.
khushi-infinity commentedon Oct 10, 2026
Thanks. trailing_doc stores documentation comments left pending at the end of a struct. The parser captures them for structs and clears the pending comment state, and the C reader emits them before the struct’s closing brace. This keeps them from appearing on the following declaration. The change is limited to generated C; BFBS remains unchanged. The regression test also checks that trailing table and union comments aren’t emitted.
My original sanitizer note was imprecise. The initial build stopped before running tests because Apple Clang marked the existing sprintf call as deprecated and the project’s -Werror setting promoted the warning to an error. I reran cgen_test under ASan/UBSan with -Werror temporarily disabled; it passed with no memory-error or undefined-behavior reports. LeakSanitizer isn’t available on this macOS host, so leaks weren’t checked. I’ve restored the CMake settings and updated the test’s opening comment to describe its current output flow.
mikkelfj commentedon Oct 10, 2026
I'm going to reject this. The suggested approach is not wrong, butg PR is not doing that and I don't want to babysit an AI through an intermediary. Why keep trailing_doc if it is not used, and if it is used, why only on structs, and my question on sprintf was not answered. The error message from ubsan was asked for. The likely answer: use snprintf with the buffer size. But just generally, it faster to do myself with or without AI.