Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
struct S {
int a;
int b;
int c;
unsigned long *d;

union {
struct {
const char *e;
int f;
S *g;
const char *h;
int i;
bool j;
bool k;
const char *l;
char **m;
} n;

struct {
bool o;
bool p;
} q;
} r;
};

int too_many_constants_init(S *s);

char *too_many_constants(const char *h, bool k, int i) {
const char *e = "";
char l[64] = "";
char *m;

S s[] = {
{.a = 0, .c = 0, .d = nullptr, .r = {.n = {.e = e, .f = 1, .g = nullptr, .h = h, .i = i, .j = false, .k = k, .l = l, .m = &m}}},
{.a = 0, .c = 0, .d = nullptr, .r = {.q = {.o = true, .p = true}}}
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be nice with a small comment explaining what the interesting thing being tested here is. Something with too many constants? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing anymore, after the extractor fix I merged yesterday. This test is just here to make sure we do not regress.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then is is testing something interesting right? That we don't regress for some specific case. I'm suggesting it would be nice to write out what the special thing about this case here is. Is it the number of fields, the nesting, etc? It's not clear to me :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to politely refuse, because this is related to internal extractor details. If you want the full story, please follow the link to the internal PR that references this PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be clear, I'm not suggesting writing anything about the extractor, but that we highlight the interesting/key aspect of the test. I've suggested a comment below.


too_many_constants_init(s);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
too_many_constants_init(s);
too_many_constants_init(s); // Initializer being passed a literal with many constants
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now you need to define what a constant is, because the aggregate here clearly isn't. Can we not go down this rabbit hole, please.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see the problem, but ok.


return m; // GOOD - initialized by too_many_constants_init
}