-
Notifications
You must be signed in to change notification settings - Fork 122
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
[WIP] Add Useful Analysis to the forward mode #1120
base: master
Are you sure you want to change the base?
Conversation
18258de
to
d7bef62
Compare
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.
clang-tidy made some suggestions
There were too many comments to post at once. Showing the first 10 out of 11. Check the log or trigger a new build to see more.
public: | ||
/// Function to be differentiated. | ||
const clang::FunctionDecl* Function = nullptr; | ||
bool ReqAdj = true; |
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.
warning: member variable 'ReqAdj' has public visibility [cppcoreguidelines-non-private-member-variables-in-classes]
bool ReqAdj = true;
^
@@ -1063,7 +1063,14 @@ StmtDiff BaseForwardModeVisitor::VisitDeclRefExpr(const DeclRefExpr* DRE) { | |||
// If DRE is of type pointer, then the derivative is a null pointer. | |||
if (clonedDRE->getType()->isPointerType()) | |||
return StmtDiff(clonedDRE, nullptr); | |||
|
|||
if (auto* i = cast<VarDecl>(DRE->getDecl())) { |
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.
warning: 'auto *i' can be declared as 'const auto *i' [readability-qualified-auto]
if (auto* i = cast<VarDecl>(DRE->getDecl())) { | |
if (const auto* i = cast<VarDecl>(DRE->getDecl())) { |
@@ -1063,7 +1063,14 @@ | |||
// If DRE is of type pointer, then the derivative is a null pointer. | |||
if (clonedDRE->getType()->isPointerType()) | |||
return StmtDiff(clonedDRE, nullptr); | |||
|
|||
if (auto* i = cast<VarDecl>(DRE->getDecl())) { |
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.
warning: cast<> in conditional will assert rather than return a null pointer [llvm-prefer-isa-or-dyn-cast-in-conditionals]
if (auto* i = cast<VarDecl>(DRE->getDecl())) { | |
if (auto* i = dyn_cast<VarDecl>(DRE->getDecl())) { |
for (Decl* D : DS->decls()) { | ||
if (!isa<VarDecl>(D)) | ||
continue; | ||
if (auto* VD = cast<VarDecl>(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.
warning: cast<> in conditional will assert rather than return a null pointer [llvm-prefer-isa-or-dyn-cast-in-conditionals]
if (auto* VD = cast<VarDecl>(D)) { | |
if (auto* VD = dyn_cast<VarDecl>(D)) { |
@@ -0,0 +1,69 @@ | |||
#include "clang/AST/RecursiveASTVisitor.h" |
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.
warning: header is missing header guard [llvm-header-guard]
#include "clang/AST/RecursiveASTVisitor.h" | |
#ifndef GITHUB_WORKSPACE_LIB_DIFFERENTIATOR_USEFULANALYZER_H | |
#define GITHUB_WORKSPACE_LIB_DIFFERENTIATOR_USEFULANALYZER_H | |
#include "clang/AST/RecursiveASTVisitor.h" |
lib/Differentiator/UsefulAnalyzer.h:-1:
+
+ #endif
bool m_Useful = false; | ||
bool m_Marking = false; | ||
|
||
std::set<const clang::VarDecl*>& m_UsefulDecls; |
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.
warning: member 'm_UsefulDecls' of type 'std::set<const clang::VarDecl *> &' is a reference [cppcoreguidelines-avoid-const-or-ref-data-members]
std::set<const clang::VarDecl*>& m_UsefulDecls;
^
bool m_Marking = false; | ||
|
||
std::set<const clang::VarDecl*>& m_UsefulDecls; | ||
std::set<const clang::FunctionDecl*>& m_UsefulFuncs; |
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.
warning: member 'm_UsefulFuncs' of type 'std::set<const clang::FunctionDecl *> &' is a reference [cppcoreguidelines-avoid-const-or-ref-data-members]
std::set<const clang::FunctionDecl*>& m_UsefulFuncs;
^
/// \param toAssign - Parameter to initialize new VarsData with. | ||
/// \return Unique pointer to a new object of type Varsdata. | ||
static std::unique_ptr<VarsData> createNewVarsData(VarsData toAssign) { | ||
return std::unique_ptr<VarsData>(new VarsData(std::move(toAssign))); |
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.
warning: use std::make_unique instead [modernize-make-unique]
lib/Differentiator/UsefulAnalyzer.h:7:
- #include <stack>
+ #include <memory>
+ #include <stack>
return std::unique_ptr<VarsData>(new VarsData(std::move(toAssign))); | |
return std::make_unique<VarsData>(std::move(toAssign)); |
|
||
clang::CFGBlock* getCFGBlockByID(unsigned ID); | ||
|
||
clang::ASTContext& m_Context; |
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.
warning: member 'm_Context' of type 'clang::ASTContext &' is a reference [cppcoreguidelines-avoid-const-or-ref-data-members]
clang::ASTContext& m_Context;
^
8f8e30e
to
7d344aa
Compare
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.
clang-tidy made some suggestions
@@ -0,0 +1,72 @@ | |||
#ifndef CLAD_DIFFERENTIATOR_USEFULANALYZER_H | |||
#define CLAD_DIFFERENTIATOR_USEFULANALYZER_H |
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.
warning: header guard does not follow preferred style [llvm-header-guard]
#define CLAD_DIFFERENTIATOR_USEFULANALYZER_H | |
#ifndef GITHUB_WORKSPACE_LIB_DIFFERENTIATOR_USEFULANALYZER_H | |
#define GITHUB_WORKSPACE_LIB_DIFFERENTIATOR_USEFULANALYZER_H |
lib/Differentiator/UsefulAnalyzer.h:71:
+ endif // GITHUB_WORKSPACE_LIB_DIFFERENTIATOR_USEFULANALYZER_H
/// \param toAssign - Parameter to initialize new VarsData with. | ||
/// \return Unique pointer to a new object of type Varsdata. | ||
static std::unique_ptr<VarsData> createNewVarsData(VarsData toAssign) { | ||
return std::unique_ptr<VarsData>(new VarsData(std::move(toAssign))); |
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.
warning: use std::make_unique instead [modernize-make-unique]
lib/Differentiator/UsefulAnalyzer.h:9:
- #include <stack>
+ #include <memory>
+ #include <stack>
return std::unique_ptr<VarsData>(new VarsData(std::move(toAssign))); | |
return std::make_unique<VarsData>(std::move(toAssign)); |
void copyVarToCurBlock(const clang::VarDecl* VD); | ||
VarsData& getCurBlockVarsData() { return *m_BlockData[m_CurBlockID]; } | ||
[[nodiscard]] const VarsData& getCurBlockVarsData() const { | ||
return const_cast<UsefulAnalyzer*>(this)->getCurBlockVarsData(); |
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.
warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast]
return const_cast<UsefulAnalyzer*>(this)->getCurBlockVarsData();
^
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1120 +/- ##
==========================================
- Coverage 94.36% 91.49% -2.88%
==========================================
Files 50 52 +2
Lines 8365 8837 +472
==========================================
+ Hits 7894 8085 +191
- Misses 471 752 +281
... and 29 files with indirect coverage changes
|
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.
clang-tidy made some suggestions
@@ -1063,7 +1063,14 @@ | |||
// If DRE is of type pointer, then the derivative is a null pointer. | |||
if (clonedDRE->getType()->isPointerType()) | |||
return StmtDiff(clonedDRE, nullptr); | |||
|
|||
if (auto* i = dyn_cast<VarDecl>(DRE->getDecl())) { |
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.
warning: 'auto *i' can be declared as 'const auto *i' [readability-qualified-auto]
if (auto* i = dyn_cast<VarDecl>(DRE->getDecl())) { | |
if (const auto* i = dyn_cast<VarDecl>(DRE->getDecl())) { |
No description provided.