Skip to content
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

Multi-decls containing unnamed inline structs may be orphaned even without -alltypes #646

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

Comments

@mattmccutchen-cci
Copy link
Member

mattmccutchen-cci commented Jul 8, 2021

It's a known issue that when -alltypes is on, 3C may rewrite a multi-decl containing an unnamed inline struct definition, which results in code that doesn't compile (#542). However, when -alltypes is off, in accordance with the goal of avoiding compile errors in the output code, 3C tries to generate constraints to prevent anything from happening that would trigger rewriting of the multi-decl:

  1. If a variable in the multi-decl is a pointer or array, InlineStructDetector::processVarDecl constrains it to wild to avoid having to wrap the struct in _Ptr<...>.
  2. A lesser-known issue: If a field of the struct is a pointer and it becomes checked, 3C would need to add initializers to any variables in the multi-decl that don't already have them. To avoid this, if the first variable in the multi-decl lacks an initializer, then InlineStructDetector::processVarDecl constrains any pointer fields of the struct to wild. (It also does this for array fields, which AFAICT is not needed in general, although it is needed for an array of pointers, so maybe we just didn't do the work to make that distinction. Furthermore, these constraints are generated even if the inline struct is named, which probably isn't necessary.)

However, (2) is insufficient to avoid rewriting of the multi-decl. For one thing, it only considers the first variable in the multi-decl; obviously it would need to consider all of them. Example:

void foo() {
  struct { int *y; } s = {}, s2;
}

Currently, without -alltypes, this example crashes 3C in the same way as #468. However, this is easily fixed by extending the workaround in #497 to apply regardless of -alltypes. Then I get the following output:

void foo() {
  struct { _Ptr<int> y; };
struct (anonymous struct at /home/matt/test/struct-denesting/mixed-unnamed.c:2:3) s = {};
struct (anonymous struct at /home/matt/test/struct-denesting/mixed-unnamed.c:2:3) s2 = {};

}

It probably wouldn't be hard to check all variables in the multi-decl. However, a more serious problem remains: if the unnamed struct has a field whose type is a named struct defined elsewhere, and the inner struct has a pointer field that becomes checked, then we need to add initializers to the multi-decl. Example:

void foo() {
  struct inner {
    int *inner_p;
  };
  struct {
    struct inner in;
  } xx;
}

3C output:

void foo() {
  struct inner {
    _Ptr<int> inner_p;
  };
  struct {
    struct inner in;
  };
struct (anonymous struct at /home/matt/test/struct-denesting/nested-initializer.c:5:3) xx = {};

}

If we remain committed to the goal of avoiding compile errors when -alltypes is off, we have the following options that I can think of:

  1. Add a second code path for rewriting a multi-decl containing unnamed inline structs that doesn't split up the multi-decl and only supports adding initializers to variables in the multi-decl, not making any other changes.
  2. When we see a multi-decl containing an unnamed inline struct and a variable with no initializer, scan the struct definition recursively for pointer fields and constrain them all wild. To me, this feels like an amount of 3C code and wildness that is disproportionate to the problem.
  3. Generate names for unnamed structs, as contemplated in 3C loses struct definition as part of a field nested in another struct definition #531. As discussed there, this is potentially an invasive change to the user's code, but we may decide it's justified because unnamed structs are a poor coding practice and we are tired of dealing with 3C bugs related to unnamed structs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant