Skip to content

Invalid rewriting of multi-decl containing forward-declared struct #644

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mattmccutchen-cci opened this issue Jul 8, 2021 · 0 comments · Fixed by #657
Closed

Invalid rewriting of multi-decl containing forward-declared struct #644

mattmccutchen-cci opened this issue Jul 8, 2021 · 0 comments · Fixed by #657
Labels

Comments

@mattmccutchen-cci
Copy link
Member

I was kicking the tires of 3C's rewriting of multi-decls containing structs a bit in preparation for working on #531, and I discovered that the current code doesn't handle forward declarations of structs properly. The following file:

struct fwd *p;

rewrites to the following syntatically invalid code, with or without -alltypes:

struct }_Ptr<struct fwd> p = ((void *)0);

My understanding of Clang's AST parsing behavior is that the first mention of struct fwd in any context in a translation unit is reported as a RecordDecl whether or not it has a body. If it has a body, it is the definition, otherwise it is a forward declaration; we can use RecordDecl::isCompleteDefinition to distinguish between these cases. Subsequent occurrences of struct fwd; by itself are reported as forward declarations, but subsequent mentions of struct fwd as part of other constructs are not reported as RecordDecls at all.

AFAIK, "forward declarations" of structs as part of other constructs have no significance in C, because the first mention of a struct, wherever it appears, will be treated as a forward declaration. (There is one edge case in which a standalone forward declaration does make a difference, but 3C won't modify those anyway.) So probably what we want to do is have all 3C code that handles struct definitions in multi-decls just ignore RecordDecls for which isCompleteDefinition returns false. I prototyped this change and it seemed to have the desired effect in the example above; I'll probably submit a PR later.

@mattmccutchen-cci mattmccutchen-cci changed the title Invalid writing of multi-decl containing forward-declared struct Invalid rewriting of multi-decl containing forward-declared struct Jul 8, 2021
mattmccutchen-cci added a commit that referenced this issue Jul 21, 2021
declaration as if it were a definition.

Fixes #644.
@mattmccutchen-cci mattmccutchen-cci linked a pull request Jul 29, 2021 that will close this issue
11 tasks
mattmccutchen-cci added a commit that referenced this issue Sep 1, 2021
declaration as if it were a definition.

Fixes #644.
mattmccutchen-cci added a commit that referenced this issue Sep 17, 2021
declaration as if it were a definition.

Fixes #644.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant