Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 351029 - NPE in CPPVisitor using Generate Getters and Setters
Summary: NPE in CPPVisitor using Generate Getters and Setters
Status: RESOLVED FIXED
Alias: None
Product: CDT
Classification: Tools
Component: cdt-core (show other bugs)
Version: 8.0   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 8.0.1   Edit
Assignee: Markus Schorn CLA
QA Contact: Doug Schaefer CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-03 13:58 EDT by Marc-André Laperle CLA
Modified: 2011-07-05 09:35 EDT (History)
0 users

See Also:


Attachments
Set parent node to fix NPE (2.12 KB, patch)
2011-07-04 11:32 EDT, Marc-André Laperle CLA
malaperle: iplog-
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Marc-André Laperle CLA 2011-07-03 13:58:20 EDT
Using 8.0. If I Generate Getters and Setters in OgreMesh.h (www.ogre3d.org) I get this exception:

java.lang.NullPointerException
	at org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.CPPVisitor.getContainingScopeOrNull(CPPVisitor.java:1070)
	at org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.CPPVisitor.getContainingScope(CPPVisitor.java:1042)
	at org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.CPPSemantics.getLookupScope(CPPSemantics.java:783)
	at org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.CPPSemantics.lookup(CPPSemantics.java:896)
	at org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.CPPSemantics.resolveBinding(CPPSemantics.java:255)
	at org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.CPPVisitor.createBinding(CPPVisitor.java:227)
	at org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTName.createIntermediateBinding(CPPASTName.java:63)
	at org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTNameBase.resolveBinding(CPPASTNameBase.java:86)
	at org.eclipse.cdt.internal.core.dom.rewrite.astwriter.NameWriter.isDependentName(NameWriter.java:104)
	at org.eclipse.cdt.internal.core.dom.rewrite.astwriter.NameWriter.isDependentName(NameWriter.java:91)
	at org.eclipse.cdt.internal.core.dom.rewrite.astwriter.NameWriter.needsTemplateQualifier(NameWriter.java:82)
	at org.eclipse.cdt.internal.core.dom.rewrite.astwriter.NameWriter.writeTempalteId(NameWriter.java:61)
	at org.eclipse.cdt.internal.core.dom.rewrite.astwriter.NameWriter.writeName(NameWriter.java:45)
	at org.eclipse.cdt.internal.core.dom.rewrite.astwriter.ASTWriterVisitor.visit(ASTWriterVisitor.java:143)
	at org.eclipse.cdt.internal.core.dom.rewrite.changegenerator.ChangeGeneratorWriterVisitor.visit(ChangeGeneratorWriterVisitor.java:301)
	at org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTTemplateId.accept(CPPASTTemplateId.java:160)
	at org.eclipse.cdt.internal.core.dom.rewrite.astwriter.NameWriter.writeQualifiedName(NameWriter.java:123)
	at org.eclipse.cdt.internal.core.dom.rewrite.astwriter.NameWriter.writeName(NameWriter.java:50)
	at org.eclipse.cdt.internal.core.dom.rewrite.astwriter.ASTWriterVisitor.visit(ASTWriterVisitor.java:143)
	at org.eclipse.cdt.internal.core.dom.rewrite.changegenerator.ChangeGeneratorWriterVisitor.visit(ChangeGeneratorWriterVisitor.java:301)
	at org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTQualifiedName.accept(CPPASTQualifiedName.java:182)
	at org.eclipse.cdt.internal.core.dom.rewrite.astwriter.DeclSpecWriter.writeNamedTypeSpecifier(DeclSpecWriter.java:158)
	at org.eclipse.cdt.internal.core.dom.rewrite.astwriter.DeclSpecWriter.writeCPPDeclSpec(DeclSpecWriter.java:210)
	at org.eclipse.cdt.internal.core.dom.rewrite.astwriter.DeclSpecWriter.writeDelcSpec(DeclSpecWriter.java:73)
	at org.eclipse.cdt.internal.core.dom.rewrite.astwriter.ASTWriterVisitor.visit(ASTWriterVisitor.java:151)
	at org.eclipse.cdt.internal.core.dom.rewrite.changegenerator.ChangeGeneratorWriterVisitor.visit(ChangeGeneratorWriterVisitor.java:261)
	at org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTNamedTypeSpecifier.accept(CPPASTNamedTypeSpecifier.java:84)
	at org.eclipse.cdt.internal.core.dom.rewrite.astwriter.ASTWriterVisitor.visit(ASTWriterVisitor.java:220)
	at org.eclipse.cdt.internal.core.dom.rewrite.changegenerator.ChangeGeneratorWriterVisitor.visit(ChangeGeneratorWriterVisitor.java:309)
	at org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTParameterDeclaration.accept(CPPASTParameterDeclaration.java:90)
	at org.eclipse.cdt.internal.core.dom.rewrite.astwriter.NodeWriter.writeNodeList(NodeWriter.java:76)
	at org.eclipse.cdt.internal.core.dom.rewrite.astwriter.DeclaratorWriter.writeParameterDeclarations(DeclaratorWriter.java:169)
	at org.eclipse.cdt.internal.core.dom.rewrite.changegenerator.ModifiedASTDeclaratorWriter.writeParameterDeclarations(ModifiedASTDeclaratorWriter.java:46)
	at org.eclipse.cdt.internal.core.dom.rewrite.astwriter.DeclaratorWriter.writeParameters(DeclaratorWriter.java:120)
	at org.eclipse.cdt.internal.core.dom.rewrite.astwriter.DeclaratorWriter.writeFunctionDeclarator(DeclaratorWriter.java:103)
	at org.eclipse.cdt.internal.core.dom.rewrite.astwriter.DeclaratorWriter.writeDeclarator(DeclaratorWriter.java:61)
	at org.eclipse.cdt.internal.core.dom.rewrite.astwriter.ASTWriterVisitor.visit(ASTWriterVisitor.java:194)
	at org.eclipse.cdt.internal.core.dom.rewrite.changegenerator.ChangeGeneratorWriterVisitor.visit(ChangeGeneratorWriterVisitor.java:253)
	at org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTDeclarator.accept(CPPASTDeclarator.java:153)
	at org.eclipse.cdt.internal.core.dom.rewrite.astwriter.NodeWriter.writeNodeList(NodeWriter.java:76)
	at org.eclipse.cdt.internal.core.dom.rewrite.astwriter.DeclarationWriter.writeSimpleDeclaration(DeclarationWriter.java:335)
	at org.eclipse.cdt.internal.core.dom.rewrite.astwriter.DeclarationWriter.writeDeclaration(DeclarationWriter.java:82)
	at org.eclipse.cdt.internal.core.dom.rewrite.astwriter.DeclarationWriter.writeDeclaration(DeclarationWriter.java:69)
	at org.eclipse.cdt.internal.core.dom.rewrite.astwriter.ASTWriterVisitor.visit(ASTWriterVisitor.java:185)
	at org.eclipse.cdt.internal.core.dom.rewrite.changegenerator.ChangeGeneratorWriterVisitor.visit(ChangeGeneratorWriterVisitor.java:245)
	at org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTSimpleDeclaration.accept(CPPASTSimpleDeclaration.java:89)
	at org.eclipse.cdt.internal.core.dom.rewrite.astwriter.DeclSpecWriter.writeCompositeTypeSpecifier(DeclSpecWriter.java:268)
	at org.eclipse.cdt.internal.core.dom.rewrite.astwriter.DeclSpecWriter.writeCPPDeclSpec(DeclSpecWriter.java:202)
	at org.eclipse.cdt.internal.core.dom.rewrite.astwriter.DeclSpecWriter.writeDelcSpec(DeclSpecWriter.java:73)
	at org.eclipse.cdt.internal.core.dom.rewrite.astwriter.ASTWriterVisitor.visit(ASTWriterVisitor.java:151)
	at org.eclipse.cdt.internal.core.dom.rewrite.changegenerator.ChangeGeneratorWriterVisitor.visit(ChangeGeneratorWriterVisitor.java:261)
	at org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTCompositeTypeSpecifier.accept(CPPASTCompositeTypeSpecifier.java:155)
	at org.eclipse.cdt.internal.core.dom.rewrite.astwriter.ASTWriter.write(ASTWriter.java:86)
	at org.eclipse.cdt.internal.core.dom.rewrite.changegenerator.ChangeGenerator.synthTreatment(ChangeGenerator.java:216)
	at org.eclipse.cdt.internal.core.dom.rewrite.changegenerator.ChangeGenerator.synthTreatment(ChangeGenerator.java:208)
	at org.eclipse.cdt.internal.core.dom.rewrite.changegenerator.ChangeGenerator.visit(ChangeGenerator.java:394)
	at org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTCompositeTypeSpecifier.accept(CPPASTCompositeTypeSpecifier.java:155)
	at org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTSimpleDeclaration.accept(CPPASTSimpleDeclaration.java:96)
	at org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTNamespaceDefinition.accept(CPPASTNamespaceDefinition.java:129)
	at org.eclipse.cdt.internal.core.dom.parser.ASTTranslationUnit.accept(ASTTranslationUnit.java:279)
	at org.eclipse.cdt.internal.core.dom.rewrite.changegenerator.ChangeGenerator.generateChange(ChangeGenerator.java:111)
	at org.eclipse.cdt.internal.core.dom.rewrite.changegenerator.ChangeGenerator.generateChange(ChangeGenerator.java:104)
	at org.eclipse.cdt.internal.core.dom.rewrite.ASTRewriteAnalyzer.rewriteAST(ASTRewriteAnalyzer.java:25)
	at org.eclipse.cdt.core.dom.rewrite.ASTRewrite.rewriteAST(ASTRewrite.java:211)
	at org.eclipse.cdt.internal.ui.refactoring.ModificationCollector.createFinalChange(ModificationCollector.java:62)
	at org.eclipse.cdt.internal.ui.refactoring.CRefactoring2.createChange(CRefactoring2.java:227)
	at org.eclipse.ltk.core.refactoring.CreateChangeOperation.run(CreateChangeOperation.java:124)
	at org.eclipse.core.internal.resources.Workspace.run(Workspace.java:2344)
	at org.eclipse.ltk.internal.ui.refactoring.WorkbenchRunnableAdapter.run(WorkbenchRunnableAdapter.java:87)
	at org.eclipse.jface.operation.ModalContext$ModalContextThread.run(ModalContext.java:121)
Comment 1 Marc-André Laperle CLA 2011-07-03 16:49:28 EDT
Found an easier way to reproduce it:

#ifndef TESTNPE_H_
#define TESTNPE_H_

namespace NS {
template <class T>
class Test {
};
}

class Test{
	::NS::Test<int> test;
};

#endif
Comment 2 Markus Schorn CLA 2011-07-04 10:55:17 EDT
Although it is problematic to call name-resolution on copied AST, I have fixed the NPE.
Comment 3 Marc-André Laperle CLA 2011-07-04 11:32:06 EDT
Created attachment 199057 [details]
Set parent node to fix NPE

Hi Markus. From what I gather, the problem here is that getTranslationUnit will go through all parents until it finds a IASTranslationUnit but Generate Getters ans Setters (and other refactorings) don't set the parent node on the node to insert so this produces a NPE. Returning null will result in a CPPScopeProblem, no? So I think one potential solution would be to set the parent on the node that's being inserted by the ASTWriter (node.setParent). Since the ASTWriter has access to the parent node and the node being inserted, I think it makes sense to do it in ASTWriter instead of having the clients do it and then asserting to make sure they do. But doing that makes the AST become mix of frozen and not frozen AST. Is that why returning null is the better solution here?
Comment 4 Marc-André Laperle CLA 2011-07-05 00:23:35 EDT
Looks like this got committed to a 8_0 branch and that branch got pushed instead of cdt_8_0. Markus, can you double check your push settings?
Comment 5 Markus Schorn CLA 2011-07-05 03:41:43 EDT
(In reply to comment #4)
> Looks like this got committed to a 8_0 branch and that branch got pushed
> instead of cdt_8_0. Markus, can you double check your push settings?

Thanks for pointing this out, I have corrected my commit.
Comment 6 Markus Schorn CLA 2011-07-05 03:51:23 EDT
(In reply to comment #3)
> Created attachment 199057 [details]
> Set parent node to fix NPE
> Hi Markus. From what I gather, the problem here is that getTranslationUnit will
> go through all parents until it finds a IASTranslationUnit but Generate Getters
> ans Setters (and other refactorings) don't set the parent node on the node to
> insert so this produces a NPE. Returning null will result in a CPPScopeProblem,
> no? So I think one potential solution would be to set the parent on the node
> that's being inserted by the ASTWriter (node.setParent). Since the ASTWriter
> has access to the parent node and the node being inserted, I think it makes
> sense to do it in ASTWriter instead of having the clients do it and then
> asserting to make sure they do. But doing that makes the AST become mix of
> frozen and not frozen AST. Is that why returning null is the better solution
> here?

Setting the parent node to the original AST creates a mess, we should not do that.
For me, it does not make sense to resolve names in the copied AST. The semantics for such an operation are unclear, because the name resolution depends on how the copied AST will be used. We should probably explicitly prevent users from doing that (by throwing an exception).
In the particular case the name resolution is done (as a hack), because the ICPPASTQualifiedName does not store the information whether the template keyword has been used for one of its segments. 
--> We should probably open two new bugs: One for preventing name-resolution on
    copied AST and another one for extending ICPPASTQualifiedName.
Comment 7 Marc-André Laperle CLA 2011-07-05 09:35:13 EDT
Comment on attachment 199057 [details]
Set parent node to fix NPE

Thanks for the explanation, makes sense. I will open the two bugs.