schroedersi added a comment.
In https://reviews.llvm.org/D30946#740567, @bkramer wrote:
> Also the mutable state in PrintingPolicy is really really ugly, is there no
> better way for this? :(
Thanks for your comment :-)
I assume with mutable state you mean `PrintingPolicy::TemporarySuppressScope`?
I do not really like it either. But other states of the policy are also mutated
during the printing process (e.g. SuppressScope, SuppressTagKeyword and
SuppressSpecifiers inside the RAII helpers).
A little background: It is necessary that scope printing can be temporarily
suppressed without changing the actual scope printing kind setting. For
example, in the case of an ElaboratedType, it is necessary that the underlying
type is printed without the outer scope (because it has already been printed
based on how it was written in the source and based on the scope printing kind
setting). However, the internal type should be printed according to the scope
printing kind setting. Another example are types inside a nested name
specifier. Again, the outer scope can not be printed. However, inner scopes
(e.g. for template arguments) must be printed according to the scope printing
kind setting.
For this temporal suppression, the information about whether a scope has
already been suppressed and thus the scope printing kind setting must be used,
must be shared over all member functions involved in the printing. These member
functions are the TypePrinter member functions and
`TemplateName::print`, `NestedNameSpecifier::print`, and
`NamedDecl::printQualifiedName`. Currently, the only object shared across all
these member functions is the PrintingPolicy. That is why I added the state to
the PrintingPolicy.
As an alternative to the current solution, the above-mentioned member functions
could each be extended by a "PrintingContext" argument, which then contains the
variable states.
And a bit of clarification: Without the patch, even with `SuppressScope = true`
it is not possible to suppress all scopes, and inner types are printed with a
scope most of the time (which seems to lead to the desired effect for most
users). For example, printing the type of `A` in `namespace N{class B{};
template<typename T> class C{}; C<::N::B> A;}` with `SuppressScope = true`
results in `C< ::N::B>` which still includes scopes. That is, there is already
some kind of temporary suppress scope functionality, but it is called
SuppressScope. The patch only expands this functionality to full scopes (see
summary above), makes the behavior more consistent, and ensures that the
settings do what - at least I - would expect from their name and documentation.
I hope this explains my decision. Either way, I look forward to further
comments :-).
https://reviews.llvm.org/D30946
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
