[PATCH] D112374: [clang] WIP: Implement ElaboratedType sugaring for types written bare

Matheus Izvekov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 16 10:40:42 PST 2021 

mizvekov created this revision.
Herald added subscribers: lxfind, jdoerfert.
mizvekov updated this revision to Diff 382454.
mizvekov added a comment.
Herald added subscribers: carlosgalvezp, arphaman.
mizvekov edited the summary of this revision.
Herald added a subscriber: kristof.beyls.
mizvekov updated this revision to Diff 382467.
mizvekov updated this revision to Diff 383770.
Herald added subscribers: usaxena95, kadircet.
Herald added a reviewer: shafik.
mizvekov edited the summary of this revision.
mizvekov updated this revision to Diff 383916.
Herald added a subscriber: martong.
mizvekov updated this revision to Diff 383923.
mizvekov updated this revision to Diff 383927.
mizvekov updated this revision to Diff 385354.
mizvekov updated this revision to Diff 385355.
mizvekov updated this revision to Diff 385359.
mizvekov updated this revision to Diff 385363.
mizvekov updated this revision to Diff 385365.
mizvekov updated this revision to Diff 385373.
mizvekov updated this revision to Diff 385376.
mizvekov updated this revision to Diff 385659.
mizvekov updated this revision to Diff 385665.
mizvekov updated this revision to Diff 386181.
mizvekov updated this revision to Diff 386278.
mizvekov updated this revision to Diff 386295.
mizvekov updated this revision to Diff 386317.
mizvekov updated this revision to Diff 386333.
mizvekov marked 5 inline comments as done.
mizvekov marked an inline comment as done.
mizvekov updated this revision to Diff 387661.
mizvekov published this revision for review.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, libcxx-commits, sstefan1.
Herald added projects: clang, libc++, clang-tools-extra.
Herald added a reviewer: libc++.

.


mizvekov added a comment.

.


mizvekov added a comment.

..


mizvekov added a comment.

.


mizvekov added a comment.

.


mizvekov added a comment.

.


mizvekov added a comment.

.


mizvekov added a comment.

.


mizvekov added a comment.

.


mizvekov added a comment.

.


mizvekov added a comment.

.


mizvekov added a comment.

.


mizvekov added a comment.

.


mizvekov added a comment.

.


mizvekov added a comment.

.


mizvekov added a comment.

.


mizvekov added a comment.

.


mizvekov added a comment.

.


mizvekov added a comment.

.


mizvekov added a comment.

..


mizvekov added a comment.

- Run libcxx CI.



