Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 318785

Summary: DeclaratorWriter unable to write a pointer to array
Product: [Tools] CDT Reporter: Tomasz Wesolowski <kosashi>
Component: cdt-coreAssignee: Project Inbox <cdt-core-inbox>
Status: CLOSED INVALID QA Contact: Doug Schaefer <cdtdoug>
Severity: normal    
Priority: P3 CC: emanuel
Version: 7.0   
Target Milestone: ---   
Hardware: PC   
OS: Windows 7   
Whiteboard:
Bug Depends on:    
Bug Blocks: 318784    
Attachments:
Description Flags
null checks around found NPEs, to be examined
none
test case NOT producing this NPE emanuel: iplog-

Description Tomasz Wesolowski CLA 2010-07-03 09:46:19 EDT
Build Identifier: 

There's a problem in DeclaratorWriter. See:
org.eclipse.cdt.internal.core.dom.rewrite.astwriter.DeclaratorWriter.writeArrayDeclarator(IASTArrayDeclarator)

There's a line:

  IASTName name = arrDecl.getName();
  name.accept(visitor);

This results in an NPE when arrDecl hasn't got a name (i.e. it has a nested declarator). Example code affected by the problem is a pointer to array of pointers declaration:

  int* (*a)[2];

Which cannot be generated by this class now.

A simple null check around name.accept(visitor) seemed to do the trick for me, but I don't really know how DeclaratorWriter works in detail, so I'd like someone to have a look over there.

Reproducible: Always

Steps to Reproduce:
1. Create a new .cpp file and type this code:

void func() {
    int* (*a)[2];
    a;
}

2. Select `a` in the second statement and run Refactoring -> Extract Local Variable
3. Observe the NPE

Note that int (*a)[2] should also reproduce this problem, but doesn't at the moment, as a wrong declaration is made by DeclarationGenerator (see bug 318784).
Comment 1 Tomasz Wesolowski CLA 2010-07-03 17:30:18 EDT
Created attachment 173360 [details]
null checks around found NPEs, to be examined

I've found some more (3?) NPE cases when generating declarations for function pointers.
This time, it's mostly about the function parameters and function name in IASTFunctionDeclarator - both the parameters and the function declarator itself may have undefined name (e.g. in function pointers).

I'm attaching a patch with null checks around them which make the code generation work, but - as I've mentioned - I don't know if this is the intended behaviour and I'd like someone familiar with AST rewrites to have a look here. There also may be other similar cases around.

You can observe this behaviour when trying to generate a function pointer.

1) Create new .cpp file, enter code:
int* f(char*, int) {
   f;
}
2) Select the 'f' in second line, run Refactoring -> Extract Local Variable. 

You need to have applied the patch from bug 318784 before.
Comment 2 Emanuel Graf CLA 2010-07-05 03:03:15 EDT
I'll have a look at this issue.
Comment 3 Emanuel Graf CLA 2010-07-06 05:26:16 EDT
Created attachment 173515 [details]
test case NOT producing this NPE

I tried to come up with a testcase to generate this NPE. The DeclarationWriter is able to write this Declaration. int* (*a)[2];

The problem must be in the generate declaration.
Comment 4 Tomasz Wesolowski CLA 2010-07-06 05:43:53 EDT
I assume the testbench works by parsing the test code, generating the code for the resulting AST and comparing the two?

If the NPE isn't triggered, then either that function is not called at all (unlikely) or there's an empty AST-name instead of a null AST-name after the parse? There shouldn't be one, I think. Strange. 

I'll have a look what actually happens there.

---

Let's clarify - What's the `correct` value of ASTName for a declarator without a name, in general? Null or empty ASTName object?
Comment 5 Emanuel Graf CLA 2010-07-06 06:31:49 EDT
(In reply to comment #4)
> I assume the testbench works by parsing the test code, generating the code for
> the resulting AST and comparing the two?

exactly
> 
> If the NPE isn't triggered, then either that function is not called at all
> (unlikely) or there's an empty AST-name instead of a null AST-name after the
> parse? There shouldn't be one, I think. Strange. 
> 
> I'll have a look what actually happens there.
> 
> ---
> 
> Let's clarify - What's the `correct` value of ASTName for a declarator without
> a name, in general? Null or empty ASTName object?

a declarator should return an empty name .

/**
 * This returns the name of the declarator. If this is an abstract
 * declarator, this will return an empty name.
 * 
 * @return the name of the declarator
 */
public IASTName getName();
Comment 6 Tomasz Wesolowski CLA 2010-07-06 06:47:18 EDT
(In reply to comment #5)
> a declarator should return an empty name .

I missed that. Got mislead by AST DOM view which apparently doesn't show those empty names - I used it as the example of what AST I should create.

Seems that we have a resolution - closed invalid here and a little fix for bug 318784 - I'll do it today.

(btw- am I allowed to change bug status, or is it only up to the Committers?)
Comment 7 Emanuel Graf CLA 2010-07-06 06:51:05 EDT
closing

I thinks committers and the reporter can close a bug.