-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[clang][NFC] Refactor clang::Linkage
#71049
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
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.
@llvm/pr-subscribers-clangd @llvm/pr-subscribers-clang-modules Author: Vlad Serebrennikov (Endilll) ChangesThis patch introduces a new enumerator
This patch makes debuggers to not be mistaken about enumerator stored in this bit-field. It also converts Patch is 40.18 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/71049.diff 28 Files Affected:
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<HighlightingModifier> 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 ExternalSourceSymbolAttr;
class FunctionDecl;
class FunctionType;
class IdentifierInfo;
-enum Linkage : unsigned char;
+enum class Linkage : unsigned char;
class LinkageSpecDecl;
class Module;
class NamedDecl;
@@ -335,7 +335,6 @@ class alignas(8) Decl {
unsigned IdentifierNamespace : 14;
/// If 0, we have not computed the linkage of this declaration.
- /// Otherwise, it is the linkage + 1.
mutable unsigned CacheValidAndLinkage : 3;
/// Allocate memory for a deserialized declaration.
@@ -386,7 +385,7 @@ class alignas(8) Decl {
Implicit(false), Used(false), Referenced(false),
TopLevelDeclInObjCContainer(false), Access(AS_none), FromASTFile(0),
IdentifierNamespace(getIdentifierNamespaceForKind(DK)),
- CacheValidAndLinkage(0) {
+ CacheValidAndLinkage(llvm::to_underlying(Linkage::Invalid)) {
if (StatisticsEnabled) add(DK);
}
@@ -395,7 +394,7 @@ class alignas(8) Decl {
Used(false), Referenced(false), TopLevelDeclInObjCContainer(false),
Access(AS_none), FromASTFile(0),
IdentifierNamespace(getIdentifierNamespaceForKind(DK)),
- CacheValidAndLinkage(0) {
+ CacheValidAndLinkage(llvm::to_underlying(Linkage::Invalid)) {
if (StatisticsEnabled) add(DK);
}
@@ -405,11 +404,11 @@ class alignas(8) Decl {
void updateOutOfDate(IdentifierInfo &II) const;
Linkage getCachedLinkage() const {
- return Linkage(CacheValidAndLinkage - 1);
+ return static_cast<Linkage>(CacheValidAndLinkage);
}
void setCachedLinkage(Linkage L) const {
- CacheValidAndLinkage = L + 1;
+ CacheValidAndLinkage = llvm::to_underlying(L);
}
bool hasCachedLinkage() const {
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 23022d7e1a72928..96c12a84e139f40 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -1996,7 +1996,7 @@ class alignas(TypeAlignment) Type : public ExtQualsTypeCommonBase {
TypeBits.Dependence = static_cast<unsigned>(Dependence);
TypeBits.CacheValid = false;
TypeBits.CachedLocalOrUnnamed = false;
- TypeBits.CachedLinkage = NoLinkage;
+ TypeBits.CachedLinkage = llvm::to_underlying(Linkage::None);
TypeBits.FromAST = false;
}
diff --git a/clang/include/clang/Basic/Linkage.h b/clang/include/clang/Basic/Linkage.h
index 0b7b61954a075ae..9cf36e522947fa8 100644
--- a/clang/include/clang/Basic/Linkage.h
+++ b/clang/include/clang/Basic/Linkage.h
@@ -14,21 +14,25 @@
#ifndef LLVM_CLANG_BASIC_LINKAGE_H
#define LLVM_CLANG_BASIC_LINKAGE_H
+#include "llvm/Support/ErrorHandling.h"
#include <utility>
namespace clang {
/// Describes the different kinds of linkage
/// (C++ [basic.link], C99 6.2.2) that an entity may have.
-enum Linkage : unsigned char {
+enum class Linkage : unsigned char {
+ // Linkage hasn't been computed.
+ Invalid = 0,
+
/// No linkage, which means that the entity is unique and
/// can only be referred to from within its scope.
- NoLinkage = 0,
+ None,
/// Internal linkage, which indicates that the entity can
/// be referred to from within the translation unit (but not other
/// translation units).
- InternalLinkage,
+ Internal,
/// External linkage within a unique namespace.
///
@@ -37,21 +41,21 @@ enum Linkage : unsigned char {
/// their names are unique to this translation unit, which is
/// equivalent to having internal linkage from the code-generation
/// point of view.
- UniqueExternalLinkage,
+ UniqueExternal,
/// No linkage according to the standard, but is visible from other
/// translation units because of types defined in a inline function.
- VisibleNoLinkage,
+ VisibleNone,
/// Module linkage, which indicates that the entity can be referred
/// to from other translation units within the same module, and indirectly
/// from arbitrary other translation units through inline functions and
/// templates in the module interface.
- ModuleLinkage,
+ Module,
/// External linkage, which indicates that the entity can
/// be referred to from other translation units.
- ExternalLinkage
+ External
};
/// Describes the different kinds of language linkage
@@ -84,22 +88,33 @@ inline bool isUniqueGVALinkage(GVALinkage L) {
}
inline bool isExternallyVisible(Linkage L) {
- return L >= VisibleNoLinkage;
+ switch (L) {
+ case Linkage::Invalid:
+ llvm_unreachable("Linkage hasn't been computed!");
+ case Linkage::None:
+ case Linkage::Internal:
+ case Linkage::UniqueExternal:
+ return false;
+ case Linkage::VisibleNone:
+ case Linkage::Module:
+ case Linkage::External:
+ return true;
+ }
}
inline Linkage getFormalLinkage(Linkage L) {
switch (L) {
- case UniqueExternalLinkage:
- return ExternalLinkage;
- case VisibleNoLinkage:
- return NoLinkage;
+ case Linkage::UniqueExternal:
+ return Linkage::External;
+ case Linkage::VisibleNone:
+ return Linkage::None;
default:
return L;
}
}
inline bool isExternalFormalLinkage(Linkage L) {
- return getFormalLinkage(L) == ExternalLinkage;
+ return getFormalLinkage(L) == Linkage::External;
}
/// Compute the minimum linkage given two linkages.
@@ -111,13 +126,13 @@ inline bool isExternalFormalLinkage(Linkage L) {
/// special cases for when VisibleNoLinkage would lose the visible bit and
/// become NoLinkage.
inline Linkage minLinkage(Linkage L1, Linkage L2) {
- if (L2 == VisibleNoLinkage)
+ if (L2 == Linkage::VisibleNone)
std::swap(L1, L2);
- if (L1 == VisibleNoLinkage) {
- if (L2 == InternalLinkage)
- return NoLinkage;
- if (L2 == UniqueExternalLinkage)
- return NoLinkage;
+ if (L1 == Linkage::VisibleNone) {
+ if (L2 == Linkage::Internal)
+ return Linkage::None;
+ if (L2 == Linkage::UniqueExternal)
+ return Linkage::None;
}
return L1 < L2 ? L1 : L2;
}
diff --git a/clang/include/clang/Basic/Visibility.h b/clang/include/clang/Basic/Visibility.h
index 57d9754ae4a9858..1e196300be421eb 100644
--- a/clang/include/clang/Basic/Visibility.h
+++ b/clang/include/clang/Basic/Visibility.h
@@ -15,6 +15,7 @@
#define LLVM_CLANG_BASIC_VISIBILITY_H
#include "clang/Basic/Linkage.h"
+#include "llvm/ADT/STLForwardCompat.h"
#include <cassert>
#include <cstdint>
@@ -56,10 +57,11 @@ class LinkageInfo {
void setVisibility(Visibility V, bool E) { visibility_ = V; explicit_ = E; }
public:
- LinkageInfo() : linkage_(ExternalLinkage), visibility_(DefaultVisibility),
- explicit_(false) {}
+ LinkageInfo()
+ : linkage_(llvm::to_underlying(Linkage::External)),
+ visibility_(DefaultVisibility), explicit_(false) {}
LinkageInfo(Linkage L, Visibility V, bool E)
- : linkage_(L), visibility_(V), explicit_(E) {
+ : linkage_(llvm::to_underlying(L)), visibility_(V), explicit_(E) {
assert(getLinkage() == L && getVisibility() == V &&
isVisibilityExplicit() == E && "Enum truncated!");
}
@@ -68,23 +70,23 @@ class LinkageInfo {
return LinkageInfo();
}
static LinkageInfo internal() {
- return LinkageInfo(InternalLinkage, DefaultVisibility, false);
+ return LinkageInfo(Linkage::Internal, DefaultVisibility, false);
}
static LinkageInfo uniqueExternal() {
- return LinkageInfo(UniqueExternalLinkage, DefaultVisibility, false);
+ return LinkageInfo(Linkage::UniqueExternal, DefaultVisibility, false);
}
static LinkageInfo none() {
- return LinkageInfo(NoLinkage, DefaultVisibility, false);
+ return LinkageInfo(Linkage::None, DefaultVisibility, false);
}
static LinkageInfo visible_none() {
- return LinkageInfo(VisibleNoLinkage, DefaultVisibility, false);
+ return LinkageInfo(Linkage::VisibleNone, DefaultVisibility, false);
}
- Linkage getLinkage() const { return (Linkage)linkage_; }
+ Linkage getLinkage() const { return static_cast<Linkage>(linkage_); }
Visibility getVisibility() const { return (Visibility)visibility_; }
bool isVisibilityExplicit() const { return explicit_; }
- void setLinkage(Linkage L) { linkage_ = L; }
+ void setLinkage(Linkage L) { linkage_ = llvm::to_underlying(L); }
void mergeLinkage(Linkage L) {
setLinkage(minLinkage(getLinkage(), L));
@@ -96,10 +98,10 @@ class LinkageInfo {
void mergeExternalVisibility(Linkage L) {
Linkage ThisL = getLinkage();
if (!isExternallyVisible(L)) {
- if (ThisL == VisibleNoLinkage)
- ThisL = NoLinkage;
- else if (ThisL == ExternalLinkage)
- ThisL = UniqueExternalLinkage;
+ if (ThisL == Linkage::VisibleNone)
+ ThisL = Linkage::None;
+ else if (ThisL == Linkage::External)
+ ThisL = Linkage::UniqueExternal;
}
setLinkage(ThisL);
}
diff --git a/clang/lib/AST/APValue.cpp b/clang/lib/AST/APValue.cpp
index d08c2936b56dd45..4eae308ef5b34c6 100644
--- a/clang/lib/AST/APValue.cpp
+++ b/clang/lib/AST/APValue.cpp
@@ -1115,7 +1115,7 @@ LinkageInfo LinkageComputer::getLVForValue(const APValue &V,
auto MergeLV = [&](LinkageInfo MergeLV) {
LV.merge(MergeLV);
- return LV.getLinkage() == InternalLinkage;
+ return LV.getLinkage() == Linkage::Internal;
};
auto Merge = [&](const APValue &V) {
return MergeLV(getLVForValue(V, computation));
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 6efc177d61c03ba..e8062b680fbc3ab 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -1204,12 +1204,11 @@ Linkage NamedDecl::getFormalLinkage() const {
// [basic.namespace.general]/p2
// A namespace is never attached to a named module and never has a name with
// module linkage.
- if (isInModulePurview(this) &&
- InternalLinkage == ExternalLinkage &&
+ if (isInModulePurview(this) && InternalLinkage == Linkage::External &&
!isExportedFromModuleInterfaceUnit(
cast<NamedDecl>(this->getCanonicalDecl())) &&
!isa<NamespaceDecl>(this))
- InternalLinkage = ModuleLinkage;
+ InternalLinkage = Linkage::Module;
return clang::getFormalLinkage(InternalLinkage);
}
@@ -1337,7 +1336,7 @@ LinkageInfo LinkageComputer::getLVForClosure(const DeclContext *DC,
// visible, then the lambda is too. We apply the same rules to blocks.
if (!isExternallyVisible(OwnerLV.getLinkage()))
return LinkageInfo::none();
- return LinkageInfo(VisibleNoLinkage, OwnerLV.getVisibility(),
+ return LinkageInfo(Linkage::VisibleNone, OwnerLV.getVisibility(),
OwnerLV.isVisibilityExplicit());
}
@@ -1382,7 +1381,7 @@ LinkageInfo LinkageComputer::getLVForLocalDecl(const NamedDecl *D,
if (const VarDecl *Prev = Var->getPreviousDecl()) {
LinkageInfo PrevLV = getLVForDecl(Prev, computation);
- if (PrevLV.getLinkage())
+ if (PrevLV.getLinkage() != Linkage::Invalid)
LV.setLinkage(PrevLV.getLinkage());
LV.mergeVisibility(PrevLV);
}
@@ -1433,14 +1432,14 @@ LinkageInfo LinkageComputer::getLVForLocalDecl(const NamedDecl *D,
computation.isValueVisibility()
? Context.getLangOpts().getValueVisibilityMode()
: Context.getLangOpts().getTypeVisibilityMode();
- return LinkageInfo(VisibleNoLinkage, globalVisibility,
+ return LinkageInfo(Linkage::VisibleNone, globalVisibility,
/*visibilityExplicit=*/false);
}
}
}
if (!isExternallyVisible(LV.getLinkage()))
return LinkageInfo::none();
- return LinkageInfo(VisibleNoLinkage, LV.getVisibility(),
+ return LinkageInfo(Linkage::VisibleNone, LV.getVisibility(),
LV.isVisibilityExplicit());
}
@@ -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!");
+ case Linkage::Module:
+ case Linkage::External:
+ return true;
+ }
}
NamedDecl *NamedDecl::getUnderlyingDeclImpl() {
diff --git a/clang/lib/AST/ItaniumMangle.cpp b/clang/lib/AST/ItaniumMangle.cpp
index 261a56c4b666ae5..8530675ca2a1ce2 100644
--- a/clang/lib/AST/ItaniumMangle.cpp
+++ b/clang/lib/AST/ItaniumMangle.cpp
@@ -708,7 +708,7 @@ ItaniumMangleContextImpl::getEffectiveDeclContext(const Decl *D) {
}
bool ItaniumMangleContextImpl::isInternalLinkageDecl(const NamedDecl *ND) {
- if (ND && ND->getFormalLinkage() == InternalLinkage &&
+ if (ND && ND->getFormalLinkage() == Linkage::Internal &&
!ND->isExternallyVisible() &&
getEffectiveDeclContext(ND)->isFileContext() &&
!ND->isInAnonymousNamespace())
@@ -790,7 +790,7 @@ bool ItaniumMangleContextImpl::shouldMangleCXXName(const NamedDecl *D) {
if (DC->isFunctionOrMethod() && D->hasLinkage())
while (!DC->isFileContext())
DC = getEffectiveParentContext(DC);
- if (DC->isTranslationUnit() && D->getFormalLinkage() != InternalLinkage &&
+ if (DC->isTranslationUnit() && D->getFormalLinkage() != Linkage::Internal &&
!CXXNameMangler::shouldHaveAbiTags(*this, VD) &&
!isa<VarTemplateSpecializationDecl>(VD) &&
!VD->getOwningModuleForLinkage())
diff --git a/clang/lib/AST/MicrosoftMangle.cpp b/clang/lib/AST/MicrosoftMangle.cpp
index f6d8cdee8443d59..b7cfcbc9dfa4677 100644
--- a/clang/lib/AST/MicrosoftMangle.cpp
+++ b/clang/lib/AST/MicrosoftMangle.cpp
@@ -539,9 +539,8 @@ bool MicrosoftMangleContextImpl::shouldMangleCXXName(const NamedDecl *D) {
while (!DC->isNamespace() && !DC->isTranslationUnit())
DC = getEffectiveParentContext(DC);
- if (DC->isTranslationUnit() && D->getFormalLinkage() == InternalLinkage &&
- !isa<VarTemplateSpecializationDecl>(D) &&
- D->getIdentifier() != nullptr)
+ if (DC->isTranslationUnit() && D->getFormalLinkage() == Linkage::Internal &&
+ !isa<VarTemplateSpecializationDecl>(D) && D->getIdentifier() != nullptr)
return false;
}
diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index 98a4f12c4f574fa..d1cbfbd150ba53f 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -4175,7 +4175,7 @@ template <class Private> class TypePropertyCache {
// Compute the cached properties and then set the cache.
CachedProperties Result = computeCachedProperties(T);
T->TypeBits.CacheValid = true;
- T->TypeBits.CachedLinkage = Result.getLinkage();
+ T->TypeBits.CachedLinkage = llvm::to_underlying(Result.getLinkage());
T->TypeBits.CachedLocalOrUnnamed = Result.hasLocalOrUnnamedType();
}
};
@@ -4207,20 +4207,20 @@ static CachedProperties computeCachedProperties(const Type *T) {
// Treat instantiation-dependent types as external.
if (!T->isInstantiationDependentType()) T->dump();
assert(T->isInstantiationDependentType());
- return CachedProperties(ExternalLinkage, false);
+ return CachedProperties(Linkage::External, false);
case Type::Auto:
case Type::DeducedTemplateSpecialization:
// Give non-deduced 'auto' types external linkage. We should only see them
// here in error recovery.
- return CachedProperties(ExternalLinkage, false);
+ return CachedProperties(Linkage::External, false);
case Type::BitInt:
case Type::Builtin:
// C++ [basic.link]p8:
// A type is said to have linkage if and only if:
// - it is a fundamental type (3.9.1); or
- return CachedProperties(ExternalLinkage, false);
+ return CachedProperties(Linkage::External, false);
case Type::Record:
case Type::Enum: {
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index cc81a68b15c4324..35f651b39f6748a 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -3997,7 +3997,7 @@ TargetMVPriority(const TargetInfo &TI,
llvm::GlobalValue::LinkageTypes getMultiversionLinkage(CodeGenModule &CGM,
GlobalDecl GD) {
const FunctionDecl *FD = cast<FunctionDecl>(GD.getDecl());
- if (FD->getFormalLinkage() == InternalLinkage)
+ if (FD->getFormalLinkage() == Linkage::Internal)
return llvm::GlobalValue::InternalLinkage;
return llvm::GlobalValue::WeakODRLinkage;
}
@@ -5051,7 +5051,7 @@ void CodeGenModule::MaybeHandleStaticInExternC(const SomeDecl *D,
return;
// Must have internal linkage and an ordinary name.
- if (!D->getIdentifier() || D->getFormalLinkage() != InternalLinkage)
+ if (!D->getIdentifier() || D->getFormalLinkage() != Linkage::Internal)
return;
// Must be in an extern "C" context. Entities declared directly within
diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index c7295b3144ed16e..55d2696750ae7a0 100644
--- a/clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -3706,14 +3706,17 @@ static llvm::GlobalVariable::LinkageTypes getTypeInfoLinkage(CodeGenModule &CGM,
return llvm::GlobalValue::InternalLinkage;
switch (Ty->getLinkage()) {
- case NoLinkage:
- case InternalLinkage:
- case UniqueExternalLinkage:
+ case Linkage::Invalid:
+ llvm_unreachable("Linkage hasn't been computed!");
+
+ case Linkage::None:
+ case Linkage::Internal:
+ case Linkage::UniqueExternal:
return llvm::GlobalValue::InternalLinkage;
- case VisibleNoLinkage:
- case ModuleLinkage:
- case ExternalLinkage:
+ case Linkage::VisibleNone:
+ case Linkage::Module:
+ case Linkage::External:
// RTTI is not enabled, which means that this type info struct is going
// to be used for exception handling. Give it linkonce_odr linkage.
if ...
[truncated]
|
@llvm/pr-subscribers-clang-codegen Author: Vlad Serebrennikov (Endilll) ChangesThis patch introduces a new enumerator
This patch makes debuggers to not be mistaken about enumerator stored in this bit-field. It also converts Patch is 40.18 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/71049.diff 28 Files Affected:
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<HighlightingModifier> 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 ExternalSourceSymbolAttr;
class FunctionDecl;
class FunctionType;
class IdentifierInfo;
-enum Linkage : unsigned char;
+enum class Linkage : unsigned char;
class LinkageSpecDecl;
class Module;
class NamedDecl;
@@ -335,7 +335,6 @@ class alignas(8) Decl {
unsigned IdentifierNamespace : 14;
/// If 0, we have not computed the linkage of this declaration.
- /// Otherwise, it is the linkage + 1.
mutable unsigned CacheValidAndLinkage : 3;
/// Allocate memory for a deserialized declaration.
@@ -386,7 +385,7 @@ class alignas(8) Decl {
Implicit(false), Used(false), Referenced(false),
TopLevelDeclInObjCContainer(false), Access(AS_none), FromASTFile(0),
IdentifierNamespace(getIdentifierNamespaceForKind(DK)),
- CacheValidAndLinkage(0) {
+ CacheValidAndLinkage(llvm::to_underlying(Linkage::Invalid)) {
if (StatisticsEnabled) add(DK);
}
@@ -395,7 +394,7 @@ class alignas(8) Decl {
Used(false), Referenced(false), TopLevelDeclInObjCContainer(false),
Access(AS_none), FromASTFile(0),
IdentifierNamespace(getIdentifierNamespaceForKind(DK)),
- CacheValidAndLinkage(0) {
+ CacheValidAndLinkage(llvm::to_underlying(Linkage::Invalid)) {
if (StatisticsEnabled) add(DK);
}
@@ -405,11 +404,11 @@ class alignas(8) Decl {
void updateOutOfDate(IdentifierInfo &II) const;
Linkage getCachedLinkage() const {
- return Linkage(CacheValidAndLinkage - 1);
+ return static_cast<Linkage>(CacheValidAndLinkage);
}
void setCachedLinkage(Linkage L) const {
- CacheValidAndLinkage = L + 1;
+ CacheValidAndLinkage = llvm::to_underlying(L);
}
bool hasCachedLinkage() const {
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 23022d7e1a72928..96c12a84e139f40 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -1996,7 +1996,7 @@ class alignas(TypeAlignment) Type : public ExtQualsTypeCommonBase {
TypeBits.Dependence = static_cast<unsigned>(Dependence);
TypeBits.CacheValid = false;
TypeBits.CachedLocalOrUnnamed = false;
- TypeBits.CachedLinkage = NoLinkage;
+ TypeBits.CachedLinkage = llvm::to_underlying(Linkage::None);
TypeBits.FromAST = false;
}
diff --git a/clang/include/clang/Basic/Linkage.h b/clang/include/clang/Basic/Linkage.h
index 0b7b61954a075ae..9cf36e522947fa8 100644
--- a/clang/include/clang/Basic/Linkage.h
+++ b/clang/include/clang/Basic/Linkage.h
@@ -14,21 +14,25 @@
#ifndef LLVM_CLANG_BASIC_LINKAGE_H
#define LLVM_CLANG_BASIC_LINKAGE_H
+#include "llvm/Support/ErrorHandling.h"
#include <utility>
namespace clang {
/// Describes the different kinds of linkage
/// (C++ [basic.link], C99 6.2.2) that an entity may have.
-enum Linkage : unsigned char {
+enum class Linkage : unsigned char {
+ // Linkage hasn't been computed.
+ Invalid = 0,
+
/// No linkage, which means that the entity is unique and
/// can only be referred to from within its scope.
- NoLinkage = 0,
+ None,
/// Internal linkage, which indicates that the entity can
/// be referred to from within the translation unit (but not other
/// translation units).
- InternalLinkage,
+ Internal,
/// External linkage within a unique namespace.
///
@@ -37,21 +41,21 @@ enum Linkage : unsigned char {
/// their names are unique to this translation unit, which is
/// equivalent to having internal linkage from the code-generation
/// point of view.
- UniqueExternalLinkage,
+ UniqueExternal,
/// No linkage according to the standard, but is visible from other
/// translation units because of types defined in a inline function.
- VisibleNoLinkage,
+ VisibleNone,
/// Module linkage, which indicates that the entity can be referred
/// to from other translation units within the same module, and indirectly
/// from arbitrary other translation units through inline functions and
/// templates in the module interface.
- ModuleLinkage,
+ Module,
/// External linkage, which indicates that the entity can
/// be referred to from other translation units.
- ExternalLinkage
+ External
};
/// Describes the different kinds of language linkage
@@ -84,22 +88,33 @@ inline bool isUniqueGVALinkage(GVALinkage L) {
}
inline bool isExternallyVisible(Linkage L) {
- return L >= VisibleNoLinkage;
+ switch (L) {
+ case Linkage::Invalid:
+ llvm_unreachable("Linkage hasn't been computed!");
+ case Linkage::None:
+ case Linkage::Internal:
+ case Linkage::UniqueExternal:
+ return false;
+ case Linkage::VisibleNone:
+ case Linkage::Module:
+ case Linkage::External:
+ return true;
+ }
}
inline Linkage getFormalLinkage(Linkage L) {
switch (L) {
- case UniqueExternalLinkage:
- return ExternalLinkage;
- case VisibleNoLinkage:
- return NoLinkage;
+ case Linkage::UniqueExternal:
+ return Linkage::External;
+ case Linkage::VisibleNone:
+ return Linkage::None;
default:
return L;
}
}
inline bool isExternalFormalLinkage(Linkage L) {
- return getFormalLinkage(L) == ExternalLinkage;
+ return getFormalLinkage(L) == Linkage::External;
}
/// Compute the minimum linkage given two linkages.
@@ -111,13 +126,13 @@ inline bool isExternalFormalLinkage(Linkage L) {
/// special cases for when VisibleNoLinkage would lose the visible bit and
/// become NoLinkage.
inline Linkage minLinkage(Linkage L1, Linkage L2) {
- if (L2 == VisibleNoLinkage)
+ if (L2 == Linkage::VisibleNone)
std::swap(L1, L2);
- if (L1 == VisibleNoLinkage) {
- if (L2 == InternalLinkage)
- return NoLinkage;
- if (L2 == UniqueExternalLinkage)
- return NoLinkage;
+ if (L1 == Linkage::VisibleNone) {
+ if (L2 == Linkage::Internal)
+ return Linkage::None;
+ if (L2 == Linkage::UniqueExternal)
+ return Linkage::None;
}
return L1 < L2 ? L1 : L2;
}
diff --git a/clang/include/clang/Basic/Visibility.h b/clang/include/clang/Basic/Visibility.h
index 57d9754ae4a9858..1e196300be421eb 100644
--- a/clang/include/clang/Basic/Visibility.h
+++ b/clang/include/clang/Basic/Visibility.h
@@ -15,6 +15,7 @@
#define LLVM_CLANG_BASIC_VISIBILITY_H
#include "clang/Basic/Linkage.h"
+#include "llvm/ADT/STLForwardCompat.h"
#include <cassert>
#include <cstdint>
@@ -56,10 +57,11 @@ class LinkageInfo {
void setVisibility(Visibility V, bool E) { visibility_ = V; explicit_ = E; }
public:
- LinkageInfo() : linkage_(ExternalLinkage), visibility_(DefaultVisibility),
- explicit_(false) {}
+ LinkageInfo()
+ : linkage_(llvm::to_underlying(Linkage::External)),
+ visibility_(DefaultVisibility), explicit_(false) {}
LinkageInfo(Linkage L, Visibility V, bool E)
- : linkage_(L), visibility_(V), explicit_(E) {
+ : linkage_(llvm::to_underlying(L)), visibility_(V), explicit_(E) {
assert(getLinkage() == L && getVisibility() == V &&
isVisibilityExplicit() == E && "Enum truncated!");
}
@@ -68,23 +70,23 @@ class LinkageInfo {
return LinkageInfo();
}
static LinkageInfo internal() {
- return LinkageInfo(InternalLinkage, DefaultVisibility, false);
+ return LinkageInfo(Linkage::Internal, DefaultVisibility, false);
}
static LinkageInfo uniqueExternal() {
- return LinkageInfo(UniqueExternalLinkage, DefaultVisibility, false);
+ return LinkageInfo(Linkage::UniqueExternal, DefaultVisibility, false);
}
static LinkageInfo none() {
- return LinkageInfo(NoLinkage, DefaultVisibility, false);
+ return LinkageInfo(Linkage::None, DefaultVisibility, false);
}
static LinkageInfo visible_none() {
- return LinkageInfo(VisibleNoLinkage, DefaultVisibility, false);
+ return LinkageInfo(Linkage::VisibleNone, DefaultVisibility, false);
}
- Linkage getLinkage() const { return (Linkage)linkage_; }
+ Linkage getLinkage() const { return static_cast<Linkage>(linkage_); }
Visibility getVisibility() const { return (Visibility)visibility_; }
bool isVisibilityExplicit() const { return explicit_; }
- void setLinkage(Linkage L) { linkage_ = L; }
+ void setLinkage(Linkage L) { linkage_ = llvm::to_underlying(L); }
void mergeLinkage(Linkage L) {
setLinkage(minLinkage(getLinkage(), L));
@@ -96,10 +98,10 @@ class LinkageInfo {
void mergeExternalVisibility(Linkage L) {
Linkage ThisL = getLinkage();
if (!isExternallyVisible(L)) {
- if (ThisL == VisibleNoLinkage)
- ThisL = NoLinkage;
- else if (ThisL == ExternalLinkage)
- ThisL = UniqueExternalLinkage;
+ if (ThisL == Linkage::VisibleNone)
+ ThisL = Linkage::None;
+ else if (ThisL == Linkage::External)
+ ThisL = Linkage::UniqueExternal;
}
setLinkage(ThisL);
}
diff --git a/clang/lib/AST/APValue.cpp b/clang/lib/AST/APValue.cpp
index d08c2936b56dd45..4eae308ef5b34c6 100644
--- a/clang/lib/AST/APValue.cpp
+++ b/clang/lib/AST/APValue.cpp
@@ -1115,7 +1115,7 @@ LinkageInfo LinkageComputer::getLVForValue(const APValue &V,
auto MergeLV = [&](LinkageInfo MergeLV) {
LV.merge(MergeLV);
- return LV.getLinkage() == InternalLinkage;
+ return LV.getLinkage() == Linkage::Internal;
};
auto Merge = [&](const APValue &V) {
return MergeLV(getLVForValue(V, computation));
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 6efc177d61c03ba..e8062b680fbc3ab 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -1204,12 +1204,11 @@ Linkage NamedDecl::getFormalLinkage() const {
// [basic.namespace.general]/p2
// A namespace is never attached to a named module and never has a name with
// module linkage.
- if (isInModulePurview(this) &&
- InternalLinkage == ExternalLinkage &&
+ if (isInModulePurview(this) && InternalLinkage == Linkage::External &&
!isExportedFromModuleInterfaceUnit(
cast<NamedDecl>(this->getCanonicalDecl())) &&
!isa<NamespaceDecl>(this))
- InternalLinkage = ModuleLinkage;
+ InternalLinkage = Linkage::Module;
return clang::getFormalLinkage(InternalLinkage);
}
@@ -1337,7 +1336,7 @@ LinkageInfo LinkageComputer::getLVForClosure(const DeclContext *DC,
// visible, then the lambda is too. We apply the same rules to blocks.
if (!isExternallyVisible(OwnerLV.getLinkage()))
return LinkageInfo::none();
- return LinkageInfo(VisibleNoLinkage, OwnerLV.getVisibility(),
+ return LinkageInfo(Linkage::VisibleNone, OwnerLV.getVisibility(),
OwnerLV.isVisibilityExplicit());
}
@@ -1382,7 +1381,7 @@ LinkageInfo LinkageComputer::getLVForLocalDecl(const NamedDecl *D,
if (const VarDecl *Prev = Var->getPreviousDecl()) {
LinkageInfo PrevLV = getLVForDecl(Prev, computation);
- if (PrevLV.getLinkage())
+ if (PrevLV.getLinkage() != Linkage::Invalid)
LV.setLinkage(PrevLV.getLinkage());
LV.mergeVisibility(PrevLV);
}
@@ -1433,14 +1432,14 @@ LinkageInfo LinkageComputer::getLVForLocalDecl(const NamedDecl *D,
computation.isValueVisibility()
? Context.getLangOpts().getValueVisibilityMode()
: Context.getLangOpts().getTypeVisibilityMode();
- return LinkageInfo(VisibleNoLinkage, globalVisibility,
+ return LinkageInfo(Linkage::VisibleNone, globalVisibility,
/*visibilityExplicit=*/false);
}
}
}
if (!isExternallyVisible(LV.getLinkage()))
return LinkageInfo::none();
- return LinkageInfo(VisibleNoLinkage, LV.getVisibility(),
+ return LinkageInfo(Linkage::VisibleNone, LV.getVisibility(),
LV.isVisibilityExplicit());
}
@@ -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!");
+ case Linkage::Module:
+ case Linkage::External:
+ return true;
+ }
}
NamedDecl *NamedDecl::getUnderlyingDeclImpl() {
diff --git a/clang/lib/AST/ItaniumMangle.cpp b/clang/lib/AST/ItaniumMangle.cpp
index 261a56c4b666ae5..8530675ca2a1ce2 100644
--- a/clang/lib/AST/ItaniumMangle.cpp
+++ b/clang/lib/AST/ItaniumMangle.cpp
@@ -708,7 +708,7 @@ ItaniumMangleContextImpl::getEffectiveDeclContext(const Decl *D) {
}
bool ItaniumMangleContextImpl::isInternalLinkageDecl(const NamedDecl *ND) {
- if (ND && ND->getFormalLinkage() == InternalLinkage &&
+ if (ND && ND->getFormalLinkage() == Linkage::Internal &&
!ND->isExternallyVisible() &&
getEffectiveDeclContext(ND)->isFileContext() &&
!ND->isInAnonymousNamespace())
@@ -790,7 +790,7 @@ bool ItaniumMangleContextImpl::shouldMangleCXXName(const NamedDecl *D) {
if (DC->isFunctionOrMethod() && D->hasLinkage())
while (!DC->isFileContext())
DC = getEffectiveParentContext(DC);
- if (DC->isTranslationUnit() && D->getFormalLinkage() != InternalLinkage &&
+ if (DC->isTranslationUnit() && D->getFormalLinkage() != Linkage::Internal &&
!CXXNameMangler::shouldHaveAbiTags(*this, VD) &&
!isa<VarTemplateSpecializationDecl>(VD) &&
!VD->getOwningModuleForLinkage())
diff --git a/clang/lib/AST/MicrosoftMangle.cpp b/clang/lib/AST/MicrosoftMangle.cpp
index f6d8cdee8443d59..b7cfcbc9dfa4677 100644
--- a/clang/lib/AST/MicrosoftMangle.cpp
+++ b/clang/lib/AST/MicrosoftMangle.cpp
@@ -539,9 +539,8 @@ bool MicrosoftMangleContextImpl::shouldMangleCXXName(const NamedDecl *D) {
while (!DC->isNamespace() && !DC->isTranslationUnit())
DC = getEffectiveParentContext(DC);
- if (DC->isTranslationUnit() && D->getFormalLinkage() == InternalLinkage &&
- !isa<VarTemplateSpecializationDecl>(D) &&
- D->getIdentifier() != nullptr)
+ if (DC->isTranslationUnit() && D->getFormalLinkage() == Linkage::Internal &&
+ !isa<VarTemplateSpecializationDecl>(D) && D->getIdentifier() != nullptr)
return false;
}
diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index 98a4f12c4f574fa..d1cbfbd150ba53f 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -4175,7 +4175,7 @@ template <class Private> class TypePropertyCache {
// Compute the cached properties and then set the cache.
CachedProperties Result = computeCachedProperties(T);
T->TypeBits.CacheValid = true;
- T->TypeBits.CachedLinkage = Result.getLinkage();
+ T->TypeBits.CachedLinkage = llvm::to_underlying(Result.getLinkage());
T->TypeBits.CachedLocalOrUnnamed = Result.hasLocalOrUnnamedType();
}
};
@@ -4207,20 +4207,20 @@ static CachedProperties computeCachedProperties(const Type *T) {
// Treat instantiation-dependent types as external.
if (!T->isInstantiationDependentType()) T->dump();
assert(T->isInstantiationDependentType());
- return CachedProperties(ExternalLinkage, false);
+ return CachedProperties(Linkage::External, false);
case Type::Auto:
case Type::DeducedTemplateSpecialization:
// Give non-deduced 'auto' types external linkage. We should only see them
// here in error recovery.
- return CachedProperties(ExternalLinkage, false);
+ return CachedProperties(Linkage::External, false);
case Type::BitInt:
case Type::Builtin:
// C++ [basic.link]p8:
// A type is said to have linkage if and only if:
// - it is a fundamental type (3.9.1); or
- return CachedProperties(ExternalLinkage, false);
+ return CachedProperties(Linkage::External, false);
case Type::Record:
case Type::Enum: {
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index cc81a68b15c4324..35f651b39f6748a 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -3997,7 +3997,7 @@ TargetMVPriority(const TargetInfo &TI,
llvm::GlobalValue::LinkageTypes getMultiversionLinkage(CodeGenModule &CGM,
GlobalDecl GD) {
const FunctionDecl *FD = cast<FunctionDecl>(GD.getDecl());
- if (FD->getFormalLinkage() == InternalLinkage)
+ if (FD->getFormalLinkage() == Linkage::Internal)
return llvm::GlobalValue::InternalLinkage;
return llvm::GlobalValue::WeakODRLinkage;
}
@@ -5051,7 +5051,7 @@ void CodeGenModule::MaybeHandleStaticInExternC(const SomeDecl *D,
return;
// Must have internal linkage and an ordinary name.
- if (!D->getIdentifier() || D->getFormalLinkage() != InternalLinkage)
+ if (!D->getIdentifier() || D->getFormalLinkage() != Linkage::Internal)
return;
// Must be in an extern "C" context. Entities declared directly within
diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index c7295b3144ed16e..55d2696750ae7a0 100644
--- a/clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -3706,14 +3706,17 @@ static llvm::GlobalVariable::LinkageTypes getTypeInfoLinkage(CodeGenModule &CGM,
return llvm::GlobalValue::InternalLinkage;
switch (Ty->getLinkage()) {
- case NoLinkage:
- case InternalLinkage:
- case UniqueExternalLinkage:
+ case Linkage::Invalid:
+ llvm_unreachable("Linkage hasn't been computed!");
+
+ case Linkage::None:
+ case Linkage::Internal:
+ case Linkage::UniqueExternal:
return llvm::GlobalValue::InternalLinkage;
- case VisibleNoLinkage:
- case ModuleLinkage:
- case ExternalLinkage:
+ case Linkage::VisibleNone:
+ case Linkage::Module:
+ case Linkage::External:
// RTTI is not enabled, which means that this type info struct is going
// to be used for exception handling. Give it linkonce_odr linkage.
if ...
[truncated]
|
@@ -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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ChuanqiXu9 confirmed that touching this is fine (thank you!). 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
.
Linkage::None
being default feels like we computed the linkage and arrived at Linkage::None
even when it's not the case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed that those should be swapped to Invalid
instead of None
(better done as a separate follow-up patch as it isn't an NFC change). However, this is a change to the serialization format so a new PCH consumed by an older Clang will misbehave. Please check whether VERSION_MAJOR
(in ASTBitCodes.h
) has been updated for this release and if it hasn't been, increment it by one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. So it is natural that ParmVarDecl doesn't have a linkage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohhhh! 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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: 6fb08d8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.)
@@ -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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed that those should be swapped to Invalid
instead of None
(better done as a separate follow-up patch as it isn't an NFC change). However, this is a change to the serialization format so a new PCH consumed by an older Clang will misbehave. Please check whether VERSION_MAJOR
(in ASTBitCodes.h
) has been updated for this release and if it hasn't been, increment it by one.
clang::Linkage
clang::Linkage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I didn't understand the full context in the private chat.
@@ -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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. So it is natural that ParmVarDecl doesn't have a linkage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM aside from the comment nit from @ChuanqiXu9
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but please add NFC back into the patch title so it's more clear why there's no changes to test coverage. Thank you for the cleanup!
clang::Linkage
clang::Linkage
Local branch amd-gfx ab0d8a7 Merged main:7d77bbef4ad9 into amd-gfx:1a7ad991e1f8 Remote branch main 8775947 [clang][NFC] Refactor `clang::Linkage` (llvm#71049)
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 inclang::Decl
:This patch makes debuggers to not be mistaken about enumerator stored in this bit-field. It also converts
clang::Linkage
to a scoped enum.