[cfe-dev] ConditionalOperator::setCond is gone

[Douglas Gregor]

On Mar 9, 2011, at 2:14 AM, Olaf Krzikalla wrote:

> Hi,
> 
> Am 08.03.2011 16:59, schrieb Douglas Gregor:
>> Yes, we have been removing setXXX member functions, because mutating an AST in-place is extremely dangerous.
> Hmm that sounds really nasty. If all setXXX member functions are gone, then there are no means to efficiently manipulate an AST, aren't there?
> 
>> AST mutations look local, but it's very easy for them to have non-local effects that break the invariants of the AST.
>> 
>> Plus, immutable ASTs are far better when dealing with serialized representations of ASTs, because coping with in-place modifications to ASTs when serializing again is *extremely* complicated. See, for example, the chained PCH work in the AST reader/writer.
> Until now I hadn't any problems, but I didn't serialize manipulated ASTs but rather rewrite them only. The question is whether it is worth to sacrifce options for safety. If you take into account that I am a C++ - programmer, then you know the answer :-)


It's bad library design to write a library that can't maintain its own invariants, but that's exactly the problem with ConditionalOperator::setCond() and the many other mutators in the AST. I'm fine with having some way of mutating the AST, if it preserves invariants. 

	- Doug