Skip to content
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

VisitorBase::Clone does not handle declaration replacement #439

Closed
parth-07 opened this issue Apr 30, 2022 · 3 comments
Closed

VisitorBase::Clone does not handle declaration replacement #439

parth-07 opened this issue Apr 30, 2022 · 3 comments

Comments

@parth-07
Copy link
Collaborator

parth-07 commented Apr 30, 2022

We often use the VisitorBase::Clone function in all the Visitor classes to perform cloning of the original expression. This is mostly used for the cases where we do not need the derivative of primal expression, as in those cases, we use Visit(...) function instead.

VisitorBase::Clone function does not handle declaration replacement, and can thus produce incorrect cloned expressions. In all the Visitor classes, declaration replacement is being handled in VisitDeclRefExpr. Therefore, currently, the only way to get correct cloned expression in all cases is using Visit function.

@ro4i7
Copy link

ro4i7 commented Mar 11, 2023

Hello @parth-07 @vgvassilev

To solve this issue, you can modify the VisitorBase::Clone function to handle declaration replacement by calling the VisitDeclRefExpr function for each DeclRefExpr encountered during the cloning process. Here is an example implementation:

clang::Expr* VisitorBase::Clone(const clang::Expr* E) {
  // Create a new expression of the same type as the original.
  clang::Expr* NewExpr = E->Clone(context);
  // Replace declarations in the cloned expression.
  ReplaceDecls(NewExpr);
  return NewExpr;
}

void VisitorBase::ReplaceDecls(clang::Expr* E) {
  if (!E) return;
  switch (E->getStmtClass()) {
    case clang::Stmt::DeclRefExprClass: {
      clang::DeclRefExpr* DRE = llvm::cast<clang::DeclRefExpr>(E);
      clang::ValueDecl* VD = DRE->getDecl();
      if (clang::VarDecl* Var = llvm::dyn_cast<clang::VarDecl>(VD)) {
        if (Var->hasInit()) {
          // Recursively clone the initializer expression.
          clang::Expr* Init = Clone(Var->getInit());
          // Replace the initializer in the cloned expression.
          Var->setInit(Init);
        }
      }
      break;
    }
    default:
      // Recursively replace declarations in child expressions.
      for (auto& Child : E->children()) {
        ReplaceDecls(Child);
      }
      break;
  }
}

This implementation uses a ReplaceDecls function to recursively replace declarations in the cloned expression. It checks each DeclRefExpr encountered during the cloning process and calls Clone on the initializer expression if it exists. Then, it replaces the initializer in the cloned expression. Finally, it recursively replaces declarations in child expressions.

With this modification, you should be able to use VisitorBase::Clone to correctly clone expressions with declaration replacement in all cases.

@vgvassilev
Copy link
Owner

vgvassilev commented Mar 11, 2023

@ro4i7, please stop commenting under every issue. Select one or two issues and work to solve them, then repeat.

PS: Solving means submit a PR, get a successful review and a merged PR.

PetroZarytskyi added a commit to PetroZarytskyi/clad that referenced this issue Dec 1, 2023
vgvassilev pushed a commit to PetroZarytskyi/clad that referenced this issue Dec 1, 2023
This patch request optimizes storing and restoring in the reverse mode of Clad
and introduces TBR analysis to determine when variables should be stored.

Fixes vgvassilev#465, vgvassilev#441, vgvassilev#439. Partially resolves vgvassilev#429 and vgvassilev#606.
vgvassilev pushed a commit to PetroZarytskyi/clad that referenced this issue Dec 1, 2023
This patch optimizes storing and restoring in the reverse mode of Clad and
introduces TBR analysis to determine when variables should be stored.

Fixes vgvassilev#465, vgvassilev#441, vgvassilev#439. Partially resolves vgvassilev#429 and vgvassilev#606.
vgvassilev pushed a commit to PetroZarytskyi/clad that referenced this issue Dec 1, 2023
This patch optimizes storing and restoring in the reverse mode of Clad and
introduces TBR analysis to determine when variables should be stored.

Fixes vgvassilev#465, vgvassilev#441, vgvassilev#439, vgvassilev#429. Partially resolves vgvassilev#606.
vgvassilev pushed a commit to PetroZarytskyi/clad that referenced this issue Dec 1, 2023
This patch optimizes storing and restoring in the reverse mode of Clad and
introduces TBR analysis to determine when variables should be stored.

Fixes vgvassilev#465, vgvassilev#441, vgvassilev#439, vgvassilev#429. Partially resolves vgvassilev#606.
vgvassilev pushed a commit to PetroZarytskyi/clad that referenced this issue Dec 1, 2023
This patch optimizes storing and restoring in the reverse mode of Clad and
introduces TBR analysis to determine when variables should be stored.

Fixes vgvassilev#465, vgvassilev#441, vgvassilev#439, vgvassilev#429. Partially resolves vgvassilev#606.
vgvassilev pushed a commit that referenced this issue Dec 1, 2023
This patch optimizes storing and restoring in the reverse mode of Clad and
introduces TBR analysis to determine when variables should be stored.

Fixes #465, #441, #439, #429. Partially resolves #606.
@vgvassilev
Copy link
Owner

Fixed in #655.

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

No branches or pull requests

3 participants