Hi,
*Context and history:*
I have found a bug in CLIF <https://github.com/google/clif>, which does not
correctly fully qualify templated names when they are nested, e.g.
::tensorfn::Nested< ::std::variant<Tensor, DeviceTensor> >
should have been:
::tensorfn::Nested<
::std::variant<::tensorflow::Tensor,::tensorfn::DeviceTensor> >
I tracked it down to
https://github.com/google/clif/blob/master/clif/backend/matcher.cc#L172
which calls
https://github.com/llvm-mirror/clang/blob/master/lib/AST/QualTypeNames.cpp
and the error is exactly at the line, but I could not really understand why.
https://github.com/llvm-mirror/clang/blob/master/lib/AST/QualTypeNames.cpp#L457
Historically, it has been added by the developers of CLIF
(including Sterling Augustine)
https://github.com/llvm/llvm-project/commit/0dd191a5c4bf27cc8a2b6033436b00f0cbdc4ce7
.
They explained to me, that they pushed this to Clang hoping it would be
used by other tools and maintained by the community, but that it kind of
failed at that, and it (i.e. QualTypeName.pp) is not much used, and not
much maintained.
I was not able to understand this file to provide a fix. On the other side,
it was easy to understand TypePrinter.cpp and PrettyPrinter.cpp, so I tried
extending it to fit my need.
*Suggestion*
As I wanted fully qualified types (it's usually more convenient for tools
generating code), to prevent some complex errors), I added ~10 lines that
add an option to prepend "::" to qualified types (see the patch).
In practice, it is still a different display at what QualTypeNames.cpp was
doing, as, for example, it displays
::tensorfn::Nested<::std*::__u*::variant<tensorflow::Tensor,
::tensorfn::DeviceTensor>>
but I was able to solve my issues. More generally, it is more verbose, as
it displays the exact underlying type, including default parameter types in
template arguments. So it's verbose, but it's correct (what is best?^^).
I am contacting you, so we can discuss:
- Whether QualTypeNames.cpp
<https://github.com/llvm-mirror/clang/blob/master/lib/AST/QualTypeNames.cpp> is
something useful for the Clang project, whether you think we should fix the
bug I have found (but I cannot, as I do not understand it), or whether we
should deprecate it, or modify the current printing mechanism
(TypePrinter.cpp and PrettyPrinter.cpp) to add more options to tune the
display in ways people may want to.
- Whether adding the CL I have attached is positive, and if yes, what
should be done in addition to that (does it need tests? Are there more
types that we may want to prepend "::" to its fully qualified name?).
Thanks!
commit 667bb3761fde65671db156cedd3fa6db37d13ee1
Author: Jean-Baptiste Lespiau <jblesp...@google.com>
Date: Tue May 12 01:07:59 2020 +0200
Add an option to fully qualify classes and structs.
diff --git a/clang/include/clang/AST/PrettyPrinter.h b/clang/include/clang/AST/PrettyPrinter.h
index 616647f4443..5ea704d9465 100644
--- a/clang/include/clang/AST/PrettyPrinter.h
+++ b/clang/include/clang/AST/PrettyPrinter.h
@@ -63,7 +63,8 @@ struct PrintingPolicy {
MSWChar(LO.MicrosoftExt && !LO.WChar), IncludeNewlines(true),
MSVCFormatting(false), ConstantsAsWritten(false),
SuppressImplicitBase(false), FullyQualifiedName(false),
- PrintCanonicalTypes(false), PrintInjectedClassNameWithArguments(true) {}
+ GlobalScopeQualifiedName(false), PrintCanonicalTypes(false),
+ PrintInjectedClassNameWithArguments(true) {}
/// Adjust this printing policy for cases where it's known that we're
/// printing C++ code (for instance, if AST dumping reaches a C++-only
@@ -241,6 +242,13 @@ struct PrintingPolicy {
/// This is the opposite of SuppressScope and thus overrules it.
unsigned FullyQualifiedName : 1;
+ // When true, class and struct types (to be expanded if needed) will be
+ // prepended with "::"
+ // Note it also requires `FullyQualifiedName` to also be set to true, as it
+ // does not make sense to prepend "::" to a non fully-qualified name.
+ // This targets generated code.
+ unsigned GlobalScopeQualifiedName : 1;
+
/// Whether to print types as written or canonically.
unsigned PrintCanonicalTypes : 1;
diff --git a/clang/lib/AST/TypePrinter.cpp b/clang/lib/AST/TypePrinter.cpp
index 6f6932e6521..29f347a8730 100644
--- a/clang/lib/AST/TypePrinter.cpp
+++ b/clang/lib/AST/TypePrinter.cpp
@@ -19,6 +19,7 @@
#include "clang/AST/DeclTemplate.h"
#include "clang/AST/Expr.h"
#include "clang/AST/NestedNameSpecifier.h"
+#include "clang/AST/PrettyPrinter.h"
#include "clang/AST/TemplateBase.h"
#include "clang/AST/TemplateName.h"
#include "clang/AST/Type.h"
@@ -40,6 +41,7 @@
#include "llvm/Support/SaveAndRestore.h"
#include "llvm/Support/raw_ostream.h"
#include <cassert>
+#include <iostream>
#include <string>
using namespace clang;
@@ -320,6 +322,10 @@ void TypePrinter::printBefore(const Type *T,Qualifiers Quals, raw_ostream &OS) {
HasEmptyPlaceHolder = false;
}
+ if (Policy.FullyQualifiedName && Policy.GlobalScopeQualifiedName &&
+ (T->isStructureOrClassType())) {
+ OS << "::";
+ }
switch (T->getTypeClass()) {
#define ABSTRACT_TYPE(CLASS, PARENT)
#define TYPE(CLASS, PARENT) case Type::CLASS: \
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
