Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Adapting jsiek's executable semantics tooling for commit. #237
Adapting jsiek's executable semantics tooling for commit. #237
Changes from all commits
fedb730
205c22f
7df725b
bdd7fc9
9edcadf
97b06e2
27a94ee
f6c8709
bdb8e85
b033412
d5f6f1c
8203a0c
3f48c77
96f85d1
9200586
2e76292
89489fc
fec667f
323d90b
23cd5bc
b5000d0
0a560a4
c524e33
816ebf9
639319b
2754d53
10d9f81
afa4a94
18f7d8a
bdbb3eb
b181123
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without this change, uses of the "register" keyword in the bison output go from being warnings to being errors on MacOS.
With this change, and brew's bison first in the path, I can report success on MacOS!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per discussion on Discord, I'm wary of this fix because it feels like a broad stroke. I think it'd probably be better to investigate options further, but I'd rather not block commit on this as the current state works for Linux.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not take the broad stroke now to unblock Mac users (including the original author of the code) and open an issue to find a better fix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To validate, I successfully built on mac with no changes. I do get a
register
-related build error if I use an older version of flex, the one included by xcode.As I previously noted, a recent version of flex is required, just as with bison. Please ensure you're using a recent version.
I think we need to be really cautious about changing how builds work; it can become more difficult to fix later. While I think no change should be needed here, in the future, please provide build errors and rationale for changing code. Sometimes, by thinking these issues through, we can get to the root of the issue and arrive at a different solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing the diligent work. It didn't occur to me that the old flex was the cause, but that explains everything.