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.

Sign up for GitHub

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

Jump to bottom

Avoid diagnostic code crashing when external front-ends use "C" mode #6233

Merged
merged 1 commit into from
Jul 27, 2021
Merged

Avoid diagnostic code crashing when external front-ends use "C" mode #6233

merged 1 commit into from
Jul 27, 2021

Conversation

martin-cs
Copy link
Collaborator

This is a work-around for #6223 until #6219 gets adequately resolved. I am very open to other solutions because this is a hack.

@martin-cs martin-cs force-pushed the workaround/symtab2gb-needs-C-mode branch from 27236aa to 1164e6c Compare July 16, 2021 16:45
@martin-cs martin-cs requested a review from a team as a code owner July 16, 2021 16:45
@thomasspriggs
Copy link
Collaborator

Could we get a regression test to show the issue this fixes?

@codecov
Copy link

codecov bot commented Jul 16, 2021 •
edited
Loading

Codecov Report

Merging #6233 (7f928db) into develop (ccfbaee) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #6233   +/-   ##
========================================
  Coverage    76.18%   76.18%           
========================================
  Files         1484     1484           
  Lines       162092   162093    +1     
========================================
+ Hits        123495   123496    +1     
  Misses       38597    38597
Impacted Files Coverage Δ
src/symtab2gb/symtab2gb_parse_options.cpp 61.90% <100.00%> (+0.61%) ⬆️

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 9e3edf7...7f928db. Read the comment docs.

@martin-cs
Copy link
Collaborator Author

@thomasspriggs not easily. At the moment the only way I have to trigger #6223 involves a bug in gnat2goto which I am in the process of fixing. I will have a look at whether it may be possible to trigger this with gnat2goto working correctly.

@TGWDB
Copy link
Contributor

TGWDB commented Jul 19, 2021

Happy to approve as a work around.

@martin-cs
Copy link
Collaborator Author

Thanks @TGWDB . As this is a work-around I kind-of don't want to merge it and I hope that @tautschnig @peterschrammel @chrisr-diffblue or @kroening could comment on #6223 ( or #6219 ) to help pick a non-hack direction for this.

@chrisr-diffblue
Copy link
Contributor

Yuck :-) I totally get why its needed, and in the absence of the broader changes, agree its probably the 'easiest' solution. But also totally agree that it'd be nice not to have to merge this. Comments left on #6223 and #6219 as well.

@martin-cs
Copy link
Collaborator Author

@chrisr-diffblue thanks for your thoughts. I agree on reasons not to do it. I will ask on #6219 about a schedule and if it seems likely there will be changes soon then I will hold off merging this.

@martin-cs martin-cs mentioned this pull request Jul 20, 2021
Open
@martin-cs martin-cs force-pushed the workaround/symtab2gb-needs-C-mode branch 2 times, most recently from ff44c36 to 5f2f25a Compare July 22, 2021 09:42
Avoid diagnostic code crashing when external front-ends use "C" mode
7f928db 
At the time of writing both the rust and the Ada external front-ends
generate .json_symtab files that use the "C" mode.  If the ansi-c
front-end is not loaded then these will crash when reporting some linking
errors.
@martin-cs martin-cs force-pushed the workaround/symtab2gb-needs-C-mode branch from 5f2f25a to 7f928db Compare July 27, 2021 09:56
@martin-cs
Copy link
Collaborator Author

@thomasspriggs I have tried to create non-buggy test cases for this but have not yet succeeded.

@martin-cs
Copy link
Collaborator Author

In light of the discussion of #6219 I am going to merge this with it's removal waiting on a proper fix to #6219 .

@martin-cs martin-cs merged commit 48a41b8 into diffblue:develop Jul 27, 2021
@martin-cs martin-cs mentioned this pull request Jul 27, 2021
Closed
This was referenced Sep 30, 2021
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TGWDB TGWDB TGWDB approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@martin-cs @thomasspriggs @TGWDB @chrisr-diffblue