510212 - New .hasDefaults property does not seem to work

Status: RESOLVED WONTFIX

(Developer Infrastructure :: Source Code Analysis, defect)

Description

[Diego Novillo]

I have been trying to get at the .hasDefault property for functions, but I can't seem to see it.  For this test case:

int foo(int y, long x=42);
int X;
int bar(void)
{
  foo(X);
  foo(X,X);
}

The attached script does not show hasDefault where I was expecting it (in the params for foo).

Comment 1

[Diego Novillo]

Attachment: Script to dump decls/types/functions found

Comment 2

[[e-mail]]

The problem is that dehydra_convertAttachFunctionType(), which adds the default properties (to the decl.type.hasDefault array), gets called once. dehydra_moveDefaults(), which moves those defaults to each parameter in decl.parameters, gets called 3 times... and it removes decl.type.hasDefault when it's done.

I'm guessing that somehow decl.parameters gets overwritten between one of those 3 calls, and hasDefault gets lost. Taras, any ideas?

Comment 3

[(dormant account)]

(In reply to comment #2)

> I'm guessing that somehow decl.parameters gets overwritten between one of those
> 3 calls, and hasDefault gets lost. Taras, any ideas?

I doubt it's getting overwritten. The problem is that I decided to make decl objects(also known as variables) to be distinct in every use. It seemed like a good idea so one could add attributes, delete stuff, etc without worrying, but now I'm not sure anyone actually benefits from that.

So what i think you are seeing is that the function decl is being used in another context, it steals the defaults array and the next time decls are being created the defaults array is gone.

I think for now the fix is to not destructively move the defaults. In the longer run it probably makes sense to make decls 1:1 with gcc decls just like types are.

Comment 4

[[e-mail]]

Ok - will whip up a patch.

Comment 5

[[e-mail]]

Attachment: make hasDefault hidden, v1 (checked in)

Hides the old hasDefaults property, so it won't show up under enumeration in js. Best we can do for now.

This doesn't do anything for function calls, I'll do that separately.

Comment 6

[Diego Novillo]

Attachment: Recognize default argument values in CALL_EXPR

This patch makes dehydra recognize arguments using default values.  The way it does it is a bit wonky.  Particularly how I get to the argument that dehydra_fcallDoArgs builds.  What would be a better way to access the object that has just been added to the array of arguments?

The real problem with this approach is that it's not really sufficient to distinguish all uses of default values.  The main problem is that the parser never tells us that a CALL_EXPR is using a default value.

When the parser builds a call expression, it checks what arguments are
used by default and if the user didn't specify and argument it puts
the default value in the call.  However, it never marks that
replacement, so by the time we see the call expression in dehydra, the
call 'foo()' has been replaced with 'foo(42)' and there is no
indication that '42' has been injected by the parser.

What I've done is inspect the function prototype.  If the argument I'm
examining is the same expression as the default argument, then I mark
that argument with the .hasDefault property (meaning that the argument
has the default value).  However, this does not necessarily mean that
the user called 'foo()', they could've called 'foo(42)'.

A definitive solution for this involves changing the parser, but at the moment this is not something I can do easily.

Is this acceptable for now?

I now need to figure out how to deal with default arguments in template instantiation.

Comment 7

[(dormant account)]

(In reply to comment #6)

> Is this acceptable for now?

yes, nice hack. You really need to add testcases, it's really easy, just name your file test/test_*.js and specify some details about it in a comment on top of the file.

Comment 8

[[e-mail]]

Comment on attachment 394600 [details] [diff] [review]
make hasDefault hidden, v1 (checked in)

Checked in per bug 512887.

Comment 9

[[e-mail]]

Reassigning to nobody. If anyone wants to work on this, feel free!

Comment 10

[Joshua Cranmer [:jcranmer]]

Dehydra and treehydra are no longer maintained by Mozilla.