-
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
Add support for null (empty) statements #900
Conversation
clang-tidy review says "All clean, LGTM! 👍" |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #900 +/- ##
==========================================
- Coverage 94.15% 94.09% -0.07%
==========================================
Files 53 53
Lines 7803 7805 +2
==========================================
- Hits 7347 7344 -3
- Misses 456 461 +5
... and 2 files with indirect coverage changes
|
Can you break the long commit message lines at 80-columns? This has been somewhat standard. |
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.
LGTM, assuming the commit message is addressed.
@@ -109,6 +109,7 @@ class BaseForwardModeVisitor | |||
const clang::SubstNonTypeTemplateParmExpr* NTTP); | |||
StmtDiff VisitImplicitValueInitExpr(const clang::ImplicitValueInitExpr* IVIE); | |||
StmtDiff VisitCStyleCastExpr(const clang::CStyleCastExpr* CSCE); | |||
StmtDiff VisitNullStmt(const clang::NullStmt* NS) { return StmtDiff{}; }; |
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 suspect there is a lot of such cases for the different types of statements. Maybe you can take a look at the statements we do not have a Visit routine for. Some of them could be as trivial and easy to test. That'd increase the coverage of supported things generally speaking.
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.
alright, I'll look for things like this
clang-tidy review says "All clean, LGTM! 👍" |
1 similar comment
clang-tidy review says "All clean, LGTM! 👍" |
clang-tidy review says "All clean, LGTM! 👍" |
This commit removes the "unsupported" warning when differentiating code with ";" in both forward and reverse modes. It also prevents additional semicolons from getting pulled into the derivative code. Fixes: vgvassilev#899
clang-tidy review says "All clean, LGTM! 👍" |
Codecov complains because now the NullStmts have their own routine and we do not fall back into |
This commit removes the "unsupported" warning when differentiating code with ";" in both forward and reverse modes. It also prevents additional semicolons from getting pulled into the derivative code.
Fixes: #899