Skip to content

Remove link_to_library(goto_functions, symbol_table, ...) [blocks: #4056] #4296

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
Merged
merged 1 commit into from
Nov 7, 2020
Merged

Remove link_to_library(goto_functions, symbol_table, ...) [blocks: #4056] #4296

merged 1 commit into from
Nov 7, 2020

Conversation

tautschnig
Copy link
Collaborator

There are no in-tree uses of this function. External users should use
link_to_library(goto_model, ...). This is in preparation of further changes that
will only support the goto_modelt-variant.

  • Each commit message has a non-empty body, explaining why the change was made.
  • n/a Methods or procedures I have added are documented, following the guidelines provided in CODING_STANDARD.md.
  • n/a The feature or user visible behaviour I have added or modified has been documented in the User Guide in doc/cprover-manual/
  • n/a 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).
  • n/a My commit message includes data points confirming performance improvements (if claimed).
  • My PR is restricted to a single feature or bugfix.
  • n/a White-space or formatting changes outside the feature-related changed lines are in commits of their own.

@tautschnig tautschnig changed the title Remove link_to_library(goto_functions, symbol_table, ...) Remove link_to_library(goto_functions, symbol_table, ...) [blocks: #4056] Feb 28, 2019
allredj
allredj reviewed Feb 28, 2019
View reviewed changes
Copy link
Contributor

@allredj allredj left a comment

Choose a reason for hiding this comment

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

✔️
Passed Diffblue compatibility checks (cbmc commit: f20f307).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/102647097

@kroening
Copy link
Member

If this doesn't have urgency, then perhaps deprecate that goto_functions/symbol_tablet variant?

@kroening
Copy link
Member

Or, while at it, all other functions that take the goto_functions/symbol_tablet pair?

@tautschnig tautschnig mentioned this pull request Feb 28, 2019
Merged
3 tasks
@tautschnig tautschnig changed the title Remove link_to_library(goto_functions, symbol_table, ...) [blocks: #4056] Remove link_to_library(goto_functions, symbol_table, ...) [depends-on: #4303, blocks: #4056] Feb 28, 2019
@tautschnig
Copy link
Collaborator Author

I have created #4303 to do the deprecation instead. There is still a bunch of work left on #4056, and until that's resolved this indeed isn't urgent.

tautschnig added a commit that referenced this pull request Feb 28, 2019
@tautschnig
Merge pull request #4303 from tautschnig/deprecate-link_to_library
f61d33d 
Deprecate link_to_library(goto_functions, symbol_table, ...) [blocks: #4296]
@forejtv forejtv changed the title Remove link_to_library(goto_functions, symbol_table, ...) [depends-on: #4303, blocks: #4056] Remove link_to_library(goto_functions, symbol_table, ...) [blocks: #4056] Mar 6, 2019
@tautschnig tautschnig changed the title Remove link_to_library(goto_functions, symbol_table, ...) [blocks: #4056] Remove link_to_library(goto_functions, symbol_table, ...) [blocks: #4056, do not merge before 2019-08-28] Mar 7, 2019
allredj
allredj reviewed Mar 7, 2019
View reviewed changes
Copy link
Contributor

@allredj allredj left a comment

Choose a reason for hiding this comment

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

✔️
Passed Diffblue compatibility checks (cbmc commit: deacf8b).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/103505153

@codecov
Copy link

codecov bot commented Nov 6, 2020 •
edited
Loading

Codecov Report

Merging #4296 (c86cf4a) into develop (b4d939b) will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #4296   +/-   ##
========================================
  Coverage    68.53%   68.53%           
========================================
  Files         1187     1187           
  Lines        98268    98259    -9     
========================================
  Hits         67345    67345           
+ Misses       30923    30914    -9
Flag Coverage Δ
cproversmt2 43.09% <ø> (+<0.01%) ⬆️
regression 65.71% <ø> (+<0.01%) ⬆️
unit 32.26% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/goto-programs/link_to_library.cpp 100.00% <ø> (+45.00%) ⬆️

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 b4d939b...c86cf4a. Read the comment docs.

@tautschnig
Copy link
Collaborator Author

@kroening This is now ready for review.

martin-cs
martin-cs approved these changes Nov 6, 2020
View reviewed changes
Copy link
Collaborator

@martin-cs martin-cs left a comment

Choose a reason for hiding this comment

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

Seems reasonable as it has been deprecated for 21 months or so.

@tautschnig tautschnig changed the title Remove link_to_library(goto_functions, symbol_table, ...) [blocks: #4056, do not merge before 2019-08-28] Remove link_to_library(goto_functions, symbol_table, ...) [blocks: #4056] Nov 6, 2020
kroening
kroening reviewed Nov 7, 2020
View reviewed changes
src/goto-programs/link_to_library.h
class goto_modelt;
class message_handlert;
class symbol_tablet;

DEPRECATED(SINCE(2019, 2, 28, "Use link_to_library(goto_model, ...) instead"))
void link_to_library(
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't that DEPRECATED line go away?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you for catching this!

@tautschnig
Remove link_to_library(goto_functions, symbol_table, ...)
c86cf4a 
There are no in-tree uses of this function. External users should use
link_to_library(goto_model, ...). This is in preparation of further changes that
will only support the goto_modelt-variant.
@kroening kroening merged commit 6d832ec into diffblue:develop Nov 7, 2020
@tautschnig tautschnig deleted the linking-fix-3 branch November 7, 2020 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kroening kroening kroening left review comments

@allredj allredj allredj left review comments

@smowton smowton smowton approved these changes

@martin-cs martin-cs martin-cs approved these changes

@peterschrammel peterschrammel Awaiting requested review from peterschrammel peterschrammel is a code owner

@pkesseli pkesseli Awaiting requested review from pkesseli

Assignees

@tautschnig tautschnig

Labels
blocker cleanup
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@tautschnig @kroening @smowton @martin-cs @allredj @peterschrammel