Skip to content

Linking: replace conflicting pointer types when one declaration is extern #2024

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
Oct 3, 2018
Merged

Linking: replace conflicting pointer types when one declaration is extern #2024

merged 1 commit into from
Oct 3, 2018

Conversation

@tautschnig
Copy link
Collaborator

@tautschnig tautschnig commented Apr 9, 2018

No description provided.

smowton
smowton approved these changes Apr 10, 2018
View reviewed changes
Copy link
Contributor

@smowton smowton left a comment

Choose a reason for hiding this comment

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

lgtm

martin-cs
martin-cs approved these changes May 17, 2018
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.

The parts of it that touch my world look good.

kroening
kroening reviewed Jun 1, 2018
View reviewed changes
src/ansi-c/c_typecheck_base.cpp
warning().source_location = symbol.value.find_source_location();
warning() << "`extern' symbol should not have an initializer" << eom;
}
}
Copy link
Collaborator

@kroening kroening Jun 1, 2018

Choose a reason for hiding this comment

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

Turn into error when in gcc or clang mode?

kroening
kroening reviewed Jun 1, 2018
View reviewed changes
src/util/replace_symbol.cpp
const bool replace_with_const) const
const bool replace_with_const,
const bool ignore_type_change) const
{
Copy link
Collaborator

@kroening kroening Jun 1, 2018

Choose a reason for hiding this comment

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

I think this is getting to bizarre, and to linker specific. I'd make that a separate piece of code, just for the linker.

Copy link
Collaborator Author

@tautschnig tautschnig Aug 11, 2018

Choose a reason for hiding this comment

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

Work in progress, #2720 and #2721 are the first PRs to enable this. More to come.

@tautschnig tautschnig requested a review from pkesseli as a code owner August 11, 2018 11:33
@tautschnig tautschnig requested a review from thk123 as a code owner August 13, 2018 13:19
@tautschnig
Copy link
Collaborator Author

tautschnig commented Aug 13, 2018

This is now implemented on top of #2723 with the suggested, linker-specific implementation.

@tautschnig tautschnig force-pushed the extern-linking branch 3 times, most recently from 6b111a4 to 6a20f61 Compare August 18, 2018 13:30
allredj
allredj reviewed Sep 6, 2018
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.

This PR failed Diffblue compatibility checks (cbmc commit: 6e921c2).
Status will be re-evaluated on next push.
Please contact @peterschrammel, @thk123, or @allredj for support.

Common spurious failures:

  • the cbmc commit has disappeared in the mean time (e.g. in a force-push)
  • the author is not in the list of contributors (e.g. first-time contributors).

allredj
allredj reviewed Sep 6, 2018
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.

This PR failed Diffblue compatibility checks (cbmc commit: 6e921c2).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/83986092
Status will be re-evaluated on next push.
Please contact @peterschrammel, @thk123, or @allredj for support.

Common spurious failures:

  • the cbmc commit has disappeared in the mean time (e.g. in a force-push)
  • the author is not in the list of contributors (e.g. first-time contributors).

allredj
allredj reviewed Sep 6, 2018
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: c720555).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/84011100

allredj
allredj reviewed Sep 7, 2018
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: e8b03a7).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/84094002

@peterschrammel peterschrammel removed their assignment Sep 24, 2018
allredj
allredj reviewed Sep 27, 2018
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: d7dae29).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/86154692

@tautschnig
Copy link
Collaborator Author

tautschnig commented Oct 3, 2018

@kroening ping?

allredj
allredj reviewed Oct 3, 2018
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: 2f60e13).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/86766275

allredj
allredj reviewed Oct 3, 2018
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: 80ff4e5).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/86776976

@tautschnig tautschnig merged commit a1e86d3 into diffblue:develop Oct 3, 2018
@tautschnig tautschnig deleted the extern-linking branch October 3, 2018 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kroening kroening kroening approved these changes

@peterschrammel peterschrammel peterschrammel approved these changes

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

@chrisr-diffblue chrisr-diffblue Awaiting requested review from chrisr-diffblue

@pkesseli pkesseli Awaiting requested review from pkesseli

@thk123 thk123 Awaiting requested review from thk123

+2 more reviewers

@allredj allredj allredj left review comments

@smowton smowton smowton approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@kroening kroening

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@tautschnig @smowton @kroening @peterschrammel @martin-cs @allredj @chrisr-diffblue