================
Comment at: clang/lib/Sema/SemaDecl.cpp:278
+/// Build a ParsedType for a simple-type-specifier with a nested-name-specifier.
+static ParsedType buildNestedType(Sema &S, const CXXScopeSpec *SS, QualType T,
+                                  SourceLocation NameLoc,
----------------
Maybe `buildNamedType` given that this also covers the non-nested case now?


================
Comment at: clang/lib/Sema/SemaDecl.cpp:291
+  case Type::TemplateTypeParm:
+    return ParsedType::make(T);
+  default:
----------------
It'd be useful to add a comment such as "These can never be qualified so an `ElaboratedType` node would carry no additional meaning."


================
Comment at: clang/lib/Sema/SemaDecl.cpp:291
+  case Type::TemplateTypeParm:
+    return ParsedType::make(T);
+  default:
----------------
rsmith wrote:
> It'd be useful to add a comment such as "These can never be qualified so an `ElaboratedType` node would carry no additional meaning."
Sure. But even then, in the current implementation of the type printer, adding this node would not be a no-op, it would incorrectly change meaning as we would suppress printing the scope of not just the type under the Elaborated node, but for any children of that as well. So for example a template parameter which is sugar for a CXXRecordDecl would be printed with scope suppressed.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:296
+
+  if (!SS || !SS->isNotEmpty())
+    return ParsedType::make(
----------------



================
Comment at: clang/lib/Sema/SemaDecl.cpp:545
 
-  // NOTE: avoid constructing an ElaboratedType(Loc) if this is a
-  // constructor or destructor name (in such a case, the scope specifier
-  // will be attached to the enclosing Expr or Decl node).
-  if (SS && SS->isNotEmpty() && !IsCtorOrDtorName &&
-      !isa<ObjCInterfaceDecl, UnresolvedUsingIfExistsDecl>(IIDecl)) {
-    if (WantNontrivialTypeSourceInfo) {
-      // Construct a type with type-source information.
-      TypeLocBuilder Builder;
-      Builder.pushTypeSpec(T).setNameLoc(NameLoc);
-
-      T = getElaboratedType(ETK_None, *SS, T);
-      ElaboratedTypeLoc ElabTL = Builder.push<ElaboratedTypeLoc>(T);
-      ElabTL.setElaboratedKeywordLoc(SourceLocation());
-      ElabTL.setQualifierLoc(SS->getWithLocInContext(Context));
-      return CreateParsedType(T, Builder.getTypeSourceInfo(Context, T));
-    } else {
-      T = getElaboratedType(ETK_None, *SS, T);
-    }
-  }
-
-  return ParsedType::make(T);
+  if (isa<ObjCInterfaceDecl, UnresolvedUsingIfExistsDecl>(IIDecl))
+    return ParsedType::make(T);
----------------
Can we add these to the `switch` above and unconditionally call `buildNestedType`?


================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:10913-10917
+    TypeLoc TL = TSI->getTypeLoc();
+    if (auto ETL = TL.getAs<ElaboratedTypeLoc>())
+      TL = ETL.getNamedTypeLoc();
+
+    if (auto RetTST = TL.getAs<TemplateSpecializationTypeLoc>()) {
----------------
... maybe? I don't think any of the other kinds of adjustment can ever happen here, though, so either way seems correct.


Without this patch, clang will not wrap in an ElaboratedType node types written
without a keyword and nested name qualifier, which goes against the intent of
producing an AST which retains enough details to recover how things were
written.

The default policy of the type printer is oriented towards printing the type as written,
but when the ElaboratedType node is missing, we print the type fully nested-name qualified
instead, because that is a more conservative default for cases where that node was
simply lost, such as when going through a template parameter.

With this patch, those types will be printed as
written instead, harmonizing the behavior across all forms.

Depends on D111509 <https://reviews.llvm.org/D111509>

Signed-off-by: Matheus Izvekov <mizvekov at gmail.com>


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112374

Files:
  clang-tools-extra/clang-change-namespace/ChangeNamespace.cpp
  clang-tools-extra/clang-include-fixer/find-all-symbols/FindAllSymbols.cpp
  clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
  clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
  clang-tools-extra/clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp
  clang-tools-extra/clang-tidy/misc/MisplacedConstCheck.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
  clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
  clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.h
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/unittests/ASTTests.cpp
  clang-tools-extra/clangd/unittests/DumpASTTests.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-copy-constructor-init.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison-32bits.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability-const-return-type.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability-static-accessed-through-instance-nesting-threshold.cpp
  clang-tools-extra/unittests/clang-change-namespace/ChangeNamespaceTests.cpp
  clang/bindings/python/tests/cindex/test_type.py
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/lib/ARCMigrate/ObjCMT.cpp
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTDiagnostic.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/AST/FormatString.cpp
  clang/lib/AST/PrintfFormatString.cpp
  clang/lib/AST/ScanfFormatString.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypeLoc.cpp
  clang/lib/Analysis/RetainSummaryManager.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp
  clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
  clang/test/AST/ast-dump-APValue-anon-union.cpp
  clang/test/AST/ast-dump-APValue-struct.cpp
  clang/test/AST/ast-dump-APValue-union.cpp
  clang/test/AST/ast-dump-decl.cpp
  clang/test/AST/ast-dump-expr-json.cpp
  clang/test/AST/ast-dump-expr.cpp
  clang/test/AST/ast-dump-funcs.cpp
  clang/test/AST/ast-dump-openmp-begin-declare-variant_template_3.cpp
  clang/test/AST/ast-dump-overloaded-operators.cpp
  clang/test/AST/ast-dump-records-json.cpp
  clang/test/AST/ast-dump-recovery.cpp
  clang/test/AST/ast-dump-stmt-json.cpp
  clang/test/AST/ast-dump-stmt.cpp
  clang/test/AST/ast-dump-template-decls-json.cpp
  clang/test/AST/ast-dump-temporaries-json.cpp
  clang/test/AST/coroutine-locals-cleanup.cpp
  clang/test/AST/float16.cpp
  clang/test/AST/sourceranges.cpp
  clang/test/Analysis/Inputs/expected-plists/NewDelete-path-notes.cpp.plist
  clang/test/Analysis/Inputs/expected-plists/cxx-for-range.cpp.plist
  clang/test/Analysis/Inputs/expected-plists/method-call-path-notes.cpp.plist
  clang/test/Analysis/analyzer-display-progress.cpp
  clang/test/Analysis/auto-obj-dtors-cfg-output.cpp
  clang/test/Analysis/blocks.mm
  clang/test/Analysis/bug_hash_test.cpp
  clang/test/Analysis/cast-value-state-dump.cpp
  clang/test/Analysis/cfg-rich-constructors.cpp
  clang/test/Analysis/cfg-rich-constructors.mm
  clang/test/Analysis/cfg.cpp
  clang/test/Analysis/copy-elision.cpp
  clang/test/Analysis/cxx-uninitialized-object-inheritance.cpp
  clang/test/Analysis/dump_egraph.cpp
  clang/test/Analysis/exploded-graph-rewriter/dynamic_types.cpp
  clang/test/Analysis/initializers-cfg-output.cpp
  clang/test/Analysis/inlining/Inputs/expected-plists/path-notes.cpp.plist
  clang/test/Analysis/lambdas.cpp
  clang/test/Analysis/lifetime-cfg-output.cpp
  clang/test/Analysis/malloc-sizeof.cpp
  clang/test/Analysis/memory-model.cpp
  clang/test/Analysis/missing-bind-temporary.cpp
  clang/test/Analysis/more-dtors-cfg-output.cpp
  clang/test/Analysis/scopes-cfg-output.cpp
  clang/test/Analysis/temp-obj-dtors-cfg-output.cpp
  clang/test/CXX/basic/basic.lookup/basic.lookup.argdep/p4.cpp
  clang/test/CXX/basic/basic.lookup/basic.lookup.qual/namespace.qual/p2.cpp
  clang/test/CXX/class.access/p4.cpp
  clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
  clang/test/CXX/class/class.compare/class.eq/p2.cpp
  clang/test/CXX/class/class.compare/class.spaceship/p1.cpp
  clang/test/CXX/class/class.compare/class.spaceship/p2.cpp
  clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
  clang/test/CXX/class/class.mem/p2.cpp
  clang/test/CXX/conv/conv.fctptr/p1.cpp
  clang/test/CXX/conv/conv.mem/p4.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/dtor.cpp
  clang/test/CXX/dcl.decl/dcl.decomp/p4.cpp
  clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp
  clang/test/CXX/dcl.decl/dcl.init/dcl.init.list/p3.cpp
  clang/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-0x.cpp
  clang/test/CXX/drs/dr0xx.cpp
  clang/test/CXX/drs/dr16xx.cpp
  clang/test/CXX/drs/dr17xx.cpp
  clang/test/CXX/drs/dr1xx.cpp
  clang/test/CXX/drs/dr2xx.cpp
  clang/test/CXX/drs/dr3xx.cpp
  clang/test/CXX/drs/dr4xx.cpp
  clang/test/CXX/drs/dr5xx.cpp
  clang/test/CXX/drs/dr9xx.cpp
  clang/test/CXX/except/except.spec/p1.cpp
  clang/test/CXX/expr/expr.const/p2-0x.cpp
  clang/test/CXX/expr/expr.const/p5-0x.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.lambda/p14.cpp
  clang/test/CXX/over/over.match/over.match.funcs/over.match.class.deduct/p2.cpp
  clang/test/CXX/over/over.match/over.match.funcs/over.match.copy/p1.cpp
  clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp
  clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p9-2a.cpp
  clang/test/CXX/special/class.copy/p23-cxx11.cpp
  clang/test/CXX/special/class.copy/p3-cxx11.cpp
  clang/test/CXX/special/class.inhctor/p4.cpp
  clang/test/CXX/special/class.temporary/p1.cpp
  clang/test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp
  clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
  clang/test/CodeGen/builtin-bpf-btf-type-id.c
  clang/test/CodeGen/builtins-bpf-preserve-field-info-3.c
  clang/test/CodeGenCXX/microsoft-abi-vtables-multiple-nonvirtual-inheritance-return-adjustment.cpp
  clang/test/CodeGenCXX/microsoft-abi-vtables-return-thunks.cpp
  clang/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp
  clang/test/CodeGenCXX/predefined-expr.cpp
  clang/test/CodeGenCXX/vtable-layout.cpp
  clang/test/FixIt/fixit.cpp
  clang/test/Index/annotate-context-sensitive.cpp
  clang/test/Index/comment-cplus-decls.cpp
  clang/test/Index/keep-going.cpp
  clang/test/Index/load-stmts.cpp
  clang/test/Index/opencl-types.cl
  clang/test/Index/paren-type.c
  clang/test/Index/print-type-size.cpp
  clang/test/Index/print-type.c
  clang/test/Index/print-type.cpp
  clang/test/Layout/aix-bitfield-alignment.cpp
  clang/test/Layout/aix-power-alignment-typedef.cpp
  clang/test/Layout/dump-canonical.cpp
  clang/test/Layout/ms-x86-basic-layout.cpp
  clang/test/Layout/ms-x86-misalignedarray.cpp
  clang/test/Misc/diag-line-wrapping.cpp
  clang/test/Misc/diag-template-diffing.cpp
  clang/test/Modules/namespaces.cpp
  clang/test/Modules/odr_hash-gnu.cpp
  clang/test/Modules/odr_hash.cpp
  clang/test/OpenMP/declare_mapper_ast_print.cpp
  clang/test/OpenMP/declare_reduction_ast_print.cpp
  clang/test/OpenMP/deferred-diags.cpp
  clang/test/PCH/cxx_exprs.cpp
  clang/test/Parser/cxx1z-decomposition.cpp
  clang/test/SemaCXX/MicrosoftCompatibility.cpp
  clang/test/SemaCXX/abstract.cpp
  clang/test/SemaCXX/access-base-class.cpp
  clang/test/SemaCXX/accessible-base.cpp
  clang/test/SemaCXX/aggregate-initialization.cpp
  clang/test/SemaCXX/ambig-user-defined-conversions.cpp
  clang/test/SemaCXX/atomic-type.cpp
  clang/test/SemaCXX/attr-noreturn.cpp
  clang/test/SemaCXX/builtins.cpp
  clang/test/SemaCXX/calling-conv-compat.cpp
  clang/test/SemaCXX/class-base-member-init.cpp
  clang/test/SemaCXX/class.cpp
  clang/test/SemaCXX/compound-literal.cpp
  clang/test/SemaCXX/constant-expression-cxx11.cpp
  clang/test/SemaCXX/constant-expression-cxx2a.cpp
  clang/test/SemaCXX/constant-expression.cpp
  clang/test/SemaCXX/constexpr-default-init-value-crash.cpp
  clang/test/SemaCXX/constructor-initializer.cpp
  clang/test/SemaCXX/conversion-function.cpp
  clang/test/SemaCXX/copy-initialization.cpp
  clang/test/SemaCXX/cstyle-cast.cpp
  clang/test/SemaCXX/cxx0x-class.cpp
  clang/test/SemaCXX/cxx0x-initializer-aggregates.cpp
  clang/test/SemaCXX/cxx0x-initializer-constructor.cpp
  clang/test/SemaCXX/cxx0x-initializer-references.cpp
  clang/test/SemaCXX/cxx0x-initializer-stdinitializerlist.cpp
  clang/test/SemaCXX/cxx0x-nontrivial-union.cpp
  clang/test/SemaCXX/cxx11-inheriting-ctors.cpp
  clang/test/SemaCXX/cxx17-compat.cpp
  clang/test/SemaCXX/cxx1y-contextual-conversion-tweaks.cpp
  clang/test/SemaCXX/cxx2a-destroying-delete.cpp
  clang/test/SemaCXX/cxx98-compat-flags.cpp
  clang/test/SemaCXX/cxx98-compat-pedantic.cpp
  clang/test/SemaCXX/decl-init-ref.cpp
  clang/test/SemaCXX/default-assignment-operator.cpp
  clang/test/SemaCXX/derived-to-base-ambig.cpp
  clang/test/SemaCXX/destructor.cpp
  clang/test/SemaCXX/dynamic-cast.cpp
  clang/test/SemaCXX/elaborated-type-specifier.cpp
  clang/test/SemaCXX/enum-scoped.cpp
  clang/test/SemaCXX/enum.cpp
  clang/test/SemaCXX/exceptions.cpp
  clang/test/SemaCXX/for-range-examples.cpp
  clang/test/SemaCXX/function-extern-c.cpp
  clang/test/SemaCXX/functional-cast.cpp
  clang/test/SemaCXX/ignored-reference-qualifiers-disabled.cpp
  clang/test/SemaCXX/matrix-type-operators.cpp
  clang/test/SemaCXX/member-expr.cpp
  clang/test/SemaCXX/member-init.cpp
  clang/test/SemaCXX/microsoft-cxx0x.cpp
  clang/test/SemaCXX/microsoft-dtor-lookup.cpp
  clang/test/SemaCXX/new-array-size-conv.cpp
  clang/test/SemaCXX/new-delete.cpp
  clang/test/SemaCXX/out-of-line-def-mismatch.cpp
  clang/test/SemaCXX/overload-0x.cpp
  clang/test/SemaCXX/overload-call.cpp
  clang/test/SemaCXX/overload-member-call.cpp
  clang/test/SemaCXX/overloaded-operator.cpp
  clang/test/SemaCXX/pseudo-destructors.cpp
  clang/test/SemaCXX/recovery-expr-type.cpp
  clang/test/SemaCXX/references.cpp
  clang/test/SemaCXX/static-cast.cpp
  clang/test/SemaCXX/switch.cpp
  clang/test/SemaCXX/type-traits.cpp
  clang/test/SemaCXX/undefined-internal.cpp
  clang/test/SemaCXX/underlying_type.cpp
  clang/test/SemaCXX/vector.cpp
  clang/test/SemaCXX/virtual-override.cpp
  clang/test/SemaCXX/warn-bad-memaccess.cpp
  clang/test/SemaCXX/warn-enum-compare.cpp
  clang/test/SemaCXX/warn-new-overaligned-3.cpp
  clang/test/SemaCXX/warn-new-overaligned.cpp
  clang/test/SemaCXX/warn-reinterpret-base-class.cpp
  clang/test/SemaCXX/warn-reorder-ctor-initialization.cpp
  clang/test/SemaCXX/warn-thread-safety-parsing.cpp
  clang/test/SemaObjCXX/arc-templates.mm
  clang/test/SemaObjCXX/blocks.mm
  clang/test/SemaSYCL/float128.cpp
  clang/test/SemaSYCL/int128.cpp
  clang/test/SemaTemplate/anonymous-union.cpp
  (25 more files...)

More information about the cfe-commits mailing list