[llvm-branch-commits] [clang] [Serialization] No transitive identifier change (PR #92085)
https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/92085 >From a2fb7f50161932a9557a22a4ba23f827e80a4d6b Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Tue, 14 May 2024 15:33:12 +0800 Subject: [PATCH] [Serialization] No transitive identifier change --- .../clang/Lex/ExternalPreprocessorSource.h| 54 - clang/include/clang/Lex/HeaderSearch.h| 12 +- .../include/clang/Serialization/ASTBitCodes.h | 2 +- clang/include/clang/Serialization/ASTReader.h | 19 ++- .../include/clang/Serialization/ModuleFile.h | 3 - clang/lib/Lex/HeaderSearch.cpp| 33 +++--- clang/lib/Serialization/ASTReader.cpp | 98 clang/lib/Serialization/ASTWriter.cpp | 63 ++ clang/lib/Serialization/GlobalModuleIndex.cpp | 3 +- clang/lib/Serialization/ModuleFile.cpp| 1 - .../no-transitive-identifier-change.cppm | 110 ++ 11 files changed, 286 insertions(+), 112 deletions(-) create mode 100644 clang/test/Modules/no-transitive-identifier-change.cppm diff --git a/clang/include/clang/Lex/ExternalPreprocessorSource.h b/clang/include/clang/Lex/ExternalPreprocessorSource.h index 6775841860373..48429948dbffe 100644 --- a/clang/include/clang/Lex/ExternalPreprocessorSource.h +++ b/clang/include/clang/Lex/ExternalPreprocessorSource.h @@ -36,12 +36,64 @@ class ExternalPreprocessorSource { /// Return the identifier associated with the given ID number. /// /// The ID 0 is associated with the NULL identifier. - virtual IdentifierInfo *GetIdentifier(unsigned ID) = 0; + virtual IdentifierInfo *GetIdentifier(uint64_t ID) = 0; /// Map a module ID to a module. virtual Module *getModule(unsigned ModuleID) = 0; }; +// Either a pointer to an IdentifierInfo of the controlling macro or the ID +// number of the controlling macro. +class LazyIdentifierInfoPtr { + // If the low bit is clear, a pointer to the IdentifierInfo. If the low + // bit is set, the upper 63 bits are the ID number. + mutable uint64_t Ptr = 0; + +public: + LazyIdentifierInfoPtr() = default; + + explicit LazyIdentifierInfoPtr(const IdentifierInfo *Ptr) + : Ptr(reinterpret_cast(Ptr)) {} + + explicit LazyIdentifierInfoPtr(uint64_t ID) : Ptr((ID << 1) | 0x01) { +assert((ID << 1 >> 1) == ID && "ID must require < 63 bits"); +if (ID == 0) + Ptr = 0; + } + + LazyIdentifierInfoPtr &operator=(const IdentifierInfo *Ptr) { +this->Ptr = reinterpret_cast(Ptr); +return *this; + } + + LazyIdentifierInfoPtr &operator=(uint64_t ID) { +assert((ID << 1 >> 1) == ID && "IDs must require < 63 bits"); +if (ID == 0) + Ptr = 0; +else + Ptr = (ID << 1) | 0x01; + +return *this; + } + + /// Whether this pointer is non-NULL. + /// + /// This operation does not require the AST node to be deserialized. + bool isValid() const { return Ptr != 0; } + + /// Whether this pointer is currently stored as ID. + bool isID() const { return Ptr & 0x01; } + + IdentifierInfo *getPtr() const { +assert(!isID()); +return reinterpret_cast(Ptr); + } + + uint64_t getID() const { +assert(isID()); +return Ptr >> 1; + } +}; } #endif diff --git a/clang/include/clang/Lex/HeaderSearch.h b/clang/include/clang/Lex/HeaderSearch.h index 5ac634d4e..65700b8f9dc11 100644 --- a/clang/include/clang/Lex/HeaderSearch.h +++ b/clang/include/clang/Lex/HeaderSearch.h @@ -16,6 +16,7 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Lex/DirectoryLookup.h" +#include "clang/Lex/ExternalPreprocessorSource.h" #include "clang/Lex/HeaderMap.h" #include "clang/Lex/ModuleMap.h" #include "llvm/ADT/ArrayRef.h" @@ -119,13 +120,6 @@ struct HeaderFileInfo { LLVM_PREFERRED_TYPE(bool) unsigned IsValid : 1; - /// The ID number of the controlling macro. - /// - /// This ID number will be non-zero when there is a controlling - /// macro whose IdentifierInfo may not yet have been loaded from - /// external storage. - unsigned ControllingMacroID = 0; - /// If this file has a \#ifndef XXX (or equivalent) guard that /// protects the entire contents of the file, this is the identifier /// for the macro that controls whether or not it has any effect. @@ -134,7 +128,7 @@ struct HeaderFileInfo { /// the controlling macro of this header, since /// getControllingMacro() is able to load a controlling macro from /// external storage. - const IdentifierInfo *ControllingMacro = nullptr; + LazyIdentifierInfoPtr LazyControllingMacro; /// If this header came from a framework include, this is the name /// of the framework. @@ -580,7 +574,7 @@ class HeaderSearch { /// no-op \#includes. void SetFileControllingMacro(FileEntryRef File, const IdentifierInfo *ControllingMacro) { -getFileInfo(File).ControllingMacro = ControllingMacro; +getFileInfo(File).LazyControllingMacro = ControllingMacro; }
[llvm-branch-commits] [clang] [Serialization] No transitive identifier change (PR #92085)
@@ -124,7 +124,7 @@ struct HeaderFileInfo { /// This ID number will be non-zero when there is a controlling /// macro whose IdentifierInfo may not yet have been loaded from /// external storage. - unsigned ControllingMacroID = 0; + uint64_t ControllingMacroID = 0; ChuanqiXu9 wrote: Done by adding `LazyIdentifierInfoPtr` to `ExternalPreprocessorSource.h`. https://github.com/llvm/llvm-project/pull/92085 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [Serialization] No transitive identifier change (PR #92085)
@@ -918,7 +918,7 @@ ASTSelectorLookupTrait::ReadKey(const unsigned char* d, unsigned) { SelectorTable &SelTable = Reader.getContext().Selectors; unsigned N = endian::readNext(d); const IdentifierInfo *FirstII = Reader.getLocalIdentifier( - F, endian::readNext(d)); + F, endian::readNext(d)); ChuanqiXu9 wrote: If `IdentifierID` is not integral type, the code can't compile (`endian::readNext` won't accept that). So I feel it might not be so useful. https://github.com/llvm/llvm-project/pull/92085 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [Serialization] No transitive identifier change (PR #92085)
@@ -3896,7 +3903,7 @@ void ASTWriter::WriteIdentifierTable(Preprocessor &PP, // Write out identifiers if either the ID is local or the identifier has // changed since it was loaded. - if (ID >= FirstIdentID || !Chain || !II->isFromAST() || + if (isLocalIdentifierID(ID) || !Chain || !II->isFromAST() || ChuanqiXu9 wrote: Maybe it is because the name `FirstIdentID` is confusing. The proper name may be `FirstLocalIdentID`. Previously, before this patch, `FirstIdentID` will be `Number of identifier ID` + 1. This is the reason why the encoding of Identifier IDs may be affected by imported modules. This is also the major motivation of the patch. So all the ID bigger or equal to FirstIdentID must be the ID for local identifiers previously. But it won't be the case we make this patch. https://github.com/llvm/llvm-project/pull/92085 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [libcxx] [clang] Preserve Qualifiers and type sugar in TemplateNames (PR #93433)
@@ -9304,7 +9299,8 @@ TemplateName ASTContext::getAssumedTemplateName(DeclarationName Name) const { TemplateName ASTContext::getQualifiedTemplateName(NestedNameSpecifier *NNS, bool TemplateKeyword, TemplateName Template) const { - assert(NNS && "Missing nested-name-specifier in qualified template name"); ilya-biryukov wrote: Does that mean we will start creating `QualifiedTemplateName` with a nullptr qualifier? That's a big change in contract and there is quite a bit of code that does not check for null when accessing this member. Could you explain a bit why we need this change? What are we trying to represent? Are there any alternative AST nodes we could use or even add? https://github.com/llvm/llvm-project/pull/93433 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [libcxx] [clang] Preserve Qualifiers and type sugar in TemplateNames (PR #93433)
@@ -804,6 +804,8 @@ Bug Fixes to AST Handling - Clang now properly preserves ``FoundDecls`` within a ``ConceptReference``. (#GH82628) - The presence of the ``typename`` keyword is now stored in ``TemplateTemplateParmDecl``. - Fixed malformed AST generated for anonymous union access in templates. (#GH90842) +- Improved preservation of qualifiers and sugar in TemplateNames, including ilya-biryukov wrote: NIT: maybe say "template names". Alternatively, put TemplateName into double backticks if you are referring to the particular class. https://github.com/llvm/llvm-project/pull/93433 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [libcxx] [clang] Preserve Qualifiers and type sugar in TemplateNames (PR #93433)
https://github.com/ilya-biryukov commented: The change makes handling of template names more uniform, which overall looks like a great change. However, we are changing the contract of `QualifiedTemplateName` (it can have a null qualifier now) and this seems both counter-intuitive from the API design perspective and potentially adding bugs in various places where the qualifier is used without checking for null. I am not necessarily saying that's wrong, but maybe we need a different name for the class or a different node for the case where there is no qualifier. We could discuss this in the relevant comment thread. https://github.com/llvm/llvm-project/pull/93433 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [libcxx] [clang] Preserve Qualifiers and type sugar in TemplateNames (PR #93433)
https://github.com/ilya-biryukov edited https://github.com/llvm/llvm-project/pull/93433 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [libcxx] [clang] Preserve Qualifiers and type sugar in TemplateNames (PR #93433)
https://github.com/mizvekov updated https://github.com/llvm/llvm-project/pull/93433 >From 40e3c0fd00a9a3327123d43c6ad6447a14b4e543 Mon Sep 17 00:00:00 2001 From: Matheus Izvekov Date: Sat, 25 May 2024 13:57:39 -0300 Subject: [PATCH] [clang] Preserve Qualifiers and type sugar in TemplateNames This patch improves the preservation of qualifiers and loss of type sugar in TemplateNames. This problem is analogous to https://reviews.llvm.org/D112374 and this patch takes a very similar approach to that patch, except the impact here is much lesser. When a TemplateName was written bare, without qualifications, we wouldn't produce a QualifiedTemplate which could be used to disambiguate it from a Canonical TemplateName. This had effects in the TemplateName printer, which had workarounds to deal with this, and wouldn't print the TemplateName as-written in most situations. There are also some related fixes to help preserve this type sugar along the way into diagnostics, so that this patch can be properly tested. - Fix dropping the template keyword. - Fix type deduction to preserve sugar in TST TemplateNames. --- clang/docs/ReleaseNotes.rst | 2 + clang/include/clang/AST/TemplateName.h| 2 +- clang/include/clang/Sema/Sema.h | 3 + clang/lib/AST/ASTContext.cpp | 16 ++--- clang/lib/AST/DeclTemplate.cpp| 7 +- clang/lib/AST/ODRHash.cpp | 9 ++- clang/lib/AST/TemplateBase.cpp| 2 +- clang/lib/AST/TemplateName.cpp| 64 +-- clang/lib/AST/TextNodeDumper.cpp | 4 +- clang/lib/AST/Type.cpp| 3 +- clang/lib/AST/TypePrinter.cpp | 4 +- clang/lib/Sema/SemaDecl.cpp | 15 ++--- clang/lib/Sema/SemaDeclCXX.cpp| 12 ++-- clang/lib/Sema/SemaExpr.cpp | 4 +- clang/lib/Sema/SemaExprMember.cpp | 3 +- clang/lib/Sema/SemaTemplate.cpp | 25 +--- clang/lib/Sema/SemaTemplateDeduction.cpp | 62 +- clang/lib/Sema/SemaType.cpp | 14 ++-- clang/lib/Sema/TreeTransform.h| 8 +-- clang/test/AST/ast-dump-ctad-alias.cpp| 6 +- clang/test/AST/ast-dump-decl.cpp | 8 +-- clang/test/AST/ast-dump-expr.cpp | 2 +- clang/test/AST/ast-dump-template-decls.cpp| 6 +- clang/test/AST/ast-dump-template-name.cpp | 4 +- clang/test/AST/ast-dump-using-template.cpp| 6 +- clang/test/CXX/drs/cwg1xx.cpp | 4 +- .../over.match.oper/p3-2a.cpp | 4 +- .../temp.deduct/temp.deduct.type/p9-0x.cpp| 4 +- clang/test/Index/print-type.cpp | 2 +- clang/test/OpenMP/declare_mapper_messages.cpp | 2 +- .../Parser/cxx-template-template-recovery.cpp | 4 +- .../cxx1y-variable-templates_in_class.cpp | 10 +-- clang/test/SemaTemplate/cwg2398.cpp | 2 +- .../instantiate-requires-expr.cpp | 4 +- .../nested-implicit-deduction-guides.cpp | 2 +- clang/unittests/AST/TemplateNameTest.cpp | 40 ++-- .../map/map.cons/deduct.verify.cpp| 24 +++ .../multimap/multimap.cons/deduct.verify.cpp | 22 +++ .../multiset/multiset.cons/deduct.verify.cpp | 10 +-- .../set/set.cons/deduct.verify.cpp| 10 +-- .../priqueue.cons/deduct.verify.cpp | 10 +-- .../queue/queue.cons/deduct.verify.cpp| 6 +- .../stack/stack.cons/deduct.verify.cpp| 6 +- .../array/array.cons/deduct.verify.cpp| 2 +- .../deque/deque.cons/deduct.verify.cpp| 2 +- .../forwardlist.cons/deduct.verify.cpp| 2 +- .../list/list.cons/deduct.verify.cpp | 2 +- .../vector/vector.cons/deduct.verify.cpp | 2 +- .../unord.map.cnstr/deduct.verify.cpp | 16 ++--- .../unord.multimap.cnstr/deduct.verify.cpp| 16 ++--- .../unord.multiset.cnstr/deduct.verify.cpp| 16 ++--- .../unord.set.cnstr/deduct.verify.cpp | 16 ++--- .../range.adaptors/range.join/ctad.verify.cpp | 2 +- .../re.regex.construct/deduct.verify.cpp | 4 +- .../func.wrap.func.con/deduct_F.verify.cpp| 6 +- .../optional.object.ctor/deduct.verify.cpp| 2 +- 56 files changed, 310 insertions(+), 235 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index f7562ce74f4ed..af8188b7ca3b1 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -804,6 +804,8 @@ Bug Fixes to AST Handling - Clang now properly preserves ``FoundDecls`` within a ``ConceptReference``. (#GH82628) - The presence of the ``typename`` keyword is now stored in ``TemplateTemplateParmDecl``. - Fixed malformed AST generated for anonymous union access in templates. (#GH90842) +- Improved preservation of qualifiers and sugar in TemplateNames, including + template keyword. Miscellaneous Bug Fixes ^^^ diff --
[llvm-branch-commits] [clang] [clang] fix printing of canonical template template parameters take 2 (PR #93448)
https://github.com/mizvekov created https://github.com/llvm/llvm-project/pull/93448 Since they can also occur as the template name of template specializations, handle them from TemplateName printing instead of TemplateArgument. >From 7876afed3dec889805b2947e61ca10953a5a7456 Mon Sep 17 00:00:00 2001 From: Matheus Izvekov Date: Mon, 27 May 2024 05:51:18 -0300 Subject: [PATCH] [clang] fix printing of canonical template template parameters take 2 Since they can also occur as the template name of template specializations, handle them from TemplateName printing instead of TemplateArgument. --- clang/lib/AST/TemplateBase.cpp | 11 +-- clang/lib/AST/TemplateName.cpp | 14 ++ clang/test/SemaTemplate/deduction-guide.cpp | 10 +- 3 files changed, 20 insertions(+), 15 deletions(-) diff --git a/clang/lib/AST/TemplateBase.cpp b/clang/lib/AST/TemplateBase.cpp index 4d4991d8c38b5..bb0820f4177e4 100644 --- a/clang/lib/AST/TemplateBase.cpp +++ b/clang/lib/AST/TemplateBase.cpp @@ -539,16 +539,7 @@ void TemplateArgument::print(const PrintingPolicy &Policy, raw_ostream &Out, break; case Template: { -TemplateName TN = getAsTemplate(); -if (const auto *TD = TN.getAsTemplateDecl(); -TD && TD->getDeclName().isEmpty()) { - assert(isa(TD) && - "Unexpected anonymous template"); - const auto *TTP = cast(TD); - Out << "template-parameter-" << TTP->getDepth() << "-" << TTP->getIndex(); -} else { - TN.print(Out, Policy); -} +getAsTemplate().print(Out, Policy); break; } diff --git a/clang/lib/AST/TemplateName.cpp b/clang/lib/AST/TemplateName.cpp index 3aae998eceeb0..1ce31f09f2a6a 100644 --- a/clang/lib/AST/TemplateName.cpp +++ b/clang/lib/AST/TemplateName.cpp @@ -292,6 +292,14 @@ void TemplateName::Profile(llvm::FoldingSetNodeID &ID) { void TemplateName::print(raw_ostream &OS, const PrintingPolicy &Policy, Qualified Qual) const { + auto handleCanonicalTTP = [](TemplateDecl *TD, raw_ostream &OS) { +if (TemplateTemplateParmDecl *TTP = dyn_cast(TD); +TTP && TTP->getIdentifier() == nullptr) { + OS << "template-parameter-" << TTP->getDepth() << "-" << TTP->getIndex(); + return true; +} +return false; + }; if (NameKind Kind = getKind(); Kind == TemplateName::Template || Kind == TemplateName::UsingTemplate) { // After `namespace ns { using std::vector }`, what is the fully-qualified @@ -304,6 +312,8 @@ void TemplateName::print(raw_ostream &OS, const PrintingPolicy &Policy, // names more often than to export them, thus using the original name is // most useful in this case. TemplateDecl *Template = getAsTemplateDecl(); +if (handleCanonicalTTP(Template, OS)) + return; if (Qual == Qualified::None) OS << *Template; else @@ -320,6 +330,10 @@ void TemplateName::print(raw_ostream &OS, const PrintingPolicy &Policy, Underlying.getKind() == TemplateName::UsingTemplate); TemplateDecl *UTD = Underlying.getAsTemplateDecl(); + +if (handleCanonicalTTP(UTD, OS)) + return; + if (IdentifierInfo *II = UTD->getIdentifier(); Policy.CleanUglifiedParameters && II && isa(UTD)) diff --git a/clang/test/SemaTemplate/deduction-guide.cpp b/clang/test/SemaTemplate/deduction-guide.cpp index 96b4cd9622a24..100b580fe9f02 100644 --- a/clang/test/SemaTemplate/deduction-guide.cpp +++ b/clang/test/SemaTemplate/deduction-guide.cpp @@ -315,19 +315,19 @@ namespace TTP { // CHECK-NEXT: |-TemplateTypeParmDecl {{.+}} class depth 0 index 0 T{{$}} // CHECK-NEXT: |-TemplateTemplateParmDecl {{.+}} depth 0 index 1 TT{{$}} // CHECK-NEXT: | `-TemplateTypeParmDecl {{.+}} class depth 1 index 0{{$}} -// CHECK-NEXT: |-CXXDeductionGuideDecl {{.+}} 'auto () -> B'{{$}} -// CHECK-NEXT: | `-ParmVarDecl {{.+}} ''{{$}} +// CHECK-NEXT: |-CXXDeductionGuideDecl {{.+}} 'auto (template-parameter-0-1) -> B'{{$}} +// CHECK-NEXT: | `-ParmVarDecl {{.+}} 'template-parameter-0-1'{{$}} // CHECK-NEXT: `-CXXDeductionGuideDecl {{.+}} 'auto (A) -> TTP::B' // CHECK-NEXT:|-TemplateArgument type 'int' // CHECK-NEXT:| `-BuiltinType {{.+}} 'int'{{$}} // CHECK-NEXT:|-TemplateArgument template 'TTP::A'{{$}} // CHECK-NEXT:| `-ClassTemplateDecl {{.+}} A{{$}} // CHECK-NEXT:`-ParmVarDecl {{.+}} 'A':'TTP::A'{{$}} -// CHECK-NEXT: FunctionProtoType {{.+}} 'auto () -> B' dependent trailing_return cdecl{{$}} +// CHECK-NEXT: FunctionProtoType {{.+}} 'auto (template-parameter-0-1) -> B' dependent trailing_return cdecl{{$}} // CHECK-NEXT: |-InjectedClassNameType {{.+}} 'B' dependent{{$}} // CHECK-NEXT: | `-CXXRecord {{.+}} 'B'{{$}} -// CHECK-NEXT: `-ElaboratedType {{.+}} '' sugar dependent{{$}} -// CHECK-NEXT:`-TemplateSpecializationType {{.+}} '' dependent {{$}} +// CHECK-NEXT: `-ElaboratedType {{.+}} 'template-parameter-0-1' sugar dependent{{$}} +// CHECK-NEXT:`-Template
[llvm-branch-commits] [clang] [clang] fix printing of canonical template template parameters take 2 (PR #93448)
llvmbot wrote: @llvm/pr-subscribers-clang Author: Matheus Izvekov (mizvekov) Changes Since they can also occur as the template name of template specializations, handle them from TemplateName printing instead of TemplateArgument. --- Full diff: https://github.com/llvm/llvm-project/pull/93448.diff 3 Files Affected: - (modified) clang/lib/AST/TemplateBase.cpp (+1-10) - (modified) clang/lib/AST/TemplateName.cpp (+14) - (modified) clang/test/SemaTemplate/deduction-guide.cpp (+5-5) ``diff diff --git a/clang/lib/AST/TemplateBase.cpp b/clang/lib/AST/TemplateBase.cpp index 4d4991d8c38b5..bb0820f4177e4 100644 --- a/clang/lib/AST/TemplateBase.cpp +++ b/clang/lib/AST/TemplateBase.cpp @@ -539,16 +539,7 @@ void TemplateArgument::print(const PrintingPolicy &Policy, raw_ostream &Out, break; case Template: { -TemplateName TN = getAsTemplate(); -if (const auto *TD = TN.getAsTemplateDecl(); -TD && TD->getDeclName().isEmpty()) { - assert(isa(TD) && - "Unexpected anonymous template"); - const auto *TTP = cast(TD); - Out << "template-parameter-" << TTP->getDepth() << "-" << TTP->getIndex(); -} else { - TN.print(Out, Policy); -} +getAsTemplate().print(Out, Policy); break; } diff --git a/clang/lib/AST/TemplateName.cpp b/clang/lib/AST/TemplateName.cpp index 3aae998eceeb0..1ce31f09f2a6a 100644 --- a/clang/lib/AST/TemplateName.cpp +++ b/clang/lib/AST/TemplateName.cpp @@ -292,6 +292,14 @@ void TemplateName::Profile(llvm::FoldingSetNodeID &ID) { void TemplateName::print(raw_ostream &OS, const PrintingPolicy &Policy, Qualified Qual) const { + auto handleCanonicalTTP = [](TemplateDecl *TD, raw_ostream &OS) { +if (TemplateTemplateParmDecl *TTP = dyn_cast(TD); +TTP && TTP->getIdentifier() == nullptr) { + OS << "template-parameter-" << TTP->getDepth() << "-" << TTP->getIndex(); + return true; +} +return false; + }; if (NameKind Kind = getKind(); Kind == TemplateName::Template || Kind == TemplateName::UsingTemplate) { // After `namespace ns { using std::vector }`, what is the fully-qualified @@ -304,6 +312,8 @@ void TemplateName::print(raw_ostream &OS, const PrintingPolicy &Policy, // names more often than to export them, thus using the original name is // most useful in this case. TemplateDecl *Template = getAsTemplateDecl(); +if (handleCanonicalTTP(Template, OS)) + return; if (Qual == Qualified::None) OS << *Template; else @@ -320,6 +330,10 @@ void TemplateName::print(raw_ostream &OS, const PrintingPolicy &Policy, Underlying.getKind() == TemplateName::UsingTemplate); TemplateDecl *UTD = Underlying.getAsTemplateDecl(); + +if (handleCanonicalTTP(UTD, OS)) + return; + if (IdentifierInfo *II = UTD->getIdentifier(); Policy.CleanUglifiedParameters && II && isa(UTD)) diff --git a/clang/test/SemaTemplate/deduction-guide.cpp b/clang/test/SemaTemplate/deduction-guide.cpp index 96b4cd9622a24..100b580fe9f02 100644 --- a/clang/test/SemaTemplate/deduction-guide.cpp +++ b/clang/test/SemaTemplate/deduction-guide.cpp @@ -315,19 +315,19 @@ namespace TTP { // CHECK-NEXT: |-TemplateTypeParmDecl {{.+}} class depth 0 index 0 T{{$}} // CHECK-NEXT: |-TemplateTemplateParmDecl {{.+}} depth 0 index 1 TT{{$}} // CHECK-NEXT: | `-TemplateTypeParmDecl {{.+}} class depth 1 index 0{{$}} -// CHECK-NEXT: |-CXXDeductionGuideDecl {{.+}} 'auto () -> B'{{$}} -// CHECK-NEXT: | `-ParmVarDecl {{.+}} ''{{$}} +// CHECK-NEXT: |-CXXDeductionGuideDecl {{.+}} 'auto (template-parameter-0-1) -> B'{{$}} +// CHECK-NEXT: | `-ParmVarDecl {{.+}} 'template-parameter-0-1'{{$}} // CHECK-NEXT: `-CXXDeductionGuideDecl {{.+}} 'auto (A) -> TTP::B' // CHECK-NEXT:|-TemplateArgument type 'int' // CHECK-NEXT:| `-BuiltinType {{.+}} 'int'{{$}} // CHECK-NEXT:|-TemplateArgument template 'TTP::A'{{$}} // CHECK-NEXT:| `-ClassTemplateDecl {{.+}} A{{$}} // CHECK-NEXT:`-ParmVarDecl {{.+}} 'A':'TTP::A'{{$}} -// CHECK-NEXT: FunctionProtoType {{.+}} 'auto () -> B' dependent trailing_return cdecl{{$}} +// CHECK-NEXT: FunctionProtoType {{.+}} 'auto (template-parameter-0-1) -> B' dependent trailing_return cdecl{{$}} // CHECK-NEXT: |-InjectedClassNameType {{.+}} 'B' dependent{{$}} // CHECK-NEXT: | `-CXXRecord {{.+}} 'B'{{$}} -// CHECK-NEXT: `-ElaboratedType {{.+}} '' sugar dependent{{$}} -// CHECK-NEXT:`-TemplateSpecializationType {{.+}} '' dependent {{$}} +// CHECK-NEXT: `-ElaboratedType {{.+}} 'template-parameter-0-1' sugar dependent{{$}} +// CHECK-NEXT:`-TemplateSpecializationType {{.+}} 'template-parameter-0-1' dependent template-parameter-0-1{{$}} // CHECK-NEXT: `-TemplateArgument type 'T':'type-parameter-0-0'{{$}} // CHECK-NEXT:`-TemplateTypeParmType {{.+}} 'T' dependent depth 0 index 0{{$}} // CHECK-NEXT: `-TemplateTypeParm {{.+}} 'T'{{$}} ``
[llvm-branch-commits] [clang] [libcxx] [clang] Preserve Qualifiers and type sugar in TemplateNames (PR #93433)
https://github.com/Endilll edited https://github.com/llvm/llvm-project/pull/93433 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [libcxx] [clang] Preserve Qualifiers and type sugar in TemplateNames (PR #93433)
https://github.com/Endilll commented: `Sema.h` changes look good to me. https://github.com/llvm/llvm-project/pull/93433 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] release/18.x: [PPCMergeStringPool] Only replace constant once (#92996) (PR #93442)
https://github.com/chenzheng1030 approved this pull request. cherry-pick LGTM https://github.com/llvm/llvm-project/pull/93442 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] release/18.x: [DAGCombiner] In mergeTruncStore, make sure we aren't storing shifted in bits. (#90939) (PR #91038)
RKSimon wrote: > > @AtariDreams I've noticed you've filed a lot of backport requests. How are > > you choosing which fixes to backport? Is there a specific use case you care > > about? > > There a particular LLVM miscompile bug in WebKit I'm trying to figure out. > It's been there since 2019. Backports is literally just avoiding > miscompilations @AtariDreams Has the bug disappeared in llvm trunk and you think a recent commit has fixed/hidden it? Has this bug been reported either to WebKit or LLVM that we can track please? Have you been able to confirm if its a llvm bug or UB in WebKit? https://github.com/llvm/llvm-project/pull/91038 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [serialization] No transitive type change (PR #92511)
https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/92511 >From 1a83e2b0f00183c61e7a5303ea7eeb0b279c7e91 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Fri, 17 May 2024 14:25:53 +0800 Subject: [PATCH] [serialization] No transitive type change --- .../include/clang/Serialization/ASTBitCodes.h | 32 -- clang/include/clang/Serialization/ASTReader.h | 23 ++-- .../clang/Serialization/ASTRecordReader.h | 2 +- .../include/clang/Serialization/ModuleFile.h | 3 - clang/lib/Serialization/ASTReader.cpp | 104 +- clang/lib/Serialization/ASTWriter.cpp | 31 +++--- clang/lib/Serialization/ModuleFile.cpp| 1 - .../Modules/no-transitive-decls-change.cppm | 12 +- .../no-transitive-identifier-change.cppm | 3 - .../Modules/no-transitive-type-change.cppm| 68 clang/test/Modules/pr5.cppm | 36 +++--- 11 files changed, 196 insertions(+), 119 deletions(-) create mode 100644 clang/test/Modules/no-transitive-type-change.cppm diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h index 99ad8524bde80..092d46b41e2cf 100644 --- a/clang/include/clang/Serialization/ASTBitCodes.h +++ b/clang/include/clang/Serialization/ASTBitCodes.h @@ -26,6 +26,7 @@ #include "clang/Serialization/SourceLocationEncoding.h" #include "llvm/ADT/DenseMapInfo.h" #include "llvm/Bitstream/BitCodes.h" +#include "llvm/Support/MathExtras.h" #include #include @@ -70,38 +71,53 @@ using DeclID = DeclIDBase::DeclID; /// An ID number that refers to a type in an AST file. /// -/// The ID of a type is partitioned into two parts: the lower +/// The ID of a type is partitioned into three parts: +/// - the lower /// three bits are used to store the const/volatile/restrict -/// qualifiers (as with QualType) and the upper bits provide a -/// type index. The type index values are partitioned into two +/// qualifiers (as with QualType). +/// - the upper 29 bits provide a type index in the corresponding +/// module file. +/// - the upper 32 bits provide a module file index. +/// +/// The type index values are partitioned into two /// sets. The values below NUM_PREDEF_TYPE_IDs are predefined type /// IDs (based on the PREDEF_TYPE_*_ID constants), with 0 as a /// placeholder for "no type". Values from NUM_PREDEF_TYPE_IDs are /// other types that have serialized representations. -using TypeID = uint32_t; +using TypeID = uint64_t; /// A type index; the type ID with the qualifier bits removed. +/// Keep structure alignment 32-bit since the blob is assumed as 32-bit +/// aligned. class TypeIdx { + uint32_t ModuleFileIndex = 0; uint32_t Idx = 0; public: TypeIdx() = default; - explicit TypeIdx(uint32_t index) : Idx(index) {} + explicit TypeIdx(uint32_t Idx) : ModuleFileIndex(0), Idx(Idx) {} + + explicit TypeIdx(uint32_t ModuleFileIdx, uint32_t Idx) + : ModuleFileIndex(ModuleFileIdx), Idx(Idx) {} + + uint32_t getModuleFileIndex() const { return ModuleFileIndex; } - uint32_t getIndex() const { return Idx; } + uint64_t getValue() const { return ((uint64_t)ModuleFileIndex << 32) | Idx; } TypeID asTypeID(unsigned FastQuals) const { if (Idx == uint32_t(-1)) return TypeID(-1); -return (Idx << Qualifiers::FastWidth) | FastQuals; +unsigned Index = (Idx << Qualifiers::FastWidth) | FastQuals; +return ((uint64_t)ModuleFileIndex << 32) | Index; } static TypeIdx fromTypeID(TypeID ID) { if (ID == TypeID(-1)) return TypeIdx(-1); -return TypeIdx(ID >> Qualifiers::FastWidth); +return TypeIdx(ID >> 32, (ID & llvm::maskTrailingOnes(32)) >> + Qualifiers::FastWidth); } }; diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index ef98a49cf491f..aec4e01ea07c0 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -487,14 +487,6 @@ class ASTReader /// ID = (I + 1) << FastQual::Width has already been loaded llvm::PagedVector TypesLoaded; - using GlobalTypeMapType = - ContinuousRangeMap; - - /// Mapping from global type IDs to the module in which the - /// type resides along with the offset that should be added to the - /// global type ID to produce a local ID. - GlobalTypeMapType GlobalTypeMap; - /// Declarations that have already been loaded from the chain. /// /// When the pointer at index I is non-NULL, the declaration with ID @@ -1420,8 +1412,8 @@ class ASTReader RecordLocation(ModuleFile *M, uint64_t O) : F(M), Offset(O) {} }; - QualType readTypeRecord(unsigned Index); - RecordLocation TypeCursorForIndex(unsigned Index); + QualType readTypeRecord(serialization::TypeID ID); + RecordLocation TypeCursorForIndex(serialization::TypeID ID); void LoadedDecl(unsigned Index, Decl *D); Decl *ReadDeclRecord(GlobalDeclID
[llvm-branch-commits] [mlir] [draft] Dialect Conversion without Rollback (PR #93412)
https://github.com/Dinistro commented: IIUC, this inherits from `ConversionPatternRewriter` to ensure that the conversion patterns can be used with the new `OneShotConversionPatternRewriter`. While this makes sense from a functional perspective, the introduced inheritance relation seems a bit conflicting, as the super class has supposedly more functionality than the derived class. In many practical cases, replacing usages of `ConversionPatternRewriter` with a `OneShotCovnersionPatternRewriter` might work, this is not true in the general case, right? https://github.com/llvm/llvm-project/pull/93412 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [draft] Dialect Conversion without Rollback (PR #93412)
matthias-springer wrote: The final `OneShotConversionPatternRewriter` will support everything that `ConversionPatternRewriter` supports. A few helper functions are still missing in this PR: - `applySignatureConversion` - `convertRegionTypes` - `convertNonEntryRegionTypes` - `replaceUsesOfBlockArgument` (removed by #84725) These functions would have to become virtual, so that they can be overridden in `OneShotConversionPatternRewriter`. https://github.com/llvm/llvm-project/pull/93412 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [draft] Dialect Conversion without Rollback (PR #93412)
Dinistro wrote: > The final `OneShotConversionPatternRewriter` will support everything that > `ConversionPatternRewriter` supports. A few helper functions are still > missing in this PR: > > * `applySignatureConversion` > * `convertRegionTypes` > * `convertNonEntryRegionTypes` > * `replaceUsesOfBlockArgument` (removed by [[mlir][Transforms] Support > `replaceAllUsesWith` in dialect conversionĀ > #84725](https://github.com/llvm/llvm-project/pull/84725)) > > These functions would have to become virtual, so that they can be overridden > in `OneShotConversionPatternRewriter`. I was mainly referring to the rollback logic. A pattern using the super class might assume rollback, which then breaks when it uses the specialised class. This violates the liskov substitution principle, which might be considered problematic. I understand that we might deliberately do so, to ensure a smooth transition to the new infra, without requiring substantial changes. I just fear that we will never be able to remove the inheritance if we build this in now. https://github.com/llvm/llvm-project/pull/93412 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [draft] Dialect Conversion without Rollback (PR #93412)
matthias-springer wrote: You are right, there is one thing that conversion patterns are no longer allowed to do. From the RFC: ``` One change is needed for existing conversion patterns: they must not modify the IR if they return failure. (Same as ordinary rewrite patterns.) Most patterns should already be written in such a way. for all other patterns, that should be a simple fix. ``` We already have assertions that check this in the greedy pattern rewrite driver (`ExpensiveChecks`); must be enabled explicitly because they are expensive. This prototype is reuses some of the greedy pattern rewrite driver, so we get these checks for free. Swapping the order of the two conversion rewriters in the class hierarchy won't make a difference here: for patterns to work with the new driver, they must not make changes to the IR and then `return failure`. Also, if `ConversionRewriter` inherits from `OneShotConversionRewriter`, it won't be possible to use existing patterns with the new driver (unless we also change the name of `ConversionRewriter`->`LegacyConversionRewriter` and `OneShotConversionRewriter`->`ConversionRewriter`.) > I just fear that we will never be able to remove the inheritance if we build > this in now. I share some of that concern. But I find it important to build something that is backwards compatible (with existing conversion patterns); with a clear and simple migration path. Otherwise, nobody is going to use the new driver. (There are so many helpers like function op conversion patterns or structural SCF conversion patterns, that people will want to reuse.) https://github.com/llvm/llvm-project/pull/93412 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [libcxx] [clang] Preserve Qualifiers and type sugar in TemplateNames (PR #93433)
https://github.com/ldionne approved this pull request. `libcxx/` changes LGTM. https://github.com/llvm/llvm-project/pull/93433 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [libcxx] [clang] Preserve Qualifiers and type sugar in TemplateNames (PR #93433)
@@ -9304,7 +9299,8 @@ TemplateName ASTContext::getAssumedTemplateName(DeclarationName Name) const { TemplateName ASTContext::getQualifiedTemplateName(NestedNameSpecifier *NNS, bool TemplateKeyword, TemplateName Template) const { - assert(NNS && "Missing nested-name-specifier in qualified template name"); mizvekov wrote: In 'QualifiedTemplateName', there are two qualifiers: The nested name specifier, and the template keyword. Just as a template might be written without a template keyword qualifier, it can also be written without a nested name qualifier. You can have one, the other, both, or none. This assert was already wrong in that it was valid to have a template written with template keyword, but written without any nested name. For example an object access to a member template: `p->template foo<...>`. That is fixed in this patch and you can see that in tests. When we know how a template was written, we wish to print them as that. We have a third situation here: A canonical template name, that is, a template which we have no information on how it was written. We want to be able to tell these apart, because we don't wish to print them as if they had no qualifiers. Quite the contrary, we want to synthesize a maximal nested name for them, based on their DC, and print them with that. Otherwise, there is no in-band NestedNameSpecifier non-null value to indicate an empty nested name. Null is what we use here, and what is used in other places, like for example the aforementioned ElaboratedType. In fact, as I mentioned before, this issue is much similar to what we had for ElaboratedType before https://reviews.llvm.org/D112374, except in that case we already recognized that a class name might be written with elaboration, but no nested name, unlike here which the 'only a template keyword' situation was forgotten. In that case, a `nullptr` was already used to represent an empty NestedNameSpecifier, as expected. https://github.com/llvm/llvm-project/pull/93433 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [clang-format] Fix a bug in formatting goto labels in macros (#92494) (PR #93494)
https://github.com/llvmbot milestoned https://github.com/llvm/llvm-project/pull/93494 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [clang-format] Fix a bug in formatting goto labels in macros (#92494) (PR #93494)
llvmbot wrote: @mydeveloperday What do you think about merging this PR to the release branch? https://github.com/llvm/llvm-project/pull/93494 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [clang-format] Fix a bug in formatting goto labels in macros (#92494) (PR #93494)
https://github.com/llvmbot created https://github.com/llvm/llvm-project/pull/93494 Backport d89f20058b45e3836527e816af7ed7372e1d554d Requested by: @owenca >From ad6e7384693cf38ed89dc02fc6e2e0ac215fbd66 Mon Sep 17 00:00:00 2001 From: Owen Pan Date: Tue, 21 May 2024 01:35:31 -0700 Subject: [PATCH] [clang-format] Fix a bug in formatting goto labels in macros (#92494) Fixes #92300. (cherry picked from commit d89f20058b45e3836527e816af7ed7372e1d554d) --- clang/lib/Format/UnwrappedLineParser.cpp | 9 ++--- clang/unittests/Format/FormatTest.cpp | 8 clang/unittests/Format/TokenAnnotatorTest.cpp | 13 + 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp index f70affb732a0d..179d77bf00491 100644 --- a/clang/lib/Format/UnwrappedLineParser.cpp +++ b/clang/lib/Format/UnwrappedLineParser.cpp @@ -1185,12 +1185,6 @@ void UnwrappedLineParser::parsePPDefine() { return; } - if (FormatTok->is(tok::identifier) && - Tokens->peekNextToken()->is(tok::colon)) { -nextToken(); -nextToken(); - } - // Errors during a preprocessor directive can only affect the layout of the // preprocessor directive, and thus we ignore them. An alternative approach // would be to use the same approach we use on the file level (no @@ -1671,7 +1665,8 @@ void UnwrappedLineParser::parseStructuralElement( if (!Style.isJavaScript() && !Style.isVerilog() && !Style.isTableGen() && Tokens->peekNextToken()->is(tok::colon) && !Line->MustBeDeclaration) { nextToken(); - Line->Tokens.begin()->Tok->MustBreakBefore = true; + if (!Line->InMacroBody || CurrentLines->size() > 1) +Line->Tokens.begin()->Tok->MustBreakBefore = true; FormatTok->setFinalizedType(TT_GotoLabelColon); parseLabel(!Style.IndentGotoLabels); if (HasLabel) diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 0161a1685eb12..df730838738d1 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -3118,6 +3118,7 @@ TEST_F(FormatTest, FormatsLabels) { "g();\n" " }\n" "}"); + FormatStyle Style = getLLVMStyle(); Style.IndentGotoLabels = false; verifyFormat("void f() {\n" @@ -3157,6 +3158,13 @@ TEST_F(FormatTest, FormatsLabels) { " }\n" "}", Style); + + Style.ColumnLimit = 15; + verifyFormat("#define FOO \\\n" + "label:\\\n" + " break;", + Style); + // The opening brace may either be on the same unwrapped line as the colon or // on a separate one. The formatter should recognize both. Style = getLLVMStyle(); diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp index 44ebad9d5a872..dfa9c22430f36 100644 --- a/clang/unittests/Format/TokenAnnotatorTest.cpp +++ b/clang/unittests/Format/TokenAnnotatorTest.cpp @@ -2353,15 +2353,28 @@ TEST_F(TokenAnnotatorTest, UnderstandsLabels) { auto Tokens = annotate("{ x: break; }"); ASSERT_EQ(Tokens.size(), 7u) << Tokens; EXPECT_TOKEN(Tokens[2], tok::colon, TT_GotoLabelColon); + Tokens = annotate("{ case x: break; }"); ASSERT_EQ(Tokens.size(), 8u) << Tokens; EXPECT_TOKEN(Tokens[3], tok::colon, TT_CaseLabelColon); + Tokens = annotate("{ x: { break; } }"); ASSERT_EQ(Tokens.size(), 9u) << Tokens; EXPECT_TOKEN(Tokens[2], tok::colon, TT_GotoLabelColon); + Tokens = annotate("{ case x: { break; } }"); ASSERT_EQ(Tokens.size(), 10u) << Tokens; EXPECT_TOKEN(Tokens[3], tok::colon, TT_CaseLabelColon); + + Tokens = annotate("#define FOO label:"); + ASSERT_EQ(Tokens.size(), 6u) << Tokens; + EXPECT_TOKEN(Tokens[4], tok::colon, TT_GotoLabelColon); + + Tokens = annotate("#define FOO \\\n" +"label: \\\n" +" break;"); + ASSERT_EQ(Tokens.size(), 8u) << Tokens; + EXPECT_TOKEN(Tokens[4], tok::colon, TT_GotoLabelColon); } TEST_F(TokenAnnotatorTest, UnderstandsNestedBlocks) { ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [clang-format] Fix a bug in formatting goto labels in macros (#92494) (PR #93494)
llvmbot wrote: @llvm/pr-subscribers-clang-format Author: None (llvmbot) Changes Backport d89f20058b45e3836527e816af7ed7372e1d554d Requested by: @owenca --- Full diff: https://github.com/llvm/llvm-project/pull/93494.diff 3 Files Affected: - (modified) clang/lib/Format/UnwrappedLineParser.cpp (+2-7) - (modified) clang/unittests/Format/FormatTest.cpp (+8) - (modified) clang/unittests/Format/TokenAnnotatorTest.cpp (+13) ``diff diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp index f70affb732a0d..179d77bf00491 100644 --- a/clang/lib/Format/UnwrappedLineParser.cpp +++ b/clang/lib/Format/UnwrappedLineParser.cpp @@ -1185,12 +1185,6 @@ void UnwrappedLineParser::parsePPDefine() { return; } - if (FormatTok->is(tok::identifier) && - Tokens->peekNextToken()->is(tok::colon)) { -nextToken(); -nextToken(); - } - // Errors during a preprocessor directive can only affect the layout of the // preprocessor directive, and thus we ignore them. An alternative approach // would be to use the same approach we use on the file level (no @@ -1671,7 +1665,8 @@ void UnwrappedLineParser::parseStructuralElement( if (!Style.isJavaScript() && !Style.isVerilog() && !Style.isTableGen() && Tokens->peekNextToken()->is(tok::colon) && !Line->MustBeDeclaration) { nextToken(); - Line->Tokens.begin()->Tok->MustBreakBefore = true; + if (!Line->InMacroBody || CurrentLines->size() > 1) +Line->Tokens.begin()->Tok->MustBreakBefore = true; FormatTok->setFinalizedType(TT_GotoLabelColon); parseLabel(!Style.IndentGotoLabels); if (HasLabel) diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 0161a1685eb12..df730838738d1 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -3118,6 +3118,7 @@ TEST_F(FormatTest, FormatsLabels) { "g();\n" " }\n" "}"); + FormatStyle Style = getLLVMStyle(); Style.IndentGotoLabels = false; verifyFormat("void f() {\n" @@ -3157,6 +3158,13 @@ TEST_F(FormatTest, FormatsLabels) { " }\n" "}", Style); + + Style.ColumnLimit = 15; + verifyFormat("#define FOO \\\n" + "label:\\\n" + " break;", + Style); + // The opening brace may either be on the same unwrapped line as the colon or // on a separate one. The formatter should recognize both. Style = getLLVMStyle(); diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp index 44ebad9d5a872..dfa9c22430f36 100644 --- a/clang/unittests/Format/TokenAnnotatorTest.cpp +++ b/clang/unittests/Format/TokenAnnotatorTest.cpp @@ -2353,15 +2353,28 @@ TEST_F(TokenAnnotatorTest, UnderstandsLabels) { auto Tokens = annotate("{ x: break; }"); ASSERT_EQ(Tokens.size(), 7u) << Tokens; EXPECT_TOKEN(Tokens[2], tok::colon, TT_GotoLabelColon); + Tokens = annotate("{ case x: break; }"); ASSERT_EQ(Tokens.size(), 8u) << Tokens; EXPECT_TOKEN(Tokens[3], tok::colon, TT_CaseLabelColon); + Tokens = annotate("{ x: { break; } }"); ASSERT_EQ(Tokens.size(), 9u) << Tokens; EXPECT_TOKEN(Tokens[2], tok::colon, TT_GotoLabelColon); + Tokens = annotate("{ case x: { break; } }"); ASSERT_EQ(Tokens.size(), 10u) << Tokens; EXPECT_TOKEN(Tokens[3], tok::colon, TT_CaseLabelColon); + + Tokens = annotate("#define FOO label:"); + ASSERT_EQ(Tokens.size(), 6u) << Tokens; + EXPECT_TOKEN(Tokens[4], tok::colon, TT_GotoLabelColon); + + Tokens = annotate("#define FOO \\\n" +"label: \\\n" +" break;"); + ASSERT_EQ(Tokens.size(), 8u) << Tokens; + EXPECT_TOKEN(Tokens[4], tok::colon, TT_GotoLabelColon); } TEST_F(TokenAnnotatorTest, UnderstandsNestedBlocks) { `` https://github.com/llvm/llvm-project/pull/93494 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [clang-format] Fix a bug in formatting goto labels in macros (#92494) (PR #93494)
owenca wrote: @tstellar can we get this one in if we are doing another point release? https://github.com/llvm/llvm-project/pull/93494 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] release/18.x: [clang-format] Fix a bug in formatting goto labels in macros (#92494) (PR #93494)
https://github.com/owenca approved this pull request. https://github.com/llvm/llvm-project/pull/93494 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [LAA] Use getBackedgeTakenCountForCountableExits. (PR #93499)
https://github.com/fhahn created https://github.com/llvm/llvm-project/pull/93499 Update LAA to use getBackedgeTakenCountForCountableExits which returns the minimum of the countable exits When analyzing dependences and computing runtime checks, we need the smallest upper bound on the number of iterations. In terms of memory safety, it shouldn't matter if any uncomputable exits leave the loop, as long as we prove that there are no dependences given the minimum of the countable exits. The same should apply also for generating runtime checks. Note that this shifts the responsiblity of checking whether all exit counts are computable or handling early-exits to the users of LAA. Depends on https://github.com/llvm/llvm-project/pull/93498 >From 80decf5050269fa0e91bf0b397ac9a7565cd6d72 Mon Sep 17 00:00:00 2001 From: Florian Hahn Date: Wed, 8 May 2024 20:47:29 +0100 Subject: [PATCH] [LAA] Use getBackedgeTakenCountForCountableExits. Update LAA to use getBackedgeTakenCountForCountableExits which returns the minimum of the countable exits When analyzing dependences and computing runtime checks, we need the smallest upper bound on the number of iterations. In terms of memory safety, it shouldn't matter if any uncomputable exits leave the loop, as long as we prove that there are no dependences given the minimum of the countable exits. The same should apply also for generating runtime checks. Note that this shifts the responsiblity of checking whether all exit counts are computable or handling early-exits to the users of LAA. --- llvm/lib/Analysis/LoopAccessAnalysis.cpp | 11 ++- .../Vectorize/LoopVectorizationLegality.cpp | 10 ++ .../early-exit-runtime-checks.ll | 26 - .../Transforms/LoopDistribute/early-exit.ll | 96 +++ .../Transforms/LoopLoadElim/early-exit.ll | 61 5 files changed, 197 insertions(+), 7 deletions(-) create mode 100644 llvm/test/Transforms/LoopDistribute/early-exit.ll create mode 100644 llvm/test/Transforms/LoopLoadElim/early-exit.ll diff --git a/llvm/lib/Analysis/LoopAccessAnalysis.cpp b/llvm/lib/Analysis/LoopAccessAnalysis.cpp index bc8b9b8479e4f..f15dcaf94ee11 100644 --- a/llvm/lib/Analysis/LoopAccessAnalysis.cpp +++ b/llvm/lib/Analysis/LoopAccessAnalysis.cpp @@ -214,7 +214,7 @@ getStartAndEndForAccess(const Loop *Lp, const SCEV *PtrExpr, Type *AccessTy, if (SE->isLoopInvariant(PtrExpr, Lp)) { ScStart = ScEnd = PtrExpr; } else if (auto *AR = dyn_cast(PtrExpr)) { -const SCEV *Ex = PSE.getBackedgeTakenCount(); +const SCEV *Ex = PSE.getBackedgeTakenCountForCountableExits(); ScStart = AR->getStart(); ScEnd = AR->evaluateAtIteration(Ex, *SE); @@ -2056,8 +2056,9 @@ MemoryDepChecker::Dependence::DepType MemoryDepChecker::isDependent( // i.e. they are far enough appart that accesses won't access the same // location across all loop ierations. if (HasSameSize && - isSafeDependenceDistance(DL, SE, *(PSE.getBackedgeTakenCount()), *Dist, - MaxStride, TypeByteSize)) + isSafeDependenceDistance(DL, SE, + *(PSE.getBackedgeTakenCountForCountableExits()), + *Dist, MaxStride, TypeByteSize)) return Dependence::NoDep; const SCEVConstant *C = dyn_cast(Dist); @@ -2395,7 +2396,7 @@ bool LoopAccessInfo::canAnalyzeLoop() { } // ScalarEvolution needs to be able to find the exit count. - const SCEV *ExitCount = PSE->getBackedgeTakenCount(); + const SCEV *ExitCount = PSE->getBackedgeTakenCountForCountableExits(); if (isa(ExitCount)) { recordAnalysis("CantComputeNumberOfIterations") << "could not determine number of loop iterations"; @@ -3004,7 +3005,7 @@ void LoopAccessInfo::collectStridedAccess(Value *MemAccess) { // of various possible stride specializations, considering the alternatives // of using gather/scatters (if available). - const SCEV *BETakenCount = PSE->getBackedgeTakenCount(); + const SCEV *BETakenCount = PSE->getBackedgeTakenCountForCountableExits(); // Match the types so we can compare the stride and the BETakenCount. // The Stride can be positive/negative, so we sign extend Stride; diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp index 9de49d1bcfeac..0c18c4e146de1 100644 --- a/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp +++ b/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp @@ -1506,6 +1506,16 @@ bool LoopVectorizationLegality::canVectorize(bool UseVPlanNativePath) { return false; } + if (isa(PSE.getBackedgeTakenCount())) { +reportVectorizationFailure("could not determine number of loop iterations", + "could not determine number of loop iterations", + "CantComputeNumberOfIterations", ORE, TheLoop); +if (DoExtraAnalysis) + Result = false; +els
[llvm-branch-commits] [llvm] [LAA] Use getBackedgeTakenCountForCountableExits. (PR #93499)
llvmbot wrote: @llvm/pr-subscribers-llvm-analysis Author: Florian Hahn (fhahn) Changes Update LAA to use getBackedgeTakenCountForCountableExits which returns the minimum of the countable exits When analyzing dependences and computing runtime checks, we need the smallest upper bound on the number of iterations. In terms of memory safety, it shouldn't matter if any uncomputable exits leave the loop, as long as we prove that there are no dependences given the minimum of the countable exits. The same should apply also for generating runtime checks. Note that this shifts the responsiblity of checking whether all exit counts are computable or handling early-exits to the users of LAA. Depends on https://github.com/llvm/llvm-project/pull/93498 --- Full diff: https://github.com/llvm/llvm-project/pull/93499.diff 5 Files Affected: - (modified) llvm/lib/Analysis/LoopAccessAnalysis.cpp (+6-5) - (modified) llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp (+10) - (modified) llvm/test/Analysis/LoopAccessAnalysis/early-exit-runtime-checks.ll (+24-2) - (added) llvm/test/Transforms/LoopDistribute/early-exit.ll (+96) - (added) llvm/test/Transforms/LoopLoadElim/early-exit.ll (+61) ``diff diff --git a/llvm/lib/Analysis/LoopAccessAnalysis.cpp b/llvm/lib/Analysis/LoopAccessAnalysis.cpp index bc8b9b8479e4f..f15dcaf94ee11 100644 --- a/llvm/lib/Analysis/LoopAccessAnalysis.cpp +++ b/llvm/lib/Analysis/LoopAccessAnalysis.cpp @@ -214,7 +214,7 @@ getStartAndEndForAccess(const Loop *Lp, const SCEV *PtrExpr, Type *AccessTy, if (SE->isLoopInvariant(PtrExpr, Lp)) { ScStart = ScEnd = PtrExpr; } else if (auto *AR = dyn_cast(PtrExpr)) { -const SCEV *Ex = PSE.getBackedgeTakenCount(); +const SCEV *Ex = PSE.getBackedgeTakenCountForCountableExits(); ScStart = AR->getStart(); ScEnd = AR->evaluateAtIteration(Ex, *SE); @@ -2056,8 +2056,9 @@ MemoryDepChecker::Dependence::DepType MemoryDepChecker::isDependent( // i.e. they are far enough appart that accesses won't access the same // location across all loop ierations. if (HasSameSize && - isSafeDependenceDistance(DL, SE, *(PSE.getBackedgeTakenCount()), *Dist, - MaxStride, TypeByteSize)) + isSafeDependenceDistance(DL, SE, + *(PSE.getBackedgeTakenCountForCountableExits()), + *Dist, MaxStride, TypeByteSize)) return Dependence::NoDep; const SCEVConstant *C = dyn_cast(Dist); @@ -2395,7 +2396,7 @@ bool LoopAccessInfo::canAnalyzeLoop() { } // ScalarEvolution needs to be able to find the exit count. - const SCEV *ExitCount = PSE->getBackedgeTakenCount(); + const SCEV *ExitCount = PSE->getBackedgeTakenCountForCountableExits(); if (isa(ExitCount)) { recordAnalysis("CantComputeNumberOfIterations") << "could not determine number of loop iterations"; @@ -3004,7 +3005,7 @@ void LoopAccessInfo::collectStridedAccess(Value *MemAccess) { // of various possible stride specializations, considering the alternatives // of using gather/scatters (if available). - const SCEV *BETakenCount = PSE->getBackedgeTakenCount(); + const SCEV *BETakenCount = PSE->getBackedgeTakenCountForCountableExits(); // Match the types so we can compare the stride and the BETakenCount. // The Stride can be positive/negative, so we sign extend Stride; diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp index 9de49d1bcfeac..0c18c4e146de1 100644 --- a/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp +++ b/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp @@ -1506,6 +1506,16 @@ bool LoopVectorizationLegality::canVectorize(bool UseVPlanNativePath) { return false; } + if (isa(PSE.getBackedgeTakenCount())) { +reportVectorizationFailure("could not determine number of loop iterations", + "could not determine number of loop iterations", + "CantComputeNumberOfIterations", ORE, TheLoop); +if (DoExtraAnalysis) + Result = false; +else + return false; + } + LLVM_DEBUG(dbgs() << "LV: We can vectorize this loop" << (LAI->getRuntimePointerChecking()->Need ? " (with a runtime bound check)" diff --git a/llvm/test/Analysis/LoopAccessAnalysis/early-exit-runtime-checks.ll b/llvm/test/Analysis/LoopAccessAnalysis/early-exit-runtime-checks.ll index 0d85f11f06dce..ea3b8814cfe7f 100644 --- a/llvm/test/Analysis/LoopAccessAnalysis/early-exit-runtime-checks.ll +++ b/llvm/test/Analysis/LoopAccessAnalysis/early-exit-runtime-checks.ll @@ -4,10 +4,21 @@ define void @all_exits_dominate_latch_countable_exits_at_most_500_iterations(ptr %A, ptr %B) { ; CHECK-LABEL: 'all_exits_dominate_latch_countable_exits_at_most_500_iterations' ; CHECK-NEXT:loop.header: -; CHECK-NEXT: Report: could n
[llvm-branch-commits] [llvm] [LAA] Use getBackedgeTakenCountForCountableExits. (PR #93499)
https://github.com/fhahn updated https://github.com/llvm/llvm-project/pull/93499 >From 1ce660b45d3706912705bc9e7a8c19e86f05d0c0 Mon Sep 17 00:00:00 2001 From: Florian Hahn Date: Wed, 8 May 2024 20:47:29 +0100 Subject: [PATCH] [LAA] Use getBackedgeTakenCountForCountableExits. Update LAA to use getBackedgeTakenCountForCountableExits which returns the minimum of the countable exits When analyzing dependences and computing runtime checks, we need the smallest upper bound on the number of iterations. In terms of memory safety, it shouldn't matter if any uncomputable exits leave the loop, as long as we prove that there are no dependences given the minimum of the countable exits. The same should apply also for generating runtime checks. Note that this shifts the responsiblity of checking whether all exit counts are computable or handling early-exits to the users of LAA. --- llvm/lib/Analysis/LoopAccessAnalysis.cpp | 12 +-- .../Vectorize/LoopVectorizationLegality.cpp | 10 ++ .../early-exit-runtime-checks.ll | 39 +++- .../memcheck-wrapping-pointers.ll | 14 +-- .../Transforms/LoopDistribute/early-exit.ll | 96 +++ .../Transforms/LoopLoadElim/early-exit.ll | 61 6 files changed, 216 insertions(+), 16 deletions(-) create mode 100644 llvm/test/Transforms/LoopDistribute/early-exit.ll create mode 100644 llvm/test/Transforms/LoopLoadElim/early-exit.ll diff --git a/llvm/lib/Analysis/LoopAccessAnalysis.cpp b/llvm/lib/Analysis/LoopAccessAnalysis.cpp index bc8b9b8479e4f..58096b66f704b 100644 --- a/llvm/lib/Analysis/LoopAccessAnalysis.cpp +++ b/llvm/lib/Analysis/LoopAccessAnalysis.cpp @@ -214,7 +214,7 @@ getStartAndEndForAccess(const Loop *Lp, const SCEV *PtrExpr, Type *AccessTy, if (SE->isLoopInvariant(PtrExpr, Lp)) { ScStart = ScEnd = PtrExpr; } else if (auto *AR = dyn_cast(PtrExpr)) { -const SCEV *Ex = PSE.getBackedgeTakenCount(); +const SCEV *Ex = PSE.getSymbolicMaxBackedgeTakenCount(); ScStart = AR->getStart(); ScEnd = AR->evaluateAtIteration(Ex, *SE); @@ -2055,9 +2055,9 @@ MemoryDepChecker::Dependence::DepType MemoryDepChecker::isDependent( // stride multiplied by the backedge taken count, the accesses are independet, // i.e. they are far enough appart that accesses won't access the same // location across all loop ierations. - if (HasSameSize && - isSafeDependenceDistance(DL, SE, *(PSE.getBackedgeTakenCount()), *Dist, - MaxStride, TypeByteSize)) + if (HasSameSize && isSafeDependenceDistance( + DL, SE, *(PSE.getSymbolicMaxBackedgeTakenCount()), + *Dist, MaxStride, TypeByteSize)) return Dependence::NoDep; const SCEVConstant *C = dyn_cast(Dist); @@ -2395,7 +2395,7 @@ bool LoopAccessInfo::canAnalyzeLoop() { } // ScalarEvolution needs to be able to find the exit count. - const SCEV *ExitCount = PSE->getBackedgeTakenCount(); + const SCEV *ExitCount = PSE->getSymbolicMaxBackedgeTakenCount(); if (isa(ExitCount)) { recordAnalysis("CantComputeNumberOfIterations") << "could not determine number of loop iterations"; @@ -3004,7 +3004,7 @@ void LoopAccessInfo::collectStridedAccess(Value *MemAccess) { // of various possible stride specializations, considering the alternatives // of using gather/scatters (if available). - const SCEV *BETakenCount = PSE->getBackedgeTakenCount(); + const SCEV *BETakenCount = PSE->getSymbolicMaxBackedgeTakenCount(); // Match the types so we can compare the stride and the BETakenCount. // The Stride can be positive/negative, so we sign extend Stride; diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp index 9de49d1bcfeac..0c18c4e146de1 100644 --- a/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp +++ b/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp @@ -1506,6 +1506,16 @@ bool LoopVectorizationLegality::canVectorize(bool UseVPlanNativePath) { return false; } + if (isa(PSE.getBackedgeTakenCount())) { +reportVectorizationFailure("could not determine number of loop iterations", + "could not determine number of loop iterations", + "CantComputeNumberOfIterations", ORE, TheLoop); +if (DoExtraAnalysis) + Result = false; +else + return false; + } + LLVM_DEBUG(dbgs() << "LV: We can vectorize this loop" << (LAI->getRuntimePointerChecking()->Need ? " (with a runtime bound check)" diff --git a/llvm/test/Analysis/LoopAccessAnalysis/early-exit-runtime-checks.ll b/llvm/test/Analysis/LoopAccessAnalysis/early-exit-runtime-checks.ll index 0d85f11f06dce..a40aaa8ae99a0 100644 --- a/llvm/test/Analysis/LoopAccessAnalysis/early-exit-runtime-checks.ll +++ b/llvm/test/Analysis/LoopAccessAnalysis/early-exit-runtim
[llvm-branch-commits] [llvm] [LAA] Use PSE::getSymbolicMaxBackedgeTakenCount. (PR #93499)
https://github.com/fhahn edited https://github.com/llvm/llvm-project/pull/93499 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [LAA] Use PSE::getSymbolicMaxBackedgeTakenCount. (PR #93499)
https://github.com/fhahn edited https://github.com/llvm/llvm-project/pull/93499 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits