From 3e0853ae0c3b9703f8e6e2112829dbc6a8485c4c Mon Sep 17 00:00:00 2001 From: Nathan Ridge Date: Tue, 9 May 2017 20:58:39 -0400 Subject: [PATCH] Bug 516338 - Improve typedef preservation Besides the UX advantages of typedef preservation (such as refactorings preserving typedefs), it's important for correctness because the arguments of template aliases can be subject to SFINAE even if they don't participate in the target type. Change-Id: I4e71372553dc418d1b8c3e27bd2c0387a41a3269 --- .../parser/tests/ast2/AST2TemplateTests.java | 2 +- .../tests/IndexCPPTemplateResolutionTest.java | 53 ++++++++++++++++++- .../parser/cpp/semantics/CPPTemplates.java | 34 ++++++++++-- .../parser/cpp/semantics/SemanticUtil.java | 7 ++- 4 files changed, 88 insertions(+), 8 deletions(-) diff --git a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ast2/AST2TemplateTests.java b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ast2/AST2TemplateTests.java index b1fdf5ce787..47f4a66ef6a 100644 --- a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ast2/AST2TemplateTests.java +++ b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ast2/AST2TemplateTests.java @@ -4112,7 +4112,7 @@ public class AST2TemplateTests extends AST2CPPTestBase { public void testTypedefPreservation_380498c() throws Exception { BindingAssertionHelper ba= getAssertionHelper(); ICPPVariable s = ba.assertNonProblem("s :", "s", ICPPVariable.class); - assertTrue(s.getType() instanceof ITypedef); + assertInstance(s.getType(), ITypedef.class); assertEquals("string", ASTTypeUtil.getType(s.getType(), false)); } diff --git a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/internal/index/tests/IndexCPPTemplateResolutionTest.java b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/internal/index/tests/IndexCPPTemplateResolutionTest.java index d0026141c17..70289ca4118 100644 --- a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/internal/index/tests/IndexCPPTemplateResolutionTest.java +++ b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/internal/index/tests/IndexCPPTemplateResolutionTest.java @@ -98,7 +98,7 @@ public class IndexCPPTemplateResolutionTest extends IndexBindingResolutionTestBa } public IndexCPPTemplateResolutionTest() { - setStrategy(new ReferencedProject(true)); + setStrategy(new SinglePDOMTestStrategy(true)); } // template @@ -3089,6 +3089,55 @@ public class IndexCPPTemplateResolutionTest extends IndexBindingResolutionTestBa public void testRegression_402498() throws Exception { checkBindings(); } + + // template + // struct iterator_traits { + // typedef typename Iterator::value_type value_type; + // }; + // + // template + // struct normal; + // + // template + // struct normal { + // typedef T value_type; + // }; - + // template + // struct iterator_value { + // typedef typename iterator_traits::value_type type; + // }; + // + // template ::type> + // struct regex_compiler; + // + // typedef normal Iter; + // + // typedef regex_compiler sregex_compiler; + // + // template + // struct xpression_linker; + // + // template + // struct matchable_ex { + // typedef BidiIter iterator_type; + // typedef typename iterator_value::type char_type; + // + // void link(xpression_linker); + // }; + // + // template + // struct sub_match { + // typedef typename iterator_value::type value_type; + // operator value_type() const; + // }; + // + // void waldo(char); + // + // void foo(sub_match w) { + // waldo(w); + // } + public void testRegression_516338() throws Exception { + checkBindings(); + } } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/CPPTemplates.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/CPPTemplates.java index da964ef2b0d..75a83d7d672 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/CPPTemplates.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/CPPTemplates.java @@ -1450,9 +1450,37 @@ public class CPPTemplates { return new CPPTemplateNonTypeArgument(newEval, context.getPoint()); } - final IType orig= arg.getTypeValue(); - final IType inst= instantiateType(orig, context); - if (orig == inst) + // Which to instantiate, getOriginalTypeValue() or getTypeValue()? + // + // Using getOriginalTypeValue() is better for typedef preservation, + // and in the case of alias template instances, also necessary for + // correctness (since an alias template instance could have dependent + // arguments that don't appear in the resulting type, and these + // arguments could SFINAE out during instantiation; the popular + // "void_t" technique relies on this). + // + // However, caching of template instances is based on the normalized + // representation of arguments, which uses getTypeValue(). This, + // together with certain deficiencies in ASTTypeUtil (namely, that + // template parameters owned by different templates end up with the + // same string representation), leads to tricky bugs if we try to + // use getOriginalTypeValue() here all the time (observe, e.g., how + // IndexCPPTemplateResolutionTest.testRegression_516338 fails if we + // unconditionally use getOriginalTypeValue() here). + // + // As a compromise, we use getOriginalTypeValue() in the case where + // it's important for correctness (alias template instances), and + // getTypeValue() otherwise. + IType type; + final IType origType = arg.getOriginalTypeValue(); + if (origType instanceof ICPPAliasTemplateInstance) { + type = origType; + } else { + type = arg.getTypeValue(); + } + + final IType inst= instantiateType(type, context); + if (type == inst) return arg; return new CPPTemplateTypeArgument(inst); } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/SemanticUtil.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/SemanticUtil.java index e689d1cc156..15f78fb1fdf 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/SemanticUtil.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/SemanticUtil.java @@ -438,7 +438,8 @@ public class SemanticUtil { if (!(typedefType instanceof ITypedef)) return null; IType nestedType = type; - while (!nestedType.isSameType(((ITypedef) typedefType).getType())) { + IType typedefTarget = ((ITypedef) typedefType).getType(); + while (!nestedType.isSameType(typedefTarget)) { if (nestedType instanceof IQualifierType) { nestedType = ((IQualifierType) nestedType).getType(); } else if (nestedType instanceof IPointerType) { @@ -448,7 +449,9 @@ public class SemanticUtil { } else if (nestedType instanceof ICPPReferenceType) { nestedType = ((ICPPReferenceType) nestedType).getType(); } else { - return null; + // The typedef's target could itself be a (pointer or reference + // to, or qualified version of) a typedef. Try substituting that too. + return substituteTypedef(type, typedefTarget); } }