refactor(estate): slim elecstate.h via forward declarations#7511
Closed
Critsium-xy wants to merge 7 commits into
Closed
refactor(estate): slim elecstate.h via forward declarations#7511Critsium-xy wants to merge 7 commits into
Critsium-xy wants to merge 7 commits into
Conversation
Reduce the transitive include weight of source_estate/elecstate.h, a header included by ~140 translation units. It previously pulled in 68 headers (chain depth 11) by including parameter.h, potential_new.h, klist.h, charge.h and psi.h directly. Replace those with forward declarations of Charge, K_Vectors, Potential, psi::Psi, Input_para and the PW_Basis types, keeping only fp_energy.h and the newly-explicit matrix.h for value members. The inline constructor and destructor (which dereference Charge / delete Potential) are moved to elecstate.cpp so the pointer members can stay forward-declared. This drops elecstate.h to own_fan=2, depth=1. Add the now-required includes to the downstream files that relied on elecstate.h to provide those headers transitively and use the types via complete-type operations (member access, construction, delete). Only genuine complete-type users are touched; forward-decl-satisfiable by-reference uses are left as-is. Verified: elecstate.h parses standalone (-fsyntax-only); static include analysis reports 0 remaining complete-type breakages across the 216 TUs that transitively include it. Full MPI/FFTW build pending CI. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The previous forward declaration of psi::Psi repeated its default template argument (Device = base_device::DEVICE_CPU), which is also declared in psi.h. Any translation unit including both headers hit "redefinition of default argument" (a default template argument may be specified only once per TU). A standalone parse of elecstate.h did not surface this because the clash only manifests when both headers are combined. Since psi.h is very light (own_fan=2: memory_op.h + types.h), include it directly instead of forward-declaring. elecstate.h own_fan goes 68 -> 5, still a 93% reduction; the heavy headers (potential_new own_fan=58, klist=42, parameter, charge) remain forward-declared. No include cycle is introduced (psi.h pulls only source_base). Also add an explicit parameter.h to rdmft.cpp, which uses PARAM directly but only received it transitively through elecstate.h. Verified: TUs including psi.h + elecstate.h in either order now parse (-fsyntax-only); static analysis reports 0 complete-type breakages. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adopt the cost/benefit-optimal slimming of elecstate.h. Quantified per dropped header (own_fan saved vs downstream files needing a direct include): potential_new.h : own_fan 58, breaks ~5 -> forward-declare klist.h : own_fan 42, breaks ~52 -> forward-declare charge.h : own_fan 37, breaks ~25 -> forward-declare parameter.h : own_fan 4, breaks ~120 -> KEEP included parameter.h saves almost no include weight but its removal would force ~120 downstream PARAM/Input_para users to add their own include, so it is kept in elecstate.h. The three heavy data headers are forward-declared; UnitCell, PW_Basis, ComplexMatrix and Parallel_Grid (reached transitively through klist/charge) are forward-declared too. Result: elecstate.h own_fan 68 -> 10 (85% reduction) with no include cycle. Downstream files that used Charge / Potential / K_Vectors / UnitCell / PW_Basis via complete-type operations and previously got those headers transitively through elecstate.h now include them directly (include-what-you- use). Detection is #ifdef-proof: it flags only files that name the symbol via a complete-type pattern and lack a direct include, ignoring transitive closure (which static parsing cannot evaluate through #ifdef guards). Verified: elecstate.h parses with psi.h in either order (-fsyntax-only); static analysis reports 0 remaining complete-type breakages for the five forward-declared interface headers. Niche types (Magnetism, Symmetry, Atom, PotBase, surchem) are covered transitively by the added unitcell.h / potential_new.h includes. A few rarely-used types (Parallel_Kpoints, PW_Basis_K) may still surface in CI and will be fixed in a follow-up pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ypes The previous IWYU pass only matched by-value declarations and construction of the forward-declared types, missing the most common complete-type use: member access through a pointer (klist->get_nks(), ucell->atoms, chg->...). elecstate_energy.cpp hit "invalid use of incomplete type K_Vectors" on this->klist->get_nks(). Re-scan with comprehensive complete-type patterns (klist->/kv->, ucell->, charge->/chg->, pot->, plus value/construction) and add the direct include wherever the symbol is used that way without one. 47 files updated, mostly klist.h for the widely-used ElecState::klist pointer. Verified: static analysis now reports 0 remaining complete-type breakages for klist/unitcell/charge/potential_new under the comprehensive patterns. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
elecstate_exx.cpp failed with "TITLE is not a member of ModuleBase": ModuleBase::TITLE (tool_title.h) was reaching ~67 files only transitively through the heavy headers that are now forward-declared. The same applies to WARNING_QUIT, timer, GlobalFunc and Memory (~100 files total). These are source_base leaf utilities with own_fan~0, so by the same cost/benefit rule used for parameter.h, include them in elecstate.h rather than churning ~100 downstream files: global_function.h (which also pulls tool_title.h and tool_quit.h), timer.h and memory_recorder.h. Being base-layer, this adds no architectural coupling. elecstate.h own_fan 68 -> 20 (71% reduction); no include cycle. Static analysis: 0 remaining breakages for TITLE/WARNING_QUIT/timer/GlobalFunc/ Memory, and none for constants.h / complexmatrix.h either. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
spin_constrain.h failed with "ModulePW::PW_Basis_K has not been declared": the type was reaching these headers only transitively through elecstate.h's now-forward-declared heavy includes. All three headers (and init_sc.cpp via spin_constrain.h) use PW_Basis_K solely as a pointer member/param, so a forward declaration is the correct fix -- pw_basis_k.h has own_fan=24 and should not be pulled into widely-included headers just for a pointer. Follows the existing convention (exx_lip.h already forward-declares it). Static scan confirms 0 remaining PW_Basis_K or Parallel_Kpoints breakages. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ction
Two more transitive-provision losses from the elecstate.h slimming:
- 'surchem has not been declared' in setup_estate_pw.{h,cpp} and the
ctrl_output / forces headers. surchem is used only as a reference/pointer
param, so forward-declare 'class surchem;' (own_fan of surchem.h is not
worth pulling into these headers for a reference).
- 'invalid use of incomplete type elecstate::Potential' in setup_estate_pw.cpp
et al. The construction is 'new elecstate::Potential(...)', namespace-
qualified, which the previous complete-type scan (new\s+Potential) missed.
Add potential_new.h to the 3 files that construct it that way.
The detection pattern is broadened to new\s+(?:\w+::)?Type to catch
namespace-qualified construction generally. Comprehensive re-scan across
Charge/K_Vectors/UnitCell/Potential/PW_Basis_K/surchem now reports 0 real
breakages (the lone flag is init_sc.cpp, covered by spin_constrain.h's
forward declaration).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reduce the transitive include weight of source_estate/elecstate.h. It previously pulled in 68 headers (chain depth 11) by including parameter.h, potential_new.h, klist.h, charge.h and psi.h directly.
Parameter.h can't be removed now temporarily because it is included in too many files😅, this will be done later