Fix isUnusedResult to work with clad::array

[Daksh1603]

Fixes #350

This pull request addresses an issue with the isUnusedResult function in CLAD, which currently does not correctly identify unused results for clad::array expressions.

To fix this issue , I have made some minor changes to the isUnusedResult function from the clad/Differentiator/VisitorBase.cpp file which explicity checks whether the passed expression is an instance of 'clad::array'.

[ShounakDas101]

what is " Expr " type?

[Daksh1603]

Hi @ShounakDas101 ,

Expr is basically a class from the Clang AST . It is capable of representing multiple C/C++ expressions like operators,literal . I myself am trying to figure out Clad and Clang and as you dwell into the code and documentation , more things will make sense. I might not be entirely right but this is what I think of Expr as of now . Please let me know if you find a better explanation for the following .

[ShounakDas101]

Hi @Daksh1603 How did you check your code? I tried running this code and a few variations

#include "clad/Differentiator/Differentiator.h" #include

int main() { //int a[1]; //a[0]=0; //maybe change a[0] =3 clad::array a(3);

if(a[0])
{
	std::cout<<"true";
}
else
{
	std::cout<<"false";
}
return 1;

}

I dont think I can replicate the error properly with this nor did your changes seem to affect the outputs.

Also in the issue this is the example given

double arr(3); <-------- is this just a normal array declaration or smth else? arr[1]; // <- isUnusedResult returns true for this statement

Note: for some reason iostream not showing up

[codecov[bot]]

Codecov Report

Merging #543 (aaa9bdb) into master (7c80111) will increase coverage by 0.00%. The diff coverage is 100.00%.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #543   +/-   ##
=======================================
  Coverage   92.50%   92.50%           
=======================================
  Files          39       39           
  Lines        5600     5602    +2     
=======================================
+ Hits         5180     5182    +2     
  Misses        420      420           

Impacted Files	Coverage Δ
lib/Differentiator/VisitorBase.cpp	97.64% <100.00%> (+0.01%)	:arrow_up:

Impacted Files	Coverage Δ
lib/Differentiator/VisitorBase.cpp	97.64% <100.00%> (+0.01%)	:arrow_up:

[parth-07]

@Daksh1603 I was expecting some test changes as well that I don't see.

Can you also please answer to @ShounakDas101 questions?

[Daksh1603]

Hi @ShounakDas101 ,

Apologies for the late reply , as I had my mid semester test today . I don't think the code snippet you mentioned is the right way to replicate this error . In my opinion , for you to replicate this error in it's entirety , try running the following code :

#include <iostream>
#include "clad/Differentiator/Differentiator.h"
#include "clad/Differentiator/Array.h"

double fn(double* a) {
  	clad::array<int> b(3);
	b[1];
	double arr1[3];
	arr1[1];
        return a[1];
}

int main() {
  clad::array<double> a(10); 
  auto darray_dx = clad::differentiate(fn,"a[0]");
  std::cout<<darray_dx.execute(a)<<"\n"; 
  return 0;
}

However , this is what I think is right and may not be absolutely correct .

[Daksh1603]

Hi @parth-07 @vgvassilev , As I dwell into this problem more , I am discovering more vulnerabilities in my commit . The earlier solution which I implemented doesn't seem to work for me , but I couldn't find the flaw . I have 2 possible questions and wanted to ask you for help ->

• I have two separate build directories for the CLAD plugin . One is the standard CLAD build and the other is my own forked CLAD build , to where I make changes . Now in order for me to inspect the changes I have made and whether the issue is really solved , I need to understand how can I swap between these two builds at ease . Usually , I just end up running the make && make install command for respective build directory . But perhaps , this might be the wrong way to go about it .
• Secondly , I tried implementing an alternate solution for this , please have a look ->

if (auto array = dyn_cast<ArraySubscriptExpr>(E)) {
        if (auto variable = dyn_cast<VarDecl>(array->getBase()->getReferencedDeclOfCallee())) {
                if (variable->getNameAsString()== "clad::array") {
                    return !array->getBase()->isLValue();
                }
            }
        }

This seems like a reasonable approach but what concerns me is the fact that getReferencedDeclOfCallee() might end up pointing to the operator which is being overloaded instead . A little more guidance will be appreciated on this and I am confident that Ill end up fixing it at the earliest .

[parth-07]

• I have two separate build directories for the CLAD plugin . One is the standard CLAD build and the other is my own forked CLAD build , to where I make changes . Now in order for me to inspect the changes I have made and whether the issue is really solved , I need to understand how can I swap between these two builds at ease . Usually , I just end up running the make && make install command for respective build directory . But perhaps , this might be the wrong way to go about it .

Why do you need two builds? Ideally, you do not need two builds at all. You can have one build with multiple remotes. And did you globally install Clad in your system? I would recommend locally installing Clad. That generally makes things more clear. And this way, if you have multiple builds, then you can choose which Clad to use when running a program with Clad plugin, by passing the correct path to the Clad plugin library when running the clang command.

• Secondly , I tried implementing an alternate solution for this , please have a look ->

if (auto array = dyn_cast<ArraySubscriptExpr>(E)) {
        if (auto variable = dyn_cast<VarDecl>(array->getBase()->getReferencedDeclOfCallee())) {
                if (variable->getNameAsString()== "clad::array") {
                    return !array->getBase()->isLValue();
                }
            }
        }

This seems like a reasonable approach but what concerns me is the fact that getReferencedDeclOfCallee() might end up pointing to the operator which is being overloaded instead . A little more guidance will be appreciated on this and I am confident that Ill end up fixing it at the earliest .

This solution has some major flaws. It seems it would only work for clad::array. Do pure array types have the same issue as of clad::array? If yes, then we should fix that as well. Here the important thing to check is, if an expression has some side effects, it does then we want to keep it. If not, then we can skip outputting them. I suspect there may be some Clang API to check if an expression has side-effects, but I am not sure at the moment.

[vgvassilev]

Let's close this PR as the issue was fixed in #797.