Bug 519361 - [C++17] Add support for 'template <auto>'
Summary: [C++17] Add support for 'template <auto>'
Status: RESOLVED FIXED
Alias: None
Product: CDT
Classification: Tools
Component: cdt-indexer (show other bugs)
Version: 9.3.0   Edit
Hardware: PC Windows 7
: P3 enhancement with 1 vote (vote)
Target Milestone: 9.5.0   Edit
Assignee: Vlad Ivanov CLA
QA Contact: Markus Schorn CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 519734
  Show dependency tree
 
Reported: 2017-07-07 03:34 EDT by 陈 林熙 CLA
Modified: 2018-05-22 01:16 EDT (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description 陈 林熙 CLA 2017-07-07 03:34:48 EDT
Eclipse since oxygenm7, CDT's index has been a lot of bugs, as far as oxygenr. 
for example:
Template<auto fr>
Class Ctest{
};
Comment 1 Nathan Ridge CLA 2017-07-07 16:34:59 EDT
(In reply to 陈 林熙 from comment #0)
> Template<auto fr>
> Class Ctest{
> };

"template <auto>" is a C++17 feature that CDT doesn't support yet.
Comment 2 陈 林熙 CLA 2017-07-08 22:58:45 EDT
But I've been working from cdt9.2 to cdt9.3 M7, and index has been working well, without any errors.
But now there are 270 syntax errors; 1454 unresolved names (0.32%).
Comment 3 Nathan Ridge CLA 2017-07-08 23:21:22 EDT
(In reply to 陈 林熙 from comment #2)
> But I've been working from cdt9.2 to cdt9.3 M7, and index has been working
> well, without any errors.
> But now there are 270 syntax errors; 1454 unresolved names (0.32%).

Can you provide example code that didn't produce errors before and does now?
Comment 4 陈 林熙 CLA 2017-07-16 00:20:42 EDT
I used one of my previous backup library tests that didn't use template <auto> to work properly. That's the template <auto> problem.
There are other mistakes, but those mistakes don't stop index from working.
By the way, does CDT have a plan to support Concept?
Comment 5 Nathan Ridge CLA 2017-07-16 13:47:16 EDT
(In reply to 陈 林熙 from comment #4)
> I used one of my previous backup library tests that didn't use template
> <auto> to work properly. That's the template <auto> problem.



> By the way, does CDT have a plan to support Concept?
Comment 6 Nathan Ridge CLA 2017-07-16 13:48:57 EDT
(Please ignore the previous comment.)

(In reply to 陈 林熙 from comment #4)
> I used one of my previous backup library tests that didn't use template
> <auto> to work properly. That's the template <auto> problem.

I don't really follow, but in any case let's use this bug to track support for the C++17 feature 'template <auto>'.

If you have issues with code that doesn't use 'template <auto>', please file a new bug.

> By the way, does CDT have a plan to support Concept?

Yes, tracked in bug 492682.
Comment 7 陈 林熙 CLA 2017-07-19 03:07:36 EDT
Thanks a lot. You'are so kind.
Comment 8 Nathan Ridge CLA 2017-07-19 11:32:35 EDT
I think I understand now what may have prompted this bug report.

As I mentioned, Eclipse has never supported 'template <auto>', so that an attempt to use a 'template <auto>' class would lead to name resolution errors.

However, in Eclipse Oxygen there was a regression, bug 519790, which caused the indexer to additionally throw an exception when encountering 'template <auto>', making the behaviour even worse than before. That has been fixed in bug 519790, so the behaviour should be back to the pre-Oxygen behaviour.

I will continue using this bug to track full support for 'template <auto>', since we will want that eventually.
Comment 9 陈 林熙 CLA 2017-08-07 10:11:33 EDT
Where can I download the latest version of CDT milestones?
Comment 10 Nathan Ridge CLA 2017-08-07 20:04:26 EDT
(In reply to 陈 林熙 from comment #9)
> Where can I download the latest version of CDT milestones?

Nightly builds can be downloaded from this update site:

http://download.eclipse.org/tools/cdt/builds/master/nightly

Milestone builds for the Photon release will presumably be found here, but we don't have any yet (and so the update site doesn't exist yet):

http://download.eclipse.org/tools/cdt/builds/photon/milestones/

If it's the fix for bug 519790 that you're after, I can backport that to the 9.3 branch so it will make it into the Oxygen.1 release in September.
Comment 11 陈 林熙 CLA 2017-08-07 21:07:05 EDT
I upgraded to the nightly version, and now it's ok.
Comment 12 Nathan Ridge CLA 2017-08-07 21:46:57 EDT
(In reply to 陈 林熙 from comment #11)
> I upgraded to the nightly version, and now it's ok.

Glad to hear it!
Comment 13 陈 林熙 CLA 2017-08-08 11:31:56 EDT
By the way, does CDT have a plan to support Class template deduction?
For example, std:: tuple t(4, 2.5);
Comment 14 Nathan Ridge CLA 2017-08-09 00:01:42 EDT
(In reply to 陈 林熙 from comment #13)
> By the way, does CDT have a plan to support Class template deduction?
> For example, std:: tuple t(4, 2.5);

I filed bug 520722 for it.

Note that CDT is still catching up on support for C++14 features, so do not expect C++17 feature support to happen very soon. (Contributions are always welcome!)
Comment 15 Vlad Ivanov CLA 2018-04-26 10:29:53 EDT
The following patch removes some of type resolution errors:

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 98574a7689..06e80546f4 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
@@ -2865,6 +2865,13 @@ public class CPPTemplates {
                        }
                        return null;
                }
+
+               if (paramType instanceof ProblemType) {
+                       if (((ProblemType) paramType).getID() == ProblemType.TYPE_CANNOT_DEDUCE_AUTO_TYPE) {
+                               return new CPPTemplateNonTypeArgument(arg.getNonTypeValue(), arg.getTypeOfNonTypeValue());
+                       }
+               }
+
                Cost cost = Conversions.checkImplicitConversionSequence(p, a, LVALUE, UDCMode.FORBIDDEN,
                                Context.ORDINARY);
                if (cost == null || !cost.converts()) {

However, this still doesn't fix "using" binding so tests on variable type will fail with this code.
Comment 16 Nathan Ridge CLA 2018-04-26 13:18:27 EDT
Thank you for the patch, Vlad.

Even though it's a small patch, could you please submit it via Gerrit?

Also, could you give an example of a type resolution error that is avoided with this change?
Comment 17 Vlad Ivanov CLA 2018-04-26 14:03:44 EDT
Hello Nathan,

I'll try to submit it tomorrow. As for type definition errors, here's a simple example:

template<auto F>
struct A {
    using T = decltype(F);
}

void test() {

}

using Type = A<&test>::T;
Comment 18 Eclipse Genie CLA 2018-04-27 02:24:42 EDT
New Gerrit change created: https://git.eclipse.org/r/121828
Comment 20 Nathan Ridge CLA 2018-05-03 23:39:39 EDT
Thanks, Vlad, I have merged your partial fix.

It would also be useful to give an example of code that still gives an error with this change.
Comment 21 Vlad Ivanov CLA 2018-05-04 03:41:45 EDT
Thanks Nathan,

here's an reduced example code that gives errors:

template<typename T>
struct Helper {};

void test() {}

template<auto F>
struct C {
	using T = decltype(F);
	using H = Helper<T>;
};

From my limited knowledge of CDT, it may make sense
to create a special "lazily evaluated" type which
only would be resolved when a template is instantiated.

Ideally for Ctrl-click user could be presented with
a list of possible options to go to.
Comment 22 Nathan Ridge CLA 2018-05-04 11:17:10 EDT
Thanks.

(In reply to Vlad Ivanov from comment #21)
> From my limited knowledge of CDT, it may make sense
> to create a special "lazily evaluated" type which
> only would be resolved when a template is instantiated.

When I implemented C++14 return type deduction, I introduced CPPPlaceholderType to represent a use of 'auto' when the deduced type was not available yet. (For return type deduction that can be necessary because you can forward-declare a function like 'auto foo();', and provide the definition (which provides the deduced return type) later.)

We could reuse CPPPlaceholderType for this purpose. When CPPVisitor.createAutoType() creates the type for the 'auto', it could create a CPPPlaceholderType instead of ProblemType.CANNOT_DEDUCE_AUTO_TYPE. The code you added here could check for CPPPlaceholderType instead.

I'm not sure whether that's enough to solve the problem, but it would at least be a first step.
Comment 23 Eclipse Genie CLA 2018-05-07 09:50:24 EDT
New Gerrit change created: https://git.eclipse.org/r/122263
Comment 24 Vlad Ivanov CLA 2018-05-07 09:54:04 EDT
I have pushed a patch which fixes this particular testcase.

Next one is as follows:

void test() {}

template<typename T>
struct Y {
	static void call() {}
};

template<auto F>
struct C {
	using T = decltype(F);
	using H = Y<T>;
};

void foo() {
	using T = C<&test>::H;
	H::call();
}
Comment 25 Vlad Ivanov CLA 2018-05-07 10:25:24 EDT
Please disregard the previous comment — it has a mistake, it should be T::call(). Sorry about that. Actual issue I have with a different code could as well be unrelated.
Comment 27 Vlad Ivanov CLA 2018-05-08 04:07:59 EDT
Nathan, thanks for merging.

Here's an updated and reduced testcase that still produces an error:

template<typename Type, Type v>
struct helper {
	static void call() {}
};

template<auto F>
class call_helper {
	using functor_t = decltype(F);
public:
	using type = helper<functor_t, F>;
};

struct Something {
	void foo() {}
};

void test() {
	using A = call_helper<&Something::foo>::type;
	A::call();
}
Comment 28 Nathan Ridge CLA 2018-05-08 19:16:31 EDT
I had a look at the testcase from comment 27.

The issue here is that this testcase triggers resolution of the non-type template parameter's type prior to instantiation, and stashes that type away (in call_helper<F>::functor_t). The way we've implemented it, this is going to be a CPPPlaceholderType. However, when instantiating that type with [F = &Something::foo], we can't do anything useful with the CPPPlaceholderType.

That suggests that we need to use a type that somehow refers to or wraps the non-type template parameter, so that we can instantiate the type correctly (by insantiating the wrapped non-type template parameter). We could use a TypeOfDependentExpression for this purpose, wrapping an EvalBinding pointing to the non-type template parameter.

Here's one way I can envision this working:

  - In CPPVisitor.createAutoType(), we still need to
    create a CPPPlaceholderType, because we don't have
    the template parameter binding available there.

  - In CPPTemplateNonTypeParameter.getType(), we can
    check for a CPPPlaceholderType, and replace it with
    a TypeOfDependentExpression.

  - As a result, by the time we get to CPPTemplates.
    convertNonTypeTemplateArgument(), the parameter type
    will be a TypeOfDependentExpression, so we need to
    check for that instead of CPPPlaceholderType.

      - Since TypeOfDependentExpression is also used
        for other purposes, we need to add a flag like
        "isForTemplateAuto" to it to discriminate.
Comment 29 Vlad Ivanov CLA 2018-05-10 04:19:32 EDT
Should a new kind of evaluation, derived from CPPDependentEvaluation, be implemented so it can be used with TypeOfDependentExpression?
Comment 30 Nathan Ridge CLA 2018-05-10 12:45:39 EDT
(In reply to Vlad Ivanov from comment #29)
> Should a new kind of evaluation, derived from CPPDependentEvaluation, be
> implemented so it can be used with TypeOfDependentExpression?

Nope, we can use EvalBinding as mentioned. We can use the (IBinding,IType,IASTNode) constructor of EvalBinding, passing in the template parameter for the binding, null for the type, and any IASTNode available in the CPPTemplateNonTypeParameter (like getPrimaryDeclaration(), or the 'parent' variable in getType()).
Comment 31 Eclipse Genie CLA 2018-05-11 02:58:46 EDT
New Gerrit change created: https://git.eclipse.org/r/122458
Comment 33 Nathan Ridge CLA 2018-05-21 12:55:06 EDT
Vlad, I would be interested to know whether the third patch solves all of your errors, or if there are still some remaining.
Comment 34 Vlad Ivanov CLA 2018-05-21 13:50:11 EDT
Hello Nathan,

I couldn't come up with any example to produce more errors related to this feature. With my code base there are no such errors anymore.
Comment 35 Nathan Ridge CLA 2018-05-22 01:16:06 EDT
That's great to hear! In that case, I will mark this bug as fixed, and assign it to you.

If anyone spots additional problems with template <auto>, we can reopen this bug, or file a new one for them.