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.
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.
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
explicitly instantiate CrystalLattice simplify initialization #5341
explicitly instantiate CrystalLattice simplify initialization #5341
Changes from 1 commit
aaf5cfc
f4cc3b2
26aee19
845e77a
6fa7137
2455429
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Check warning on line 43 in src/Particle/Lattice/CrystalLattice.cpp
src/Particle/Lattice/CrystalLattice.cpp#L43
Check warning on line 47 in src/Particle/Lattice/CrystalLattice.cpp
src/Particle/Lattice/CrystalLattice.cpp#L45-L47
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this really needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't successfully forward declare xmlNodePtr. This is needed to support the factory function I use instead of the stateful LatticeParser in later PR's.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't mean using forward declare. I cannot find any use of xmlNodePtr under
src/Particle/Lattice
and thus feel the actual consumer of xmlNodePtr should have the include line.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That causes the include to basically always be included before CrystalLattice.h, I did the right thing and figured out how to foward declare it.
Check warning on line 92 in src/Particle/Lattice/CrystalLattice.h
src/Particle/Lattice/CrystalLattice.h#L90-L92
Check warning on line 99 in src/Particle/Lattice/CrystalLattice.h
src/Particle/Lattice/CrystalLattice.h#L94-L99
Check warning on line 103 in src/Particle/Lattice/CrystalLattice.h
src/Particle/Lattice/CrystalLattice.h#L101-L103
Check warning on line 110 in src/Particle/Lattice/CrystalLattice.h
src/Particle/Lattice/CrystalLattice.h#L106-L110
Check warning on line 113 in src/Particle/Lattice/CrystalLattice.h
src/Particle/Lattice/CrystalLattice.h#L112-L113
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the dimension be just
D
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can't partially specialize a class friend. So as soon as this stops being a struct and we stop direct access to lattice internals. The assignment operator becomes harder to write and dependent on accessors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. I forgot...