Remove unnecessary includes #7235

tautschnig · 2022-10-12T20:42:27Z

Manual removal of includes based on include-what-you-use's output, filtered for includes that should be removed. The goal is to avoid unnecessary build dependencies, reducing the amount of code that needs to be rebuilt during incremental builds.

This is a second iteration of this cleanup; the first round took place in 450845d.

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/
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.

codecov · 2022-10-12T22:14:17Z

Codecov Report

Base: 78.34% // Head: 78.03% // Decreases project coverage by -0.30% ⚠️

Coverage data is based on head (cb91b12) compared to base (a53fa0f).
Patch coverage: 92.13% of modified lines in pull request are covered.

❗ Current head cb91b12 differs from pull request most recent head a8514e0. Consider uploading reports for the commit a8514e0 to get more accurate results

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #7235      +/-   ##
===========================================
- Coverage    78.34%   78.03%   -0.31%     
===========================================
  Files         1644     1625      -19     
  Lines       190313   187422    -2891     
===========================================
- Hits        149097   146252    -2845     
+ Misses       41216    41170      -46

Impacted Files	Coverage Δ
jbmc/src/janalyzer/janalyzer_parse_options.cpp	`48.58% <ø> (ø)`
.../src/java_bytecode/character_refine_preprocess.cpp	`54.27% <ø> (ø)`
...mc/src/java_bytecode/character_refine_preprocess.h	`100.00% <ø> (ø)`
jbmc/src/java_bytecode/ci_lazy_methods.h	`100.00% <ø> (ø)`
jbmc/src/java_bytecode/ci_lazy_methods_needed.cpp	`98.41% <ø> (ø)`
jbmc/src/java_bytecode/ci_lazy_methods_needed.h	`100.00% <ø> (ø)`
jbmc/src/java_bytecode/code_with_references.cpp	`84.61% <ø> (ø)`
jbmc/src/java_bytecode/java_bmc_util.cpp	`100.00% <ø> (ø)`
...mc/src/java_bytecode/java_bytecode_convert_class.h	`0.00% <ø> (ø)`
...src/java_bytecode/java_bytecode_convert_method.cpp	`97.69% <ø> (ø)`
... and 505 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

feliperodri

Thanks for this clean up!

tautschnig · 2022-10-13T20:20:53Z

@chris-ryder Could you please take a look at this one?

chris-ryder

Looks like a painful, but very useful cleanup!

thomasspriggs · 2022-10-27T10:15:22Z

src/solvers/smt2_incremental/smt_response_validation.h

 #include <util/nodiscard.h>

-#include <solvers/smt2_incremental/ast/smt_responses.h>


response_or_errort handles the type it is specialised with by-value like optional does. response_or_errort is specialised with smt_responset for the return value of validate_smt_response. Therefore I would say that it is incorrect to remove this include as it is used.

I have now added // IWYU pragma: keep instead.

thomasspriggs · 2022-10-27T10:20:53Z

src/solvers/smt2_incremental/smt_to_smt2_string.cpp

@@ -13,7 +13,6 @@

 #include <functional>
 #include <iostream>
-#include <sstream>


std::stringstream is used in the smt_to_smt2_string function (all overloads) and is defined in the <sstream> header. Therefore this include should not be removed, as it is used. I assume this file still compiles without the include due to transitive includes of some type. My understanding is that some of these transitive includes can be in implementations of the STL but not required by the standard.

I have now added // IWYU pragma: keep instead.

thomasspriggs · 2022-10-27T10:28:22Z

src/solvers/smt2_incremental/type_size_mapping.h


-#include <solvers/smt2_incremental/ast/smt_terms.h>


smt_termtis defined in smt_terms.h and is used in the definition of type_size_mapt in this file. Therefore this include should be kept.

I have now added // IWYU pragma: keep instead.

thomasspriggs · 2022-10-27T10:41:05Z

unit/solvers/smt2_incremental/smt_to_smt2_string.cpp


 #include <solvers/smt2_incremental/ast/smt_commands.h>
-#include <solvers/smt2_incremental/ast/smt_logics.h>


The logics defined in this header are used as part of the "Test smt_set_logic_commandt to string conversion" test in this file. Removing the include still compiles as it depends on a transitive include via smt_commends.h.

I have now added // IWYU pragma: keep instead.

src/goto-symex/symex_main.cpp

martin-cs

All areas I am maintainer for look good.

There was a spurious include of the implementation file rather than the header.

The empty namespace will lack symbol information that may be needed, and is just unnecessary anyway.

6b88028 removed the existing test (for good reasons) but didn't replace it with any other.

Whitespace cleanup only so as not to break syntax highlighting.

This enables use of symbol_table_baset instead of symbol_tablet in many places.

We almost never create a fresh symbol table, and any time we do so we should really make sure this is the intended behaviour.

This is to make sure that we can run include-what-you-use without being told to remove includes when doing so would fail the build.

Manual removal of includes based on include-what-you-use's output, filtered for includes that should be removed. The goal is to avoid unnecessary build dependencies, reducing the amount of code that needs to be rebuilt during incremental builds. This is a second iteration of this cleanup; the first round took place in 450845d.

This is to make sure that future pull requests do not introduce unnecessary includes (or remove ones that are no longer necessary).

tautschnig self-assigned this Oct 12, 2022

tautschnig force-pushed the cleanup/iwyu-2 branch 2 times, most recently from bac0f77 to a0cf7f0 Compare October 12, 2022 21:18

peterschrammel approved these changes Oct 13, 2022

