Skip to content
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

Fix bug in compiler, and add tests #1789

Merged
merged 3 commits into from
Oct 26, 2017

Conversation

ChrisJefferson
Copy link
Contributor

This patch fixes #1781 (which was easy, as there was a fix already in that PR!), but more usefully adds a simple testing infrastructure for the GAP->C compiler, and adds some simple tests (including for the fixed bug).

Obviously, the number of tests here is very small, but it provides a basic sanity check and a place for people to add more tests if they want.

@ChrisJefferson ChrisJefferson added the release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes label Oct 19, 2017
@codecov
Copy link

codecov bot commented Oct 19, 2017

Codecov Report

Merging #1789 into master will increase coverage by 0.04%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1789      +/-   ##
==========================================
+ Coverage   62.93%   62.97%   +0.04%     
==========================================
  Files         969      969              
  Lines      293986   293986              
  Branches    12986    12962      -24     
==========================================
+ Hits       185013   185140     +127     
+ Misses     106162   106022     -140     
- Partials     2811     2824      +13
Impacted Files Coverage Δ
src/intrprtr.c 71.34% <ø> (ø) ⬆️
lib/queue.g 66.4% <0%> (-3.2%) ⬇️
src/objset.c 82.36% <0%> (-0.24%) ⬇️
src/read.c 83.9% <0%> (+0.07%) ⬆️
src/lists.c 56.71% <0%> (+0.11%) ⬆️
src/funcs.c 72.1% <0%> (+0.14%) ⬆️
hpcgap/lib/hpc/stdtasks.g 38.87% <0%> (+0.25%) ⬆️
src/permutat.c 76.4% <0%> (+0.9%) ⬆️
src/exprs.c 79.59% <0%> (+1.68%) ⬆️
src/compiler.c 65.28% <0%> (+3.35%) ⬆️
... and 1 more

@fingolfin
Copy link
Member

Instead of renaming .out to .output, you could also just adjust the rule in .gitignore... :-) E.g. replacing it by this:

/doc/*/main.out
/log/*.out

@ChrisJefferson
Copy link
Contributor Author

Touching the .gitignore seems to lead to bikesheding, so I thought it was easier to just rename the files intead...

@fingolfin
Copy link
Member

@ChrisJefferson not sure why you think changing entries in .gitignore leads to bike shedding? Those entries all are there to hide build artifacts. In some cases, they are pretty generic, for simplicity; but we can always replace a relatively "general" entry with more specialized ones, as is needed here. I see no room for bikeshedding whatsoever.

And even if, then we should actively combat that, instead of avoiding to make certain changes.

That said, if you prefer .output over .out, that's of course also OK, but I will now change the .gitignore rule for .out anyway...

@fingolfin
Copy link
Member

Specifically, I already change .gitignore in PR #1780

@fingolfin
Copy link
Member

@ChrisJefferson I am sorry but this now conflicts :/. Would you terribly mind updating it, then we can merge it ASAP.

@markuspf markuspf merged commit abd6584 into gap-system:master Oct 26, 2017
@ChrisJefferson ChrisJefferson deleted the info-test branch November 6, 2017 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compiler emits call to InfoDecision but symbol is defined static in intrprtr.c
4 participants