Skip to content

Register the json_symtab languauge in symtab2gb #6175

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

Merged

Conversation

martin-cs
Copy link
Collaborator

To quote langapi/mode.cpp:

"Note: registering a language is required for using the functions
in language_util.h"

symtab2gb previously failed to do this and so fails in invariant
under some obscure and unclear circumstances.

  • Each commit message has a non-empty body, explaining why the change was made.
  • Methods or procedures I have added are documented, following the guidelines provided in CODING_STANDARD.md.
  • The feature or user visible behaviour I have added or modified has been documented in the User Guide in doc/cprover-manual/
  • Regression or unit tests are included, or existing tests cover the modified code (in this case I have detailed which ones those are in the commit message).
  • My commit message includes data points confirming performance improvements (if claimed).
  • My PR is restricted to a single feature or bugfix.
  • White-space or formatting changes outside the feature-related changed lines are in commits of their own.

@martin-cs
Copy link
Collaborator Author

Unfortunately I don't have access to the test case that triggers this but as it is a violation of our own documentation, I figure it is clearly enough a bug.

@@ -111,6 +111,7 @@ int symtab2gb_parse_optionst::doit()
{
gb_filename = cmdline.get_value(SYMTAB2GB_OUT_FILE_OPT);
}
register_language(new_json_symtab_language);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Failing build note: symtab2gb_parse_options.cpp:114:3: error: ‘register_language’ was not declared in this scope

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooops. Should be fixed now.

@martin-cs martin-cs force-pushed the fix/symtab2gb-register-language branch from 2e0be7b to 0a4a18f Compare June 12, 2021 16:07
@codecov
Copy link

codecov bot commented Jun 12, 2021

Codecov Report

Merging #6175 (3b5b849) into develop (6b50008) will decrease coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #6175      +/-   ##
===========================================
- Coverage    75.30%   75.26%   -0.04%     
===========================================
  Files         1450     1450              
  Lines       158480   158481       +1     
===========================================
- Hits        119348   119287      -61     
- Misses       39132    39194      +62     
Impacted Files Coverage Δ
src/symtab2gb/symtab2gb_parse_options.cpp 61.29% <100.00%> (+0.63%) ⬆️
src/ansi-c/gcc_types.cpp 41.86% <0.00%> (-46.52%) ⬇️
src/ansi-c/scanner.l 40.71% <0.00%> (-1.66%) ⬇️
src/ansi-c/parser.y 56.54% <0.00%> (-1.58%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4dc2dd5...3b5b849. Read the comment docs.

To quote langapi/mode.cpp:

"Note: registering a language is required for using the functions
in language_util.h"

symtab2gb previously failed to do this and so fails in invariant
under some obscure and unclear circumstances.
@martin-cs martin-cs force-pushed the fix/symtab2gb-register-language branch from 0a4a18f to 3b5b849 Compare June 13, 2021 12:49
@martin-cs
Copy link
Collaborator Author

I am not sure how the over-all coverage is decreasing if the patch has 100% coverage.

@martin-cs martin-cs merged commit fcf989a into diffblue:develop Jun 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants