[clang] [clang-tools-extra] [clang] Refactor `clang::Linkage` (PR #71049)
@@ -1996,7 +1996,7 @@ class alignas(TypeAlignment) Type : public
ExtQualsTypeCommonBase {
TypeBits.Dependence = static_cast(Dependence);
TypeBits.CacheValid = false;
TypeBits.CachedLocalOrUnnamed = false;
-TypeBits.CachedLinkage = NoLinkage;
+TypeBits.CachedLinkage = llvm::to_underlying(Linkage::Invalid);
Endilll wrote:
CC @AaronBallman This passes Clang tests with all those `llvm_unreachable`
around, so I'm rather confident about this. Note that I removed `NFC` tag from
the title.
https://github.com/llvm/llvm-project/pull/71049
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang] Refactor `clang::Linkage` (PR #71049)
@@ -2214,7 +2214,7 @@ void ASTWriter::WriteDeclAbbrevs() {
Abv->Add(BitCodeAbbrevOp(0)); // TSCSpec
Abv->Add(BitCodeAbbrevOp(0)); // InitStyle
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isARCPseudoStrong
- Abv->Add(BitCodeAbbrevOp(0)); // Linkage
+ Abv->Add(BitCodeAbbrevOp(1)); // Linkage
Endilll wrote:
Applied!
https://github.com/llvm/llvm-project/pull/71049
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang] Refactor `clang::Linkage` (PR #71049)
https://github.com/Endilll updated
https://github.com/llvm/llvm-project/pull/71049
>From 05089e60021c321b4113db7e4bdf59bdaaa19de7 Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov
Date: Thu, 2 Nov 2023 14:40:12 +0300
Subject: [PATCH 1/4] [clang][NFC] Refactor `clang::Linkage`
This patch introduces a new enumerator `Invalid = 0`, shifting other
enumerators by +1. Contrary to how it might sound, this actually affirms status
quo of how this enum is stored in `clang::Decl`:
```
/// If 0, we have not computed the linkage of this declaration.
/// Otherwise, it is the linkage + 1.
mutable unsigned CacheValidAndLinkage : 3;
```
This patch makes debuggers to not be mistaken about enumerator stored in this
bit-field. It also converts `clang::Linkage` to a scoped enum.
---
clang-tools-extra/clang-doc/Serialize.cpp | 4 +-
clang-tools-extra/clangd/Quality.cpp | 3 +-
.../clangd/SemanticHighlighting.cpp | 3 +-
clang/include/clang/AST/DeclBase.h| 11 ++--
clang/include/clang/AST/Type.h| 2 +-
clang/include/clang/Basic/Linkage.h | 53 +++---
clang/include/clang/Basic/Visibility.h| 28 +-
clang/lib/AST/APValue.cpp | 2 +-
clang/lib/AST/Decl.cpp| 28 +++---
clang/lib/AST/ItaniumMangle.cpp | 4 +-
clang/lib/AST/MicrosoftMangle.cpp | 5 +-
clang/lib/AST/Type.cpp| 8 +--
clang/lib/CodeGen/CodeGenModule.cpp | 4 +-
clang/lib/CodeGen/ItaniumCXXABI.cpp | 15 +++--
clang/lib/CodeGen/MicrosoftCXXABI.cpp | 15 +++--
clang/lib/Index/IndexSymbol.cpp | 20 ---
clang/lib/Sema/SemaChecking.cpp | 2 +-
clang/lib/Sema/SemaDecl.cpp | 24
clang/lib/Sema/SemaExpr.cpp | 2 +-
clang/lib/Sema/SemaModule.cpp | 6 +-
clang/lib/Sema/SemaOverload.cpp | 2 +-
clang/lib/Sema/SemaTemplate.cpp | 4 +-
clang/lib/Serialization/ASTReaderDecl.cpp | 4 +-
clang/lib/Serialization/ASTWriterDecl.cpp | 6 +-
.../ClangExtDefMapGen.cpp | 8 ++-
clang/tools/libclang/CIndex.cpp | 14 +++--
clang/tools/libclang/CXIndexDataConsumer.cpp | 56 +--
clang/unittests/AST/DeclTest.cpp | 8 +--
28 files changed, 189 insertions(+), 152 deletions(-)
diff --git a/clang-tools-extra/clang-doc/Serialize.cpp
b/clang-tools-extra/clang-doc/Serialize.cpp
index ac8e253ac06ea0b..3b074d849e8a9cf 100644
--- a/clang-tools-extra/clang-doc/Serialize.cpp
+++ b/clang-tools-extra/clang-doc/Serialize.cpp
@@ -257,8 +257,8 @@ static bool isPublic(const clang::AccessSpecifier AS,
const clang::Linkage Link) {
if (AS == clang::AccessSpecifier::AS_private)
return false;
- else if ((Link == clang::Linkage::ModuleLinkage) ||
- (Link == clang::Linkage::ExternalLinkage))
+ else if ((Link == clang::Linkage::Module) ||
+ (Link == clang::Linkage::External))
return true;
return false; // otherwise, linkage is some form of internal linkage
}
diff --git a/clang-tools-extra/clangd/Quality.cpp
b/clang-tools-extra/clangd/Quality.cpp
index 8840f805f0e87c7..7371d95fbf27547 100644
--- a/clang-tools-extra/clangd/Quality.cpp
+++ b/clang-tools-extra/clangd/Quality.cpp
@@ -274,7 +274,8 @@ computeScope(const NamedDecl *D) {
return SymbolRelevanceSignals::ClassScope;
// ExternalLinkage threshold could be tweaked, e.g. module-visible as global.
// Avoid caching linkage if it may change after enclosing code completion.
- if (hasUnstableLinkage(D) || D->getLinkageInternal() < ExternalLinkage)
+ if (hasUnstableLinkage(D) || llvm::to_underlying(D->getLinkageInternal()) <
+ llvm::to_underlying(Linkage::External))
return SymbolRelevanceSignals::FileScope;
return SymbolRelevanceSignals::GlobalScope;
}
diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp
b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index 7649e37e1f96663..49e479abf456210 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -617,7 +617,8 @@ std::optional scopeModifier(const
NamedDecl *D) {
if (DC->isTranslationUnit() && D->isTemplateParameter())
return std::nullopt;
// ExternalLinkage threshold could be tweaked, e.g. module-visible as global.
- if (D->getLinkageInternal() < ExternalLinkage)
+ if (llvm::to_underlying(D->getLinkageInternal()) <
+ llvm::to_underlying(Linkage::External))
return HighlightingModifier::FileScope;
return HighlightingModifier::GlobalScope;
}
diff --git a/clang/include/clang/AST/DeclBase.h
b/clang/include/clang/AST/DeclBase.h
index df1d6e8a3b5af72..f784fa73af5bad5 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -49,7 +49,7 @@ class
[clang] [clang-tools-extra] [clang] Refactor `clang::Linkage` (PR #71049)
https://github.com/AaronBallman approved this pull request. LGTM aside from the comment nit from @ChuanqiXu9 https://github.com/llvm/llvm-project/pull/71049 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang] Refactor `clang::Linkage` (PR #71049)
@@ -2214,7 +2214,7 @@ void ASTWriter::WriteDeclAbbrevs() {
Abv->Add(BitCodeAbbrevOp(0)); // TSCSpec
Abv->Add(BitCodeAbbrevOp(0)); // InitStyle
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isARCPseudoStrong
- Abv->Add(BitCodeAbbrevOp(0)); // Linkage
+ Abv->Add(BitCodeAbbrevOp(1)); // Linkage
AaronBallman wrote:
Excellent, thank you! Hmmm, we should probably put a comment there like "this
value was last bumped for the Clang NN release" so we don't have to do so much
work in the future... (Not as part of these changes, just thinking out loud.)
https://github.com/llvm/llvm-project/pull/71049
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang] Refactor `clang::Linkage` (PR #71049)
@@ -2214,7 +2214,7 @@ void ASTWriter::WriteDeclAbbrevs() {
Abv->Add(BitCodeAbbrevOp(0)); // TSCSpec
Abv->Add(BitCodeAbbrevOp(0)); // InitStyle
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isARCPseudoStrong
- Abv->Add(BitCodeAbbrevOp(0)); // Linkage
+ Abv->Add(BitCodeAbbrevOp(1)); // Linkage
Endilll wrote:
> Please check whether VERSION_MAJOR (in ASTBitCodes.h) has been updated for
> this release and if it hasn't been, increment it by one.
This has been bumped after 17 cut-off at least once:
https://github.com/llvm/llvm-project/commit/6fb08d8f558a6f28db7835acdb88cab83aea2eb4
https://github.com/llvm/llvm-project/pull/71049
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang] Refactor `clang::Linkage` (PR #71049)
@@ -2214,7 +2214,7 @@ void ASTWriter::WriteDeclAbbrevs() {
Abv->Add(BitCodeAbbrevOp(0)); // TSCSpec
Abv->Add(BitCodeAbbrevOp(0)); // InitStyle
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isARCPseudoStrong
- Abv->Add(BitCodeAbbrevOp(0)); // Linkage
+ Abv->Add(BitCodeAbbrevOp(1)); // Linkage
AaronBallman wrote:
O! Thanks. None makes far more sense now. Still a change to the
serialization values though, so we need to check whether we should do a version
bump or not.
https://github.com/llvm/llvm-project/pull/71049
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang] Refactor `clang::Linkage` (PR #71049)
@@ -2214,7 +2214,7 @@ void ASTWriter::WriteDeclAbbrevs() {
Abv->Add(BitCodeAbbrevOp(0)); // TSCSpec
Abv->Add(BitCodeAbbrevOp(0)); // InitStyle
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isARCPseudoStrong
- Abv->Add(BitCodeAbbrevOp(0)); // Linkage
+ Abv->Add(BitCodeAbbrevOp(1)); // Linkage
ChuanqiXu9 wrote:
> If I guess correctly that this value corresponds to Linkage::None we have in
> some default constructors, we should consider changing that default to
> Linkage::Invalid.
No. It is not the case. Sorry for didn't explaining things clearly. The value
here means the linkage for ParmVarDecl according to line
[2185](https://github.com/llvm/llvm-project/pull/71049/files#diff-125f472e690aa3d973bc42aa3c5d580226c5c47661551aca2889f960681aa64dR2185).
So it is natural that ParmVarDecl doesn't have a linkage.
https://github.com/llvm/llvm-project/pull/71049
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang] Refactor `clang::Linkage` (PR #71049)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/71049 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang] Refactor `clang::Linkage` (PR #71049)
https://github.com/ChuanqiXu9 approved this pull request. LGTM. I didn't understand the full context in the private chat. https://github.com/llvm/llvm-project/pull/71049 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang] Refactor `clang::Linkage` (PR #71049)
@@ -1921,7 +1920,20 @@ bool NamedDecl::declarationReplaces(NamedDecl *OldD,
bool IsKnownNewer) const {
}
bool NamedDecl::hasLinkage() const {
- return getFormalLinkage() != NoLinkage;
+ switch (getFormalLinkage()) {
+ case Linkage::Invalid:
+llvm_unreachable("Linkage hasn't been computed!");
+ case Linkage::None:
+return false;
+ case Linkage::Internal:
+return true;
+ case Linkage::UniqueExternal:
+ case Linkage::VisibleNone:
+llvm_unreachable("Non-formal linkage is not allowed here!");
AaronBallman wrote:
Ah I see the logic now, this is actually an NFC change. I think it's fine to
leave in as-is in this patch.
https://github.com/llvm/llvm-project/pull/71049
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang] Refactor `clang::Linkage` (PR #71049)
https://github.com/Endilll edited https://github.com/llvm/llvm-project/pull/71049 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
