[cfe-commits] [patch] Qualifiers refactor

[John McCall]

Daniel Dunbar wrote:
> I still don't see why we need the bump. All we need is a sentinel
> value to indicate that we should check for extquals, so why not only
> require this check when both restrict and volatile are present. Since
> restrict and volatile are unlikely to occur together, in practice this
> mean should avoid wasting unnecessary memory. Although 609 is also a
> very small number, so I'm not particularly concerned about it.
>
> I mentioned this to John, who thought it added more complexity than
> needed, but since I haven't actually read the patch I don't understand
> why yet.
>   

A sentinel value would make it much more expensive to merge or add
"participating" qualifiers, because you'd have to guard against accidentally
creating the sentinel.  This is something we do a lot, because pretty much
any type manipulation has to be careful to not discard qualifiers.

For essentially the same reason, we'd also have to push ASTContexts into 
a lot of
interfaces that currently don't need them --- for example, 
getDesugaredType()
would need a context, because it might merge qualifiers in a way that 
force an
ExtQuals node to be created.  Since getAs<> is currently implemented using
getDesugaredType(), that would be pretty unfortunate --- although I'm 
working on
a patch to let us desugar types for getAs<> much more efficiently.

The idea about adding a non-trivial branch to the old version is a good 
one;  I'll
let you know when I have numbers.

John.