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

Reported coordinates not accurate for BinaryOp while getting json dump of AST #411

Open
akash-isu opened this issue Mar 18, 2021 · 4 comments

Comments

@akash-isu
Copy link

akash-isu commented Mar 18, 2021

Look at this code:

#include<stdio.h>
int main(int argc, char \*argv[]) { 
	int a,i,j,k;
	scanf("%d",&a);
	k=a%10;
	j=a/10%10;
	if(a>0)
		printf("%d",a);
	else 
	{
		if(k>j)
			printf("%d",a/100\*10+k);
		
		else printf("%d",a/10);

	}
	return 0;
}

And consider the only the JSON output for only the binary operators:

{'_nodetype': 'BinaryOp', 'coord': '313-A-bug-16465019-16465065/313-A-16465065.c:6:4', 'left': {'_nodetype': 'BinaryOp', 'coord': '313-A-bug-16465019-16465065/313-A-16465065.c:6:4', 'left': {'_nodetype': 'ID', 'coord': '313-A-bug-16465019-16465065/313-A-16465065.c:6:4', 'name': 'a'}, 'op': '/', 'right': {'_nodetype': 'Constant', 'coord': '313-A-bug-16465019-16465065/313-A-16465065.c:6:6', 'type': 'int', 'value': '10'}}, 'op': '%', 'right': {'_nodetype': 'Constant', 'coord': '313-A-bug-16465019-16465065/313-A-16465065.c:6:9', 'type': 'int', 'value': '10'}}

{'_nodetype': 'BinaryOp', 'coord': '313-A-bug-16465019-16465065/313-A-16465065.c:6:4', 'left': {'_nodetype': 'ID', 'coord': '313-A-bug-16465019-16465065/313-A-16465065.c:6:4', 'name': 'a'}, 'op': '/', 'right': {'_nodetype': 'Constant', 'coord': '313-A-bug-16465019-16465065/313-A-16465065.c:6:6', 'type': 'int', 'value': '10'}}

How can the coordinates 6:4 have 2 operators? (namely '%' and '/')
This is the line in question:
j=a/10%10;

Am I missing something or is this the expected behavior?
I've only included the sections of the JSON in question for easier readability.

@eliben
Copy link
Owner

eliben commented Mar 18, 2021

A simpler reproducer:

int a = b / 10 % 20;

It seems like the coordinate attribution for binary ops could be improved, though it's not clear what the right way to report it is. The way it currently works is that the BinaryOp node gets the coord of its LHS. In this case both LHSes start at the first a. Alternatively the op's coord could be its operator's coord, though it's not clear whether this is the best choice either.

@akash-isu
Copy link
Author

To me, a good approach seems to be a relative coordinate reporting for the individual operators wrt the index of the LHS.
For example, for the given instance:
if the coord of the LHS is 1:4, then the coord for '/' can be 1:4:6, where 6 is the number of spaces between the LHS and the operator.
I suggest this approach, as the 'tab' and 'space' counts often varies between the actual code and the pre-processed code.

@eliben
Copy link
Owner

eliben commented Mar 18, 2021

I think that what's often done is nodes would have a start coord and end coord, covering their extent, e.g. consider:

int a 
     +
     b;

This is valid code, and yet where would you report the coord for the binary op node? Right now it's reported on a, but it could also legitimately be on +. It could be that we want to extend it to be with a start coord + end coord, covering the whole binary op.

It's a large feature request though.

@JulianKemmerer
Copy link

JulianKemmerer commented Mar 18, 2021

This is probably bad advice - but what I did to get around this issue in the meantime when I came across it
(which at the time, I thought too roughly it's not clear what the right way to report it is and that it would always be an issue...)
was to identify with the normal ex. 1:4 but then also did a python like hash(str(c_ast_node)) The two nodes have the same coord but their text representation is still unique - hash converts it to some number. Saving identifiers like my_bin_op_1:4_0xdeadbeef

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants