Skip to content

Add support for SourceLocExpr - #1929

Merged
vgvassilev merged 1 commit into
vgvassilev:masterfrom
Vedant2005goyal:assert_builtin_support
Jul 30, 2026
Merged

Add support for SourceLocExpr#1929
vgvassilev merged 1 commit into
vgvassilev:masterfrom
Vedant2005goyal:assert_builtin_support

Conversation

@Vedant2005goyal

@Vedant2005goyal Vedant2005goyal commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

This PR extends Clad's AST visitor to support SourceLocExpr, enabling differentiation of code containing assert statements and other source-location builtins for both forward and reverse mode.

@Vedant2005goyal
Vedant2005goyal marked this pull request as draft July 28, 2026 10:00

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

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

DECLARE_CLONE_FN(CXXCatchStmt)
DECLARE_CLONE_FN(CXXTryStmt)
DECLARE_CLONE_FN(PredefinedExpr)
DECLARE_CLONE_FN(SourceLocExpr)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no header providing "clang::SourceLocExpr" is directly included [misc-include-cleaner]

    DECLARE_CLONE_FN(SourceLocExpr)
                     ^

@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@vgvassilev

Copy link
Copy Markdown
Owner

We need a test.

@Vedant2005goyal
Vedant2005goyal force-pushed the assert_builtin_support branch from e890f82 to 56b1ca8 Compare July 29, 2026 15:43
@Vedant2005goyal
Vedant2005goyal marked this pull request as ready for review July 29, 2026 15:45
@Vedant2005goyal

Copy link
Copy Markdown
Contributor Author

We need a test.

Done!

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

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

StmtDiff VisitWhileStmt(const clang::WhileStmt* WS);
StmtDiff VisitDoStmt(const clang::DoStmt* DS);
StmtDiff VisitContinueStmt(const clang::ContinueStmt* ContStmt);
StmtDiff VisitSourceLocExpr(const clang::SourceLocExpr* E);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no header providing "clang::SourceLocExpr" is directly included [misc-include-cleaner]

  StmtDiff VisitSourceLocExpr(const clang::SourceLocExpr* E);
                                           ^

clad_compat::ExprSetDeps(result, Node);
return result;
}
#if CLANG_VERSION_MAJOR >= 15

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no header providing "CLANG_VERSION_MAJOR" is directly included [misc-include-cleaner]

lib/Differentiator/StmtClone.cpp:17:

+ #include <clang/Basic/Version.h>

return result;
}
#if CLANG_VERSION_MAJOR >= 15
DEFINE_CLONE_EXPR(SourceLocExpr,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: initializing non-owner 'SourceLocExpr *' with a newly created 'gsl::owner<>' [cppcoreguidelines-owning-memory]

DEFINE_CLONE_EXPR(SourceLocExpr,
                  ^
Additional context

lib/Differentiator/StmtClone.cpp:38: expanded from macro 'DEFINE_CLONE_EXPR'

  CLASS* result = new (Ctx) CLASS CTORARGS;             \
  ^

@Vedant2005goyal
Vedant2005goyal force-pushed the assert_builtin_support branch from 56b1ca8 to 812ec46 Compare July 30, 2026 09:52

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

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

for (unsigned i = 0, e = CE->getNumArgs(); i < e; ++i) {
for (unsigned i = 0, e = diffArgs.size(); i < e; ++i) {
Expr* dArg = diffArgs[i];
// If argDiff.expr_dx is nullptr or is a constant 0, then the derivative

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: use range-based for loop instead [modernize-loop-convert]

Suggested change
// If argDiff.expr_dx is nullptr or is a constant 0, then the derivative
for (auto dArg : diffArgs) {
// If argDiff.expr_dx is nullptr or is a constant 0, then the derivative

@Vedant2005goyal
Vedant2005goyal force-pushed the assert_builtin_support branch 2 times, most recently from 167a65e to e75e0af Compare July 30, 2026 10:31
…ontaining assert and other source-location builtins.
@Vedant2005goyal
Vedant2005goyal force-pushed the assert_builtin_support branch from e75e0af to c37b38f Compare July 30, 2026 11:01

@vgvassilev vgvassilev left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@vgvassilev
vgvassilev merged commit fde70aa into vgvassilev:master Jul 30, 2026
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants