error C2995: 'void fst::Prune(fst::MutableFst<A> *,const fst::PruneOptions<Arc,ArcFilter> &)': function template has already been defined #6
Comments
|
Let me try to repro. What is the cc file you are compiling? And VS 2015 or 2017? |
|
arciterator-class.cc and I'm using MSVC 17 (Community Edition)
I have found discussion here:
https://stackoverflow.com/questions/26753393/what-is-wrong-with-this-use-of-stdenable-if
that seems to make sense
y.
…On Mon, Nov 6, 2017 at 3:54 PM, Kirill Katsnelson ***@***.***> wrote:
Let me try to repro. What is the cc file you are compiling? And VS 2015 or
2017?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#6 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AKisXyRgFxBpWdPJuvor9uOisDw6lehfks5sz3IigaJpZM4QT2jT>
.
|
|
Yep, I am reproducing it by simply compiling this file! Let's see if I may have any ideas too. |
|
This is the weird part: It's like BUT OF COURSE it's not, that's the whole point of this template... Very strange. |
|
changing the functions like this:
```
template <class Arc, class ArcFilter>
typename std::enable_if<(Arc::Weight::Properties() & kPath) != kPath>::type
Prune(const Fst<Arc> &, MutableFst<Arc> *ofst,
const PruneOptions<Arc, ArcFilter> &) {
FSTERROR() << "Prune: Weight needs to have the path property: "
<< Arc::Weight::Type();
ofst->SetProperties(kError, kError);
}
```
worked partially -- while for some function the issue got resolved, I still
have issues with
```
1>C:\Users\jtrmal\Documents\openfst\src\include\fst/prune.h(308): error
C2995: 'std::enable_if<,void>::type fst::Prune(const fst::Fst<Arc>
&,fst::MutableFst<A> *,const fst::PruneOptions<Arc,ArcFilter> &)': function
template has already been defined
1>C:\Users\jtrmal\Documents\openfst\src\include\fst/prune.h(215): note: see
declaration of 'fst::Prune'
```
which is fairly confusing as it seems there that both the condition and the
negation of the condition are fullfiled
and with WeightCompare which I don't know how to deal with, as it returns
bool
```
1>C:\Users\jtrmal\Documents\openfst\src\include\fst/isomorphic.h(48): error
C2995: 'bool fst::internal::WeightCompare(Weight,Weight,float,bool *)':
function template has already been defined
1>C:\Users\jtrmal\Documents\openfst\src\include\fst/isomorphic.h(28): note:
see declaration of 'fst::internal::WeightCompare'
```
…On Mon, Nov 6, 2017 at 4:12 PM, Kirill Katsnelson ***@***.***> wrote:
This is the weird part:
error C2039: 'type': is not a member of 'std::enable_if<false,_Ty>'
It's like BUT OF COURSE it's not, that's the whole point of this
template... Very strange.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#6 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AKisXyNduNdLthhpUQ361B6_KLmi2z2nks5sz3YngaJpZM4QT2jT>
.
|
|
It looks like MSVC cannot digest that. Other compilers can. But the deeper issue is why are they using I believe the best way to handle this is merge the two templates like // Plus(weight, Weight::One()) == Weight::One()
template <class Arc, class ArcFilter>
void Prune(MutableFst<Arc> *fst, const PruneOptions<Arc, ArcFilter> &opts) {
using StateId = typename Arc::StateId;
using Weight = typename Arc::Weight;
using StateHeap = Heap<StateId, internal::PruneCompare<StateId, Weight>>;
if ((Weight::Properties() & kPath) != kPath) {
FSTERROR() << "Prune: Weight needs to have the path property: "
<< Arc::Weight::Type();
fst->SetProperties(kError, kError);
return;
}
auto ns = fst->NumStates();
if (ns < 1) return;
. . .I do not understand why the code with enable_if is trying to help the compiler to compile one of the two code paths, but since the expression in the Does my reasoning make sense, or am I missing something deeper? |
|
As for this error message from your second attempt. this is certainly a bug in the compiler:
|
|
yes, IIRC that was something they had before but for some reason, they
changed to enable_if semantics (perhaps saving a couple of instructions?.
I wonder if that kPath test is really constexpr because it seems that it's
both the condition and it's negation is evaluated as true, for which I
don't see any than some deep compiler reasoning reasons.
And additionally, why GCC doesn't mind? :)
I'll dig for the previous version of that file and try to get the
preivous version working...
y.
…On Mon, Nov 6, 2017 at 4:42 PM, Kirill Katsnelson ***@***.***> wrote:
It looks like MSVC cannot digest that. Other compilers can.
But the deeper issue is *why* are they using enable_if; in this case it
has no point.
I believe the best way to handle this is merge the two templates like
// Plus(weight, Weight::One()) == Weight::One()template <class Arc, class ArcFilter>void Prune(MutableFst<Arc> *fst, const PruneOptions<Arc, ArcFilter> &opts) {
using StateId = typename Arc::StateId;
using Weight = typename Arc::Weight;
using StateHeap = Heap<StateId, internal::PruneCompare<StateId, Weight>>;
if ((Weight::Properties() & kPath) != kPath) {
FSTERROR() << "Prune: Weight needs to have the path property: "
<< Arc::Weight::Type();
fst->SetProperties(kError, kError);
return;
}
auto ns = fst->NumStates();
if (ns < 1) return;
. . .
I do not understand why the code with enable_if is trying to help the
compiler to compile one of the two code paths, but since the expression in
the if() is pretty much constexpr (it should be, or it would not be
allowed in enable_if anyway), the compiler must be totally insane to
generate any code for the false branch, effectively producing the same
template expansion.
Does my reasoning make sense, or am I missing something deeper?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#6 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AKisXwSMsNTwPsS1du3otOw5ae9BV9R-ks5sz30ogaJpZM4QT2jT>
.
|
If you look closer at the error message, they both are evaluated as empty! |
|
oh, I wasn't able to parse that function prototype before (the "emptiness"
of the expression). Now I see.
y.
…On Mon, Nov 6, 2017 at 4:53 PM, Kirill Katsnelson ***@***.***> wrote:
I wonder if that kPath test is really constexpr because it seems that it's
both the condition and it's negation is evaluated as true
If you look closer at the error message, they both are evaluated as empty!
enable_if<,void>. Complete nonsense.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#6 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AKisX-ePYnHJHQ72bfojJvWueMUB7ssoks5sz3_jgaJpZM4QT2jT>
.
|
That cannot save any this way, if anything goes as it should. Essentially, a compiler always compiles The only reason I could think is they worked around some bug in a certain compiler? |
|
I am getting this error in 3 headers from compiling this one file. Should not be a big patch. When you compile full fstscript, are there more than this? |
|
I gave up when I had 5k errors, becuase the prune.h is included everywhere
-- will keep working on it.
…On Mon, Nov 6, 2017 at 5:01 PM, Kirill Katsnelson ***@***.***> wrote:
I am getting this error in 3 headers from compiling this one file. Should
not be a big patch. When you compile full fstscript, are there more than
this?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#6 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AKisXxA0adPMFyksFytzjEYUCseKehF6ks5sz4GtgaJpZM4QT2jT>
.
|
|
Let me come up with a "minimal" patch, so that it would be easy to maintain. |
|
Looks like enable_if is used only in these 3 includes (the 4th use in util.h is rather valid). |
|
OK, I'll wait.
y.
…On Mon, Nov 6, 2017 at 5:05 PM, Kirill Katsnelson ***@***.***> wrote:
Looks like enable_if is used only in these 3 includes (the 4th use in
util.h is rather valid).
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#6 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AKisX5JGs9KySVATFuRX4iFe0C1duxJeks5sz4KhgaJpZM4QT2jT>
.
|
|
There. That file compiled. Please give the change a glance over if you have time: 7b7db52?diff=unified I went for half-indent of the if's to keep most code lines intact. |
|
This is a new change in 1.6.5; here's a relevant comment from the old version: So their idea is to produce a compile-time error instead of a runtime call to |
|
I'm not sure if that's related to some of our issues, but apparently, even
MSVC 17 doesn't support SFINAE completely:
https://blogs.msdn.microsoft.com/vcblog/2017/08/11/c17-features-and-stl-fixes-in-vs-2017-15-3/
y.
…On Mon, Nov 6, 2017 at 6:02 PM, Kirill Katsnelson ***@***.***> wrote:
This is a new change in 1.6.5; here's a relevant comment from the old
version:
// TODO(kbg): Make this a compile-time static_assert once:
// 1) All weight properties are made constexpr for all weight types.
// 2) We have a pleasant way to "deregister" this operation for non-path
// semirings so an informative error message is produced. The best
// solution will probably involve some kind of SFINAE magic.
So their idea is to produce a *compile-time* error instead of a runtime
call to FSTERROR(). Now they are half-way there. Ok, let's see if the MS
compiler catches up with this. I am not holding my breath tho. :(
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#6 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AKisXzBMwSSK-RttyMBymcGAQ8vH_k6rks5sz5AdgaJpZM4QT2jT>
.
|
|
Thanks, quite an interesting post! I need to go through it carefully. A very interesting note in the SO post that you sent me, exactly this answer about templates having to be distinguishable. It never occurred to me that it was a requirement. Now, this code fragment is accepted by icl, gcc and clang, but not MSVC: #include <type_traits>
template <class T>
typename std::enable_if<T::SomeConstexprFunction() != 0>::type f() {
}
template <class T>
typename std::enable_if<T::SomeConstexprFunction() == 0>::type f() {
}MSVC returns the familiar error Now, I do not understand whether these templates are really "distinguishable" or not. Adding an extra parameter like Also, I always thought that the use of the type was essential in a template, and it appears to be the case. In a sense, I understand the logic of the compiler: If a type is not used in the template, it can be evaluated immediately for a partial specialization. Where it breaks is the evaluation depends on other template arguments (which does not work, as the compiler wants to evaluate So in the end, the following pattern works (with the type of template <class Weight, typename = void,
typename = std::enable_if<
(Weight::Properties() & kIdempotent) == kIdempotent> >
bool WeightCompare(Weight w1, Weight w2, float delta, bool *error) {
return NaturalLess<Weight>()(w1, w2);
}
template <class Weight,
typename = std::enable_if<
((Weight::Properties() & kIdempotent) != kIdempotent)> >
bool WeightCompare(Weight w1, Weight w2, float delta, bool *error) {
// No natural order; use hash
. . .But the extra C++ templates are magic anyways, and I always prefer to leave magic to the magicians. :) Do you think it makes sense to change the pattern now? Since it works now, I would rather leave it alone, and change to this pattern when (or if) they really start using SFINAE for compile-time errors. EDIT: Added missing |
|
Oh, and what they really want is probably just |
|
I have experimented with the additional parameter to the template as was
described in the SO post.
It compiled cleanly (there were some other probably unrelated issues), but
what I got worried about is that using that approach we might be hiding the
other template definition completely (as the enable_if always evaluates to
the same, empty, results), so while it compiles, it wouldn't work as
expected. But I didn't test it -- I'm, too, inclined to use the solution
with the condition in the code. Every sane compiler should be able to
optimize the extra code away.
y.
…On Mon, Nov 6, 2017 at 8:21 PM, Kirill Katsnelson ***@***.***> wrote:
Thanks, quite an interesting post! I need to go through it carefully.
A very interesting note in the SO post that you sent me, exactly this
answer <https://stackoverflow.com/a/26753794/1149924> about templates
having to be distinguishable. It never occurred to me that it was a
requirement. Now, this code fragment is accepted by icl, gcc and clang, but
not MSVC:
#include <type_traits>
template <class T>typename std::enable_if<T::SomeConstexprFunction() != 0>::type f() {
}
template <class T>typename std::enable_if<T::SomeConstexprFunction() == 0>::type f() {
}
MSVC returns the familiar error
error C2995: 'std::enable_if<,void>::type f(void)': function template has already been defined
Now, I do not understand whether these templates are really
"distinguishable" or not. Adding an extra parameter like template <class
T, typename = void> fixes the problem for MSVC.
Also, I always thought that the use of the type was essential in a
template, and it appears to be the case. In a sense, I understand the logic
of the compiler: If a type is not used in the template, it can be evaluated
immediately for a partial specialization. Where it breaks is the evaluation
depends on other template arguments (which does not work, as the compiler
wants to evaluate Weight::Properties() even though Weight is a parameter
to the same template). So we are on a shaky ground here.
So in the end, the following pattern works (with the type of enable_if on
the right side as a default argument type, which delays the evaluation
until template expansion attempts) :
template <class Weight, typename = void,
typename = std::enable_if<
(Weight::Properties() & kIdempotent) == kIdempotent> >bool WeightCompare(Weight w1, Weight w2, float delta, bool *error) {
return NaturalLess<Weight>()(w1, w2);
}
template <class Weight,
typename = std::enable_if<
((Weight::Properties & kIdempotent) != kIdempotent)> >bool WeightCompare(Weight w1, Weight w2, float delta, bool *error) {
// No natural order; use hash
. . .
But the extra typename = void is still required. And here we are looking
at the scenario where the use of enable_if is probably not helpful (there
is no potential compile-time error branch, if I got their intentions).
C++ templates are magic anyways, and I always prefer to leave magic to the
magicians. :)
Do you think it makes sense to change the pattern now? Since it works now,
I would rather leave it alone, and change to this pattern when (or if) they
really start using SFINAE for compile-time errors.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#6 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AKisX_g3zpute9Po6fRRMmgftZd9Z9dUks5sz7CogaJpZM4QT2jT>
.
|
|
yes -- that's what I didn't understand -- if they want compile-time error,
why not just static_assert?
Anyway, I'd deal with that when it comes :)
y.
…On Mon, Nov 6, 2017 at 8:26 PM, Kirill Katsnelson ***@***.***> wrote:
Oh, and what they *really* want is probably just static_assert :)
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#6 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AKisX2e5r0tRFJZsh4DtpDYKmuPY7Tysks5sz7HBgaJpZM4QT2jT>
.
|
|
Sure! So be it! :) |
|
@jtrmal, I just committed the new version 1.6.7. I've hit this problem couple times more, as they added more template tests for properties. I think I found a better workaround for this bug, implemented here. I've created a new branch Changes kinda hard to review, but here's our cumulative set of changes as it stands: original...8b6ac5d |
When compiling fstscript I'll get a bunch of errors saying:
I was playing with that and seems it's because of the enable_if magic.
After changing
to
(as http://en.cppreference.com/w/cpp/types/enable_if describes)
I end up with the error
which I think relates to "Notes" in here:
http://en.cppreference.com/w/cpp/types/enable_if
but from there, I have no idea if there is an easy way how to resolve it
The text was updated successfully, but these errors were encountered: