These implicit castings are dangerous.

Can you explain why?

The implicit casting can cause hard to find segmentation faults. Any function or method (including constructors) that takes a CTransaction (or a reference to it) as parameter will create a temporary instance using the implicit constructor from a CMutableTransaction. That instance is only in memory during the execution of the function and gets destroyed just after it. Any later access to a reference of the temporary CTransaction should raise an unauthorized memory access exception.
That would be the case in #4989 if it wasn't for

    const CTransaction txAux;
    SignatureHasher(const CMutableTransaction& txToIn, unsigned int nInIn) : txAux(txToIn), txTo(this->txAux), nIn(nInIn) { }

jtimon@dfd5c6d#diff-b81dfdd8a5bd80fe9f82b5a40c4c991eR45

The auxiliary variable and constructor could be eliminated if there was a guarantee that the Constructor will always be called with a CTransaction& and never a CMutationCTransaction that gets implicitly copied to a temporary CTransaction. That guarantee can be achieved with the "explicit" keyword.
http://stackoverflow.com/questions/121162/what-does-the-explicit-keyword-in-c-mean

Indeed, the parameter will create a temporary instance using the implicit constructor. It will have the scope of the current statement only.

Isn't the root issue then that some functions assume that a const reference that is passed has a longer validity than the function call? Not the implicit conversion itself?

Ie if a function takes a const std::string &, this could be a temporary converted from a const char*. There is nothing inherently dangerous in this except if the callee assumes that the caller will keep the reference alive instead of making a copy.

IMO if the goal is to pass a longer-living structure, I don't think references are the right thing to use. Using a pointer will avoid any implicit conversions, and documents that something potentially dangerous is going on (and you need reference counting, or ownership transfer, or at least make it clear what the scope is). Or if acceptable, just make a copy.

What you say makes sense, but creating an additional copy has some cost that I would like to save when it's not necessary. Maybe this only makes sense after #4989.
@sipa told me that he introduced that constructor without the explicit keyword to minimize the diff but leave it as something to do later.

So use a pointer, then. I think no matter what, assuming that a reference that is passed to a function outlives that function is wrong.
Implicit conversions are just a fact of life with references. You can make one constructor explicit, but this will keep biting you as long as you use C++.

I tried to do it with pointers and castings instead of implicit constructors but the tests keep failing " memory access violation"
https://github.com/jtimon/bitcoin/tree/ptrsighash

We could make all constructors with a single parameter explicit. We could even make that a style rule. If we did, when we "assume that a reference that is passed to a function outlives that function" either by mistake or due to a conscientious decision based purely on performance (and no, just like performance is not always more important than appropriate semantics, style is not always more important than performance). And although in my preferred language "explicit is better than implicit", I don't think it's fair to blame C++ in this case when it provides a specific keyword to avoid the explosions.
In any case, I will use a copy in #4989 and its continuations as suggested, maintaining that branch independent from this one (I'm mostly reffering to https://github.com/jtimon/bitcoin/tree/libscriptpreview a rewrite of #4809 on top of this and the latest changes). The penalty shouldn't be very big anyway and it can always be optimized later. After rewriting it without this, it should be easier to review as well.

But if you don't mind, I will keep this open for now to hear more feedback.

I am not blaming C++. I'm just saying that temporaries are something to be expected if you use (const) references. You can try to change the world around it, or just accept it and use a style that is not conductive to a kind of error. Even if you make sure that all constructors in bitcoin core itself are explicit, there could still be an implicit conversion through boost, or the standard library (like the std::string/const char* I mention above).

(to be clear: there are very good reasons for making constructors explicit, for example if the goal of the constructor is not to convert between types, or when the constructor does something unsafe to the object passed in. But CMutableTranscaction->CTransaction seems to me to be the kind of conversion that makes sense, and making that explicit adds a lot of verbosity to the code)

Avoiding a copy is probably always an optimization. Your rule of simply not using constructors that use a const ref and expect it to remain in memory makes a lot of sense to me.
I wasn't misunderstanding explicit, I was misunderstanding const&.
Thanks for the explanations, closing (and sorry for not having closing it earlier).