[clang] [clang-tools-extra] [clang][NFC] Refactor `clang::Linkage` (PR #71049)

2023-11-02 Thread Vlad Serebrennikov via cfe-commits 


@@ -560,7 +562,7 @@ bool CXIndexDataConsumer::handleDecl(const NamedDecl *D,
 
   if (shouldSuppressRefs())
 markEntityOccurrenceInFile(D, Loc);
-  
+

Endilll wrote:

I guess that's clang-format at work. I'll revert this.

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][NFC] Refactor `clang::Linkage` (PR #71049)

2023-11-02 Thread Aaron Ballman via cfe-commits 


@@ -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:

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.

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][NFC] Refactor `clang::Linkage` (PR #71049)

2023-11-02 Thread Aaron Ballman via cfe-commits 


@@ -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:

Can you explain this change a bit? This looks like a functional change?

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][NFC] Refactor `clang::Linkage` (PR #71049)

2023-11-02 Thread Aaron Ballman via cfe-commits 


@@ -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!");

AaronBallman wrote:

I think this is a good approach.

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][NFC] Refactor `clang::Linkage` (PR #71049)

2023-11-02 Thread Vlad Serebrennikov via cfe-commits 


@@ -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:

@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.

CC @AaronBallman 

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][NFC] Refactor `clang::Linkage` (PR #71049)

2023-11-02 Thread Vlad Serebrennikov via cfe-commits 


@@ -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!");

Endilll wrote:

@AaronBallman I sprinkled a lot of `llvm_unreachable` in this patch, following 
an example I've seen in one of the switches on `Linkage`. Let me know if you'd 
like this to be handled in a different way.

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][NFC] Refactor `clang::Linkage` (PR #71049)

2023-11-02 Thread via cfe-commits 

llvmbot wrote:




@llvm/pr-subscribers-clang-codegen

Author: Vlad Serebrennikov (Endilll)


Changes

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.

---

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:

- (modified) clang-tools-extra/clang-doc/Serialize.cpp (+2-2) 
- (modified) clang-tools-extra/clangd/Quality.cpp (+2-1) 
- (modified) clang-tools-extra/clangd/SemanticHighlighting.cpp (+2-1) 
- (modified) clang/include/clang/AST/DeclBase.h (+5-6) 
- (modified) clang/include/clang/AST/Type.h (+1-1) 
- (modified) clang/include/clang/Basic/Linkage.h (+34-19) 
- (modified) clang/include/clang/Basic/Visibility.h (+15-13) 
- (modified) clang/lib/AST/APValue.cpp (+1-1) 
- (modified) clang/lib/AST/Decl.cpp (+20-8) 
- (modified) clang/lib/AST/ItaniumMangle.cpp (+2-2) 
- (modified) clang/lib/AST/MicrosoftMangle.cpp (+2-3) 
- (modified) clang/lib/AST/Type.cpp (+4-4) 
- (modified) clang/lib/CodeGen/CodeGenModule.cpp (+2-2) 
- (modified) clang/lib/CodeGen/ItaniumCXXABI.cpp (+9-6) 
- (modified) clang/lib/CodeGen/MicrosoftCXXABI.cpp (+9-6) 
- (modified) clang/lib/Index/IndexSymbol.cpp (+11-9) 
- (modified) clang/lib/Sema/SemaChecking.cpp (+1-1) 
- (modified) clang/lib/Sema/SemaDecl.cpp (+10-14) 
- (modified) clang/lib/Sema/SemaExpr.cpp (+1-1) 
- (modified) clang/lib/Sema/SemaModule.cpp (+3-3) 
- (modified) clang/lib/Sema/SemaOverload.cpp (+1-1) 
- (modified) clang/lib/Sema/SemaTemplate.cpp (+2-2) 
- (modified) clang/lib/Serialization/ASTReaderDecl.cpp (+2-2) 
- (modified) clang/lib/Serialization/ASTWriterDecl.cpp (+3-3) 
- (modified) clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp (+5-3) 
- (modified) clang/tools/libclang/CIndex.cpp (+8-6) 
- (modified) clang/tools/libclang/CXIndexDataConsumer.cpp (+28-28) 
- (modified) clang/unittests/AST/DeclTest.cpp (+4-4) 


``diff
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 ExternalSourceSymbolAttr;
 class FunctionDecl;
 class FunctionType;
 class IdentifierInfo;
-enum Linkage :

[clang] [clang-tools-extra] [clang][NFC] Refactor `clang::Linkage` (PR #71049)

2023-11-02 Thread via cfe-commits 

llvmbot wrote:




@llvm/pr-subscribers-clang-modules

Author: Vlad Serebrennikov (Endilll)


Changes

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.

---

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:

- (modified) clang-tools-extra/clang-doc/Serialize.cpp (+2-2) 
- (modified) clang-tools-extra/clangd/Quality.cpp (+2-1) 
- (modified) clang-tools-extra/clangd/SemanticHighlighting.cpp (+2-1) 
- (modified) clang/include/clang/AST/DeclBase.h (+5-6) 
- (modified) clang/include/clang/AST/Type.h (+1-1) 
- (modified) clang/include/clang/Basic/Linkage.h (+34-19) 
- (modified) clang/include/clang/Basic/Visibility.h (+15-13) 
- (modified) clang/lib/AST/APValue.cpp (+1-1) 
- (modified) clang/lib/AST/Decl.cpp (+20-8) 
- (modified) clang/lib/AST/ItaniumMangle.cpp (+2-2) 
- (modified) clang/lib/AST/MicrosoftMangle.cpp (+2-3) 
- (modified) clang/lib/AST/Type.cpp (+4-4) 
- (modified) clang/lib/CodeGen/CodeGenModule.cpp (+2-2) 
- (modified) clang/lib/CodeGen/ItaniumCXXABI.cpp (+9-6) 
- (modified) clang/lib/CodeGen/MicrosoftCXXABI.cpp (+9-6) 
- (modified) clang/lib/Index/IndexSymbol.cpp (+11-9) 
- (modified) clang/lib/Sema/SemaChecking.cpp (+1-1) 
- (modified) clang/lib/Sema/SemaDecl.cpp (+10-14) 
- (modified) clang/lib/Sema/SemaExpr.cpp (+1-1) 
- (modified) clang/lib/Sema/SemaModule.cpp (+3-3) 
- (modified) clang/lib/Sema/SemaOverload.cpp (+1-1) 
- (modified) clang/lib/Sema/SemaTemplate.cpp (+2-2) 
- (modified) clang/lib/Serialization/ASTReaderDecl.cpp (+2-2) 
- (modified) clang/lib/Serialization/ASTWriterDecl.cpp (+3-3) 
- (modified) clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp (+5-3) 
- (modified) clang/tools/libclang/CIndex.cpp (+8-6) 
- (modified) clang/tools/libclang/CXIndexDataConsumer.cpp (+28-28) 
- (modified) clang/unittests/AST/DeclTest.cpp (+4-4) 


``diff
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 ExternalSourceSymbolAttr;
 class FunctionDecl;
 class FunctionType;
 class IdentifierInfo;
-enum Linkage :

[clang] [clang-tools-extra] [clang][NFC] Refactor `clang::Linkage` (PR #71049)

2023-11-02 Thread Vlad Serebrennikov via cfe-commits 

https://github.com/Endilll created 
https://github.com/llvm/llvm-project/pull/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 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.

>From 05089e60021c321b4113db7e4bdf59bdaaa19de7 Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov 
Date: Thu, 2 Nov 2023 14:40:12 +0300
Subject: [PATCH] [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.