Skip to content

Commit

Permalink
Stop mkString from producing the extra space in int * x[10].
Browse files Browse the repository at this point in the history
The problem already affected several regression tests for more complex
cases (which expected the extra space), and we didn't seem to notice or
care. But now it is affecting every unchanged variable of that form in a
multi-decl that gets rewritten, and this broke several other regression
tests.
  • Loading branch information
mattmccutchen-cci committed Sep 14, 2021
1 parent 17de215 commit 9dc4968
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 12 deletions.
29 changes: 26 additions & 3 deletions clang/lib/3C/ConstraintVariables.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -858,10 +858,31 @@ PointerVariableConstraint::mkString(Constraints &CS,
if (!ForItype && InferredGenericIndex == -1 && isVoidPtr())
K = Atom::A_Wild;

// In a case like `_Ptr<T> g[1]`, we need to push the ` g` onto EndStrs
// before we can push the `>` and start drilling into T.
//
// Exception: In a case like `int *g[1]`, we don't want an extra space after
// the `*` but we don't yet know whether we have a `*`, so we skip emitting
// the name. We have to also skip the addArrayAnnotations because it would
// cause the array annotations to be emitted before the name.
//
// Exception to the exception: In a case like `void (*fs[2])()`, we still
// want to avoid emitting the name (which would add an extra space after the
// `*`), but we _do_ need to call addArrayAnnotations so the array
// annotations end up with the variable name rather than after the function
// parameter list. The function pointer code knows to emit the name before
// EndStrs.
//
// This passes the current regression tests but feels very ad-hoc.
// REVIEW: Help me redesign this code!
if (PrevArr && ArrSizes.at(TypeIdx).first != O_SizedArray && !EmittedName) {
EmittedName = true;
addArrayAnnotations(ConstArrs, EndStrs);
EndStrs.push_front(" " + UseName);
if (K != Atom::A_Wild || FV != nullptr) {
addArrayAnnotations(ConstArrs, EndStrs);
}
if (K != Atom::A_Wild) {
EmittedName = true;
EndStrs.push_front(" " + UseName);
}
}
PrevArr = ArrSizes.at(TypeIdx).first == O_SizedArray;

Expand Down Expand Up @@ -951,6 +972,8 @@ PointerVariableConstraint::mkString(Constraints &CS,
// the the stack array type.
if (PrevArr && !EmittedName && AllArrays) {
EmittedName = true;
// Note: If the whole type is an array, we can't have a "*", so we don't
// need to worry about an extra space.
EndStrs.push_front(" " + UseName);
}

Expand Down
2 changes: 1 addition & 1 deletion clang/test/3C/compound_literal.c
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ void lists() {

int *d[2] = (int *[2]){&x, (int *)1};
//CHECK_NOALL: int *d[2] = (int *[2]){&x, (int *)1};
//CHECK_ALL: int * d _Checked[2] = (int * _Checked[2]){&x, (int *)1};
//CHECK_ALL: int *d _Checked[2] = (int * _Checked[2]){&x, (int *)1};
int *d0 = d[0];
//CHECK: int *d0 = d[0];

Expand Down
10 changes: 5 additions & 5 deletions clang/test/3C/fptr_array.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@

void (*fs[2])(int *);
void (*f)(int *);
//CHECK_NOALL: void (* fs[2])(_Ptr<int>) = {((void *)0)};
//CHECK_NOALL: void (*fs[2])(_Ptr<int>) = {((void *)0)};
//CHECK_NOALL: void (*f)(_Ptr<int>) = ((void *)0);
//CHECK_ALL: _Ptr<void (_Ptr<int>)> fs _Checked[2] = {((void *)0)};
//CHECK_ALL: _Ptr<void (_Ptr<int>)> f = ((void *)0);

void (*gs[2])(int *);
void g_impl(int *x) { x = 1; }
void (*g)(int *) = g_impl;
//CHECK_NOALL: void (* gs[2])(int * : itype(_Ptr<int>)) = {((void *)0)};
//CHECK_NOALL: void (*gs[2])(int * : itype(_Ptr<int>)) = {((void *)0)};
//CHECK_NOALL: void g_impl(int *x : itype(_Ptr<int>)) { x = 1; }
//CHECK_NOALL: void (*g)(int * : itype(_Ptr<int>)) = g_impl;

Expand All @@ -30,21 +30,21 @@ void (*h)(void *);

int *(*is[2])(void);
int *(*i)(void);
//CHECK_NOALL: _Ptr<int> (* is[2])(void) = {((void *)0)};
//CHECK_NOALL: _Ptr<int> (*is[2])(void) = {((void *)0)};
//CHECK_NOALL: _Ptr<int> (*i)(void) = ((void *)0);
//CHECK_ALL: _Ptr<_Ptr<int> (void)> is _Checked[2] = {((void *)0)};
//CHECK_ALL: _Ptr<_Ptr<int> (void)> i = ((void *)0);

int *(**js[2])(void);
int *(**j)(void);
//CHECK_NOALL: _Ptr<int> (** js[2])(void) = {((void *)0)};
//CHECK_NOALL: _Ptr<int> (**js[2])(void) = {((void *)0)};
//CHECK_NOALL: _Ptr<int> (**j)(void) = ((void *)0);
//CHECK_ALL: _Ptr<_Ptr<_Ptr<int> (void)>> js _Checked[2] = {((void *)0)};
//CHECK_ALL: _Ptr<_Ptr<_Ptr<int> (void)>> j = ((void *)0);

int *(*ks[2][2])(void);
int *(*k)(void);
//CHECK_NOALL: _Ptr<int> (* ks[2][2])(void) = {((void *)0)};
//CHECK_NOALL: _Ptr<int> (*ks[2][2])(void) = {((void *)0)};
//CHECK_NOALL: _Ptr<int> (*k)(void) = ((void *)0);
//CHECK_ALL: _Ptr<_Ptr<int> (void)> ks _Checked[2] _Checked[2] = {((void *)0)};
//CHECK_ALL: _Ptr<_Ptr<int> (void)> k = ((void *)0);
Expand Down
2 changes: 1 addition & 1 deletion clang/test/3C/macro_end_of_decl.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ int *f EQ 0;

int(*g0) ARGS, g1 SIZE, *g2 EQ 0;
//CHECK: _Ptr<int (void)> g0 = ((void *)0);
//CHECK_NOALL: int g1[1];
//CHECK_NOALL: int g1 SIZE;
//CHECK_ALL: int g1 _Checked SIZE;
//CHECK: _Ptr<int> g2 = 0;

Expand Down
4 changes: 2 additions & 2 deletions clang/test/3C/ptr_array.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ void test1(int *a) {

int *b[1] = {a};
//CHECK_NOALL: int *b[1] = {a};
//CHECK_ALL: int * b _Checked[1] = {a};
//CHECK_ALL: int *b _Checked[1] = {a};
}

/* Example from from the issue */
Expand All @@ -40,7 +40,7 @@ int *foo() {
int z = 3;
int *ptrs[4] = {&x, &y, &z, (int *)5};
//CHECK_NOALL: int *ptrs[4] = {&x, &y, &z, (int *)5};
//CHECK_ALL: int * ptrs _Checked[4] = {&x, &y, &z, (int *)5};
//CHECK_ALL: int *ptrs _Checked[4] = {&x, &y, &z, (int *)5};
int *ret;
//CHECK: int *ret;
for (int i = 0; i < 4; i++) {
Expand Down

0 comments on commit 9dc4968

Please sign in to comment.