melt-umn
Parameterized gcc Attributes are pretty-printed with erroneous parentheses
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):
simple_cat.c:838:95: error: expected 'introduced', 'deprecated', or 'obsoleted'
signed int posix_memalign(void * * , size_t , size_t ) __attribute__((availability(macosx, ((introduced) = 10.6))));
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 cause looks to be
https://github.com/melt-umn/ableC/blob/develop/edu.umn.cs.melt.ableC/abstractsyntax/Expr.sv#L46
abstract production declRefExpr
top::Expr ::= id::Name
{
...
top.pp = parens( id.pp );
...
}
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 declRefExpr's id's pp instead of its own, which should also fix the case where an attribute requires an identifier (rather than an expression). Literals don't get parenthesized either, so they should be fine as well.
Lastly, we could make declRefExpr's pp always be id.pp without parentheses -- I tried that, but I got every single positive test failing, so I assume something's wrong in my setup. (Or my running of the tests; it's testing/runTests, right?)
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.
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" [email protected] 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 https://github.com/melt-umn/ableC/issues/47#issuecomment-305325210, 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 [email protected] 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" [email protected] 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 https://github.com/melt-umn/ableC/issues/47#issuecomment-305325210, 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 https://github.com/melt-umn/ableC/issues/47#issuecomment-305370480, or mute the thread https://github.com/notifications/unsubscribe-auth/ADOhdwkj5pp-CJOIE916y1b_e4uCe2Eiks5r_h_GgaJpZM4NsCF0 .
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" [email protected] 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 https://github.com/melt-umn/ableC/issues/47#issuecomment-305576424, 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 [email protected] 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" [email protected] 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 https://github.com/melt-umn/ableC/issues/47#issuecomment-305576424, 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 https://github.com/melt-umn/ableC/issues/47#issuecomment-305578077, 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" [email protected] 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 [email protected] 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" [email protected] 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 https://github.com/melt-umn/ableC/issues/47#issuecomment-305576424, 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 https://github.com/melt-umn/ableC/issues/47#issuecomment-305578077, 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 https://github.com/melt-umn/ableC/issues/47#issuecomment-305580757, or mute the thread https://github.com/notifications/unsubscribe-auth/AIE1ihdRkWqhEy4GEF6EDaboCkuIKiHQks5r_wPXgaJpZM4NsCF0 .