Moving DerivedFnCollector out of ClangPlugin #868
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #868 +/- ##
==========================================
+ Coverage 94.77% 94.81% +0.04%
==========================================
Files 49 51 +2
Lines 7596 7618 +22
==========================================
+ Hits 7199 7223 +24
+ Misses 397 395 -2
... and 3 files with indirect coverage changes
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -122,7 +122,7 @@ namespace clad { | |||
| Sema& S = m_CI.getSema(); | |||
|
|
|||
| if (!m_DerivativeBuilder) | |||
| m_DerivativeBuilder.reset(new DerivativeBuilder(S, *this)); | |||
| m_DerivativeBuilder.reset(new DerivativeBuilder(S, *this, m_DFC)); | |||
There was a problem hiding this comment.
warning: use std::make_unique instead [modernize-make-unique]
| m_DerivativeBuilder.reset(new DerivativeBuilder(S, *this, m_DFC)); | |
| m_DerivativeBuilder = std::make_unique<DerivativeBuilder>(S, *this, m_DFC); |
There was a problem hiding this comment.
not sure if we want to apply this suggestion?
There was a problem hiding this comment.
No, that’s only available in c++14 onward.
| @@ -122,7 +122,7 @@ | |||
| Sema& S = m_CI.getSema(); | |||
|
|
|||
| if (!m_DerivativeBuilder) | |||
| m_DerivativeBuilder.reset(new DerivativeBuilder(S, *this)); | |||
| m_DerivativeBuilder.reset(new DerivativeBuilder(S, *this, m_DFC)); | |||
There was a problem hiding this comment.
warning: initializing non-owner argument of type 'std::unique_ptrclad::DerivativeBuilder::pointer' (aka 'clad::DerivativeBuilder *') with a newly created 'gsl::owner<>' [cppcoreguidelines-owning-memory]
m_DerivativeBuilder.reset(new DerivativeBuilder(S, *this, m_DFC));
^
| @@ -0,0 +1,40 @@ | |||
| #ifndef CLAD_DIFFERENTIATOR_DERIVEDFNCOLLECTOR_H | |||
There was a problem hiding this comment.
Can we move that header file and its friends in lib?
There was a problem hiding this comment.
Any specific reason as to why we want to move this header file to lib? I thought all header files go in include directory.
There was a problem hiding this comment.
These header files are "local"/"implementation internal" files that are related to the build artifact not the user interface. That is, we do not need to have them installed for clad to work...
There was a problem hiding this comment.
I tried it, but the issue is that this file is imported by DerivativeBuilder.h, which is further imported by all the Visitors.
vaithak
force-pushed
the
derived-fn-info
branch
from
ea30bb9 to
49b2006
Compare
vaithak
force-pushed
the
derived-fn-info
branch
from
49b2006 to
b00c0c1
Compare
This is part of the initial refactoring required to remove direct access to ClangPlugin methods.