-
Notifications
You must be signed in to change notification settings - Fork 6
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
Parameterized gcc Attributes are pretty-printed with erroneous parentheses #47
Comments
The cause looks to be https://github.com/melt-umn/ableC/blob/develop/edu.umn.cs.melt.ableC/abstractsyntax/Expr.sv#L46
We parenthesize all expressions except literals (as far as I can tell from quickly skimming) to avoid implementing precedence, so we could implement that to fix this. Alternatively, Patrick is writing a helper that maps over the ExprList and returns Lastly, we could make |
We tested on a mac if |
I don't know why we wrap declRefExpr in parens- @tedinski?
The testing directory in ableC is for interference testing, which has been
broken for a while. To actually check if this breaks anything, push this in
a new feature branch and Jenkins will let you know if there are problems.
Using the correct precedence for pp would be nice to have, but this is
probably lower on out list of priorities at the moment.
On May 31, 2017 4:37 PM, "Patrick Stephen" <notifications@github.com> wrote:
We tested on a mac if availability(macosx, (introduced = 10.6)) would run,
and it did not, meaning that just removing parens from declRefExpr wouldn't
be a fix, unfortunately.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#47 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AIE1ivpWOU6P5aa1UPHqGExk9BLPYH1Xks5r_d2zgaJpZM4NsCF0>
.
|
I'm not sure if ableC has ever worked on my Mac. It would be nice if it
did, but this is not, in my opinion, a high priority.
I do plan on buying a Mac for my office that will be set up so that others
can login to (or is it "log into"?) to let us eventually fix this.
On Wed, May 31, 2017 at 9:19 PM, Lucas Kramer <notifications@github.com>
wrote:
… I don't know why we wrap declRefExpr in parens- @tedinski?
The testing directory in ableC is for interference testing, which has been
broken for a while. To actually check if this breaks anything, push this in
a new feature branch and Jenkins will let you know if there are problems.
Using the correct precedence for pp would be nice to have, but this is
probably lower on out list of priorities at the moment.
On May 31, 2017 4:37 PM, "Patrick Stephen" ***@***.***>
wrote:
We tested on a mac if availability(macosx, (introduced = 10.6)) would run,
and it did not, meaning that just removing parens from declRefExpr wouldn't
be a fix, unfortunately.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#47 (comment)>, or
mute
the thread
<https://github.com/notifications/unsubscribe-auth/
AIE1ivpWOU6P5aa1UPHqGExk9BLPYH1Xks5r_d2zgaJpZM4NsCF0>
.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#47 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ADOhdwkj5pp-CJOIE916y1b_e4uCe2Eiks5r_h_GgaJpZM4NsCF0>
.
|
I don't really know, sorry. |
I can't find an attribute that isn't obscure-platform specific outside of clang's attributes that ableC currently fails to parse, so its fair to call this a mac issue and wait on it. |
If removing the parentheses from declRefExpr doesn't seems to break
anything, I would say go ahead and do that.
…On Jun 1, 2017 1:17 PM, "Patrick Stephen" ***@***.***> wrote:
I can't find an attribute that isn't obscure-platform specific outside of
clang's attributes that ableC currently fails to parse, so its fair to call
this a mac issue and wait on it.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#47 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AIE1itN806i19oazH0wwWoiX51v_BQm0ks5r_wA_gaJpZM4NsCF0>
.
|
It doesn't actually fix this issue though -- there's still extra parens
around the assignment.
…On Thu, Jun 1, 2017, 13:23 Lucas Kramer ***@***.***> wrote:
If removing the parentheses from declRefExpr doesn't seems to break
anything, I would say go ahead and do that.
On Jun 1, 2017 1:17 PM, "Patrick Stephen" ***@***.***>
wrote:
> I can't find an attribute that isn't obscure-platform specific outside of
> clang's attributes that ableC currently fails to parse, so its fair to
call
> this a mac issue and wait on it.
>
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub
> <#47 (comment)>,
or mute
> the thread
> <
https://github.com/notifications/unsubscribe-auth/AIE1itN806i19oazH0wwWoiX51v_BQm0ks5r_wA_gaJpZM4NsCF0
>
> .
>
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#47 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AEAJtbyFUlBnA-ZUOJbWvEov6vAsd-Pbks5r_wGogaJpZM4NsCF0>
.
|
Yeah, handling this specially might be the best option... Currently the
halide extension is using some hacks to get around this issue when
generating omp pragmas.
…On Jun 1, 2017 1:32 PM, "Nathaniel Ringo" ***@***.***> wrote:
It doesn't actually fix this issue though -- there's still extra parens
around the assignment.
On Thu, Jun 1, 2017, 13:23 Lucas Kramer ***@***.***> wrote:
> If removing the parentheses from declRefExpr doesn't seems to break
> anything, I would say go ahead and do that.
>
> On Jun 1, 2017 1:17 PM, "Patrick Stephen" ***@***.***>
> wrote:
>
> > I can't find an attribute that isn't obscure-platform specific outside
of
> > clang's attributes that ableC currently fails to parse, so its fair to
> call
> > this a mac issue and wait on it.
> >
> > —
> > You are receiving this because you commented.
> > Reply to this email directly, view it on GitHub
> > <#47 (comment)>,
> or mute
> > the thread
> > <
> https://github.com/notifications/unsubscribe-auth/
AIE1itN806i19oazH0wwWoiX51v_BQm0ks5r_wA_gaJpZM4NsCF0
> >
> > .
> >
>
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub
> <#47 (comment)>,
or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AEAJtbyFUlBnA-
ZUOJbWvEov6vAsd-Pbks5r_wGogaJpZM4NsCF0>
> .
>
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#47 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AIE1ihdRkWqhEy4GEF6EDaboCkuIKiHQks5r_wPXgaJpZM4NsCF0>
.
|
An attribute like
__attribute__(name(arg1, arg2))
is currently being written by ableC as__attribute__((name(arg1,(arg2))))
, but this is invalid syntax.The specific example that brought this up was
posix_memalign
on osx in stdlib.h. Including this will raise this error (as in the matlab extension):Removing the extra parentheses (after availability) will let gcc run the file.
Ideally we'd catch something like this in the future by adding in some tests that run on or simulate an osx environment.
This TODO: mentioned in VariableAttributes.sv looks like it could solve this bug.
The text was updated successfully, but these errors were encountered: