Community
Participate
Working Groups
In the following code position the cursor on t and select Create local variable from the quick fix menu. typedef long time_t; extern time_t Now(); void test() { t = Now(); } The inserted variable declaration has long int type instead of time_t: typedef long time_t; extern time_t Now(); void test() { long int t; t = Now(); }
This happens because the method IASTExpression.getExpressionType() is aparrently designed to unwind references and typedefs. Specifically, it refers to org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.ExpressionTypes.glvalueType(IType) . Let me c/p: public static IType glvalueType(IType type) { // Reference types are removed. return SemanticUtil.getNestedType(type, TDEF | REF); } That method is used through many getExpressionType implementations. Changing that method to simply getNestedType(type, REF) would preserve typedefs and make IASTExpression.getExpressionType() as we'd expect here (preserving typedefs) but I assume that some code already relies on the current behaviour. What then? Introduce a new variant of getExpressionType into IASTExpression and all implementations, with the only difference of not calling glvalueType on the result? Something like getExactExpressionType, perhaps?
Created attachment 198011 [details] mylyn/context/zip
(In reply to comment #1) > ... > That method is used through many getExpressionType implementations. > Changing that method to simply getNestedType(type, REF) would preserve typedefs > and make IASTExpression.getExpressionType() as we'd expect here (preserving > typedefs) but I assume that some code already relies on the current behaviour. Attempting not to unwind typedefs within getExpressionType(), where this is not required, is a valid option. This may cover the most common use-cases. However, in general you cannot use the expression type as the type for the variable the extracted expression is assigned to. For example, in int x; x=1+2; the left hand side of the assignment expression 'x' is a glvalue of type int. Because you are extracting a glvalue, you'd have to use 'int&' instead of 'int'. int x; int& extracted= x; extracted= 1+2; > What then? Introduce a new variant of getExpressionType into IASTExpression and > all implementations, with the only difference of not calling glvalueType on the > result? Something like getExactExpressionType, perhaps? I do not like this idea because of the following two reasons: (1) Despite of the proposed name ('getExactExpressionType') the semantics of the method would be rather vague. The type of an expression is a notion that is defined in the c++-standard and the implementation of 'getExpressionType' follows the standard. Depending on the kind of expression it may include a conversion to a glvalue. We are still free to decide not to unwind typedefs where this yields an equivalent type. (2) Computing the expression type is difficult, it still needs to be changed (e.g. bug 299911) and I do not want to maintain two versions of it within the parser's code.
Thanks for the fast reply, Markus! I haven't yet familiarised with the notions of glvalues, prvalues etc, I'll have a read on that matter. Have I understood you correctly that you're actually suggesting to change glvalueType not to unwind typedefs? Is it safe (in terms of client code compatiblility) to perform this change? If so, should we also expose (publicly) some functionality to unwind them? SemanticUtil is internal now.
(In reply to comment #4) > Thanks for the fast reply, Markus! I haven't yet familiarised with the notions > of glvalues, prvalues etc, I'll have a read on that matter. > Have I understood you correctly that you're actually suggesting to change > glvalueType not to unwind typedefs? Is it safe (in terms of client code > compatiblility) to perform this change? If so, should we also expose (publicly) > some functionality to unwind them? SemanticUtil is internal now. I think that it is an option. It certainly requires us to review code that calls getExpressionType(). Such a change needs to be done early in the developement cycle, which increases the chance that we find all the problems it causes. To me it is an independent question, whether SemanticUtil.getNestedType(..) should be exposed to the public, and as always I am reluctant to extend our public API.
The original problem described in comment 0 has been fixed during the refactoring done in bug 299911; the expression type is now determined based on the evaluation, and implementations of ICPPEvaluation.getTypeOrFunctionSet() are careful not to remove typedefs. The concern raised by Markus in comment 3 about using the expression type as the type for the variable is still valid. However, I think that's best addressed in another bug.
(In reply to Nathan Ridge from comment #6) > The concern raised by Markus in comment 3 about using the expression type as > the type for the variable is still valid. However, I think that's best > addressed in another bug. I filed bug 484613 for this, with a variation of the testcase from comment 3. I think this bug can be closed.
Closing per comment 7.