View reviewed changes

tautschnig force-pushed the cleanup/iwyu-2 branch from a0cf7f0 to 375e82a Compare October 13, 2022 06:59

tautschnig marked this pull request as ready for review October 13, 2022 06:59

tautschnig requested review from kroening, pkesseli, thomasspriggs, NlightNFotis, TGWDB, chris-ryder, martin-cs, feliperodri, remi-delmas-3000 and romainbrenguier as code owners October 13, 2022 06:59

tautschnig assigned kroening, remi-delmas-3000 and feliperodri and unassigned tautschnig Oct 13, 2022

kroening approved these changes Oct 13, 2022

View reviewed changes

kroening removed their assignment Oct 13, 2022

TGWDB approved these changes Oct 13, 2022

View reviewed changes

feliperodri approved these changes Oct 13, 2022

View reviewed changes

feliperodri assigned chris-ryder and unassigned remi-delmas-3000 and feliperodri Oct 13, 2022

chris-ryder approved these changes Oct 25, 2022

View reviewed changes

tautschnig force-pushed the cleanup/iwyu-2 branch from c22cea8 to da93c7f Compare October 27, 2022 09:43

tautschnig removed their assignment Oct 27, 2022

thomasspriggs suggested changes Oct 27, 2022

View reviewed changes

tautschnig force-pushed the cleanup/iwyu-2 branch from da93c7f to e0080b3 Compare October 27, 2022 11:01

tautschnig assigned thomasspriggs Oct 27, 2022

thomasspriggs approved these changes Oct 27, 2022

View reviewed changes

tautschnig unassigned thomasspriggs Oct 27, 2022

thomasspriggs reviewed Oct 27, 2022

View reviewed changes

src/goto-symex/symex_main.cpp Outdated Show resolved Hide resolved

tautschnig force-pushed the cleanup/iwyu-2 branch 3 times, most recently from f3c9220 to ae5a2b6 Compare October 27, 2022 18:39

tautschnig force-pushed the cleanup/iwyu-2 branch 2 times, most recently from 8f7394b to ddebc88 Compare November 4, 2022 13:41

martin-cs approved these changes Nov 19, 2022

View reviewed changes

feliperodri assigned tautschnig and unassigned martin-cs Nov 19, 2022

tautschnig added 7 commits November 19, 2022 23:00

memory-analyzer unit test: include the header file, not cpp

714437d

There was a spurious include of the implementation file rather than the header.

Do not construct a namespace ad hoc when the environment provides one

469895b

The empty namespace will lack symbol information that may be needed, and is just unnecessary anyway.

Make format unit test do something

159e5fd

6b88028 removed the existing test (for good reasons) but didn't replace it with any other.

Remove stray whitespace in comment header

34106ac

Whitespace cleanup only so as not to break syntax highlighting.

Add validate to symbol_table_baset

72044eb

This enables use of symbol_table_baset instead of symbol_tablet in many places.

Use symbol_table_baset instead of symbol_tablet where possible

1a9ec4e

We almost never create a fresh symbol table, and any time we do so we should really make sure this is the intended behaviour.

Mark includes that include-what-you-use spuriously suggests to remove

a2e3dd4

This is to make sure that we can run include-what-you-use without being told to remove includes when doing so would fail the build.

tautschnig force-pushed the cleanup/iwyu-2 branch from ddebc88 to 5712417 Compare November 19, 2022 23:01

tautschnig added 2 commits November 20, 2022 09:54

Add include-what-you-use as GitHub action

a8514e0

This is to make sure that future pull requests do not introduce unnecessary includes (or remove ones that are no longer necessary).

tautschnig force-pushed the cleanup/iwyu-2 branch from 5712417 to a8514e0 Compare November 20, 2022 09:54

tautschnig merged commit d932d6f into diffblue:develop Nov 20, 2022

tautschnig deleted the cleanup/iwyu-2 branch November 20, 2022 15:01

tautschnig mentioned this pull request Nov 25, 2022

IWYU failing on develop and also fails too early #7383

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Remove unnecessary includes #7235

Remove unnecessary includes #7235

Uh oh!

tautschnig commented Oct 12, 2022 •

edited

Loading

Uh oh!

codecov bot commented Oct 12, 2022 •

edited

Loading

Uh oh!

feliperodri left a comment

Uh oh!

tautschnig commented Oct 13, 2022

Uh oh!

chris-ryder left a comment

Uh oh!

thomasspriggs Oct 27, 2022

Uh oh!

tautschnig Oct 27, 2022

Uh oh!

thomasspriggs Oct 27, 2022

Uh oh!

tautschnig Oct 27, 2022

Uh oh!

thomasspriggs Oct 27, 2022

Uh oh!

tautschnig Oct 27, 2022

Uh oh!

thomasspriggs Oct 27, 2022

Uh oh!

tautschnig Oct 27, 2022

Uh oh!

Uh oh!

martin-cs left a comment

Uh oh!

Uh oh!

		#include <util/nodiscard.h>

		#include <solvers/smt2_incremental/ast/smt_responses.h>

Remove unnecessary includes #7235

Remove unnecessary includes #7235

Uh oh!

Conversation

tautschnig commented Oct 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Oct 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

feliperodri left a comment

Choose a reason for hiding this comment

Uh oh!

tautschnig commented Oct 13, 2022

Uh oh!

chris-ryder left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

martin-cs left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tautschnig commented Oct 12, 2022 •

edited

Loading

codecov bot commented Oct 12, 2022 •

edited

Loading