[Lldb-commits] [clang] [lldb] [clang][AST] fix ast-print of extern <lang> with >=2 declarators, fixed, fixed (PR #98795)

via lldb-commits lldb-commits at lists.llvm.org
Sat Jul 13 22:08:06 PDT 2024


https://github.com/temyurchenko created https://github.com/llvm/llvm-project/pull/98795

Relanding of #93913. Now, it also fixes incorrect declaration context in function parameters creation.

>From 410c7ba9fb7667dabdfbc48fdbda427401ca8df0 Mon Sep 17 00:00:00 2001
From: Artem Yurchenko <44875844+temyurchenko at users.noreply.github.com>
Date: Thu, 30 May 2024 16:18:47 -0400
Subject: [PATCH 1/3] [clang][AST] fix ast-print of `extern <lang>` with >=2
 declarators (#93131)

Problem: the printer used to ignore all but the first declarator for
unbraced language linkage declarators. Furthemore, that one would
be printed without the final semicolon.

Solution: for unbraced case we traverse all declarators via
`VisitDeclContext`. Furthermore, in appropriate visitors we query
for whether they are a part of the unbraced extern language linkage
spec, and if so, print appropriately.
---
 clang/lib/AST/DeclPrinter.cpp                 | 55 ++++++++++++++-----
 clang/test/AST/ast-print-language-linkage.cpp | 31 +++++++++++
 2 files changed, 72 insertions(+), 14 deletions(-)
 create mode 100644 clang/test/AST/ast-print-language-linkage.cpp

diff --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp
index 0cf4e64f83b8d..9250a7f6eceb2 100644
--- a/clang/lib/AST/DeclPrinter.cpp
+++ b/clang/lib/AST/DeclPrinter.cpp
@@ -633,7 +633,7 @@ static void printExplicitSpecifier(ExplicitSpecifier ES, llvm::raw_ostream &Out,
   Out << Proto;
 }
 
-static void MaybePrintTagKeywordIfSupressingScopes(PrintingPolicy &Policy,
+static void maybePrintTagKeywordIfSupressingScopes(PrintingPolicy &Policy,
                                                    QualType T,
                                                    llvm::raw_ostream &Out) {
   StringRef prefix = T->isClassType()       ? "class "
@@ -643,6 +643,22 @@ static void MaybePrintTagKeywordIfSupressingScopes(PrintingPolicy &Policy,
   Out << prefix;
 }
 
+/// Return the language of the linkage spec of `D`, if applicable.
+///
+/// \Return - "C" if `D` has been declared with unbraced `extern "C"`
+///         - "C++" if `D` has been declared with unbraced `extern "C++"`
+///         - nullptr in any other case
+static const char *tryGetUnbracedLinkageLanguage(const Decl *D) {
+  const auto *SD = dyn_cast<LinkageSpecDecl>(D->getDeclContext());
+  if (!SD || SD->hasBraces())
+    return nullptr;
+  if (SD->getLanguage() == LinkageSpecLanguageIDs::C)
+    return "C";
+  assert(SD->getLanguage() == LinkageSpecLanguageIDs::CXX &&
+         "unknown language in linkage specification");
+  return "C++";
+}
+
 void DeclPrinter::VisitFunctionDecl(FunctionDecl *D) {
   if (!D->getDescribedFunctionTemplate() &&
       !D->isFunctionTemplateSpecialization()) {
@@ -662,6 +678,11 @@ void DeclPrinter::VisitFunctionDecl(FunctionDecl *D) {
   CXXConversionDecl *ConversionDecl = dyn_cast<CXXConversionDecl>(D);
   CXXDeductionGuideDecl *GuideDecl = dyn_cast<CXXDeductionGuideDecl>(D);
   if (!Policy.SuppressSpecifiers) {
+    if (const char *Lang = tryGetUnbracedLinkageLanguage(D)) {
+      // the "extern" specifier is implicit
+      assert(D->getStorageClass() == SC_None);
+      Out << "extern \"" << Lang << "\" ";
+    }
     switch (D->getStorageClass()) {
     case SC_None: break;
     case SC_Extern: Out << "extern "; break;
@@ -807,7 +828,7 @@ void DeclPrinter::VisitFunctionDecl(FunctionDecl *D) {
       }
       if (!Policy.SuppressTagKeyword && Policy.SuppressScope &&
           !Policy.SuppressUnwrittenScope)
-        MaybePrintTagKeywordIfSupressingScopes(Policy, AFT->getReturnType(),
+        maybePrintTagKeywordIfSupressingScopes(Policy, AFT->getReturnType(),
                                                Out);
       AFT->getReturnType().print(Out, Policy, Proto);
       Proto.clear();
@@ -932,6 +953,11 @@ void DeclPrinter::VisitVarDecl(VarDecl *D) {
     : D->getASTContext().getUnqualifiedObjCPointerType(D->getType());
 
   if (!Policy.SuppressSpecifiers) {
+    if (const char *Lang = tryGetUnbracedLinkageLanguage(D)) {
+      // the "extern" specifier is implicit
+      assert(D->getStorageClass() == SC_None);
+      Out << "extern \"" << Lang << "\" ";
+    }
     StorageClass SC = D->getStorageClass();
     if (SC != SC_None)
       Out << VarDecl::getStorageClassSpecifierString(SC) << " ";
@@ -961,7 +987,7 @@ void DeclPrinter::VisitVarDecl(VarDecl *D) {
 
   if (!Policy.SuppressTagKeyword && Policy.SuppressScope &&
       !Policy.SuppressUnwrittenScope)
-    MaybePrintTagKeywordIfSupressingScopes(Policy, T, Out);
+    maybePrintTagKeywordIfSupressingScopes(Policy, T, Out);
 
   printDeclType(T, (isa<ParmVarDecl>(D) && Policy.CleanUglifiedParameters &&
                     D->getIdentifier())
@@ -1064,6 +1090,8 @@ void DeclPrinter::VisitNamespaceAliasDecl(NamespaceAliasDecl *D) {
 
 void DeclPrinter::VisitEmptyDecl(EmptyDecl *D) {
   prettyPrintAttributes(D);
+  if (const char *Lang = tryGetUnbracedLinkageLanguage(D))
+    Out << "extern \"" << Lang << "\";";
 }
 
 void DeclPrinter::VisitCXXRecordDecl(CXXRecordDecl *D) {
@@ -1136,22 +1164,21 @@ void DeclPrinter::VisitCXXRecordDecl(CXXRecordDecl *D) {
 }
 
 void DeclPrinter::VisitLinkageSpecDecl(LinkageSpecDecl *D) {
-  const char *l;
+  if (!D->hasBraces()) {
+    VisitDeclContext(D);
+    return;
+  }
+  const char *L;
   if (D->getLanguage() == LinkageSpecLanguageIDs::C)
-    l = "C";
+    L = "C";
   else {
     assert(D->getLanguage() == LinkageSpecLanguageIDs::CXX &&
            "unknown language in linkage specification");
-    l = "C++";
+    L = "C++";
   }
-
-  Out << "extern \"" << l << "\" ";
-  if (D->hasBraces()) {
-    Out << "{\n";
-    VisitDeclContext(D);
-    Indent() << "}";
-  } else
-    Visit(*D->decls_begin());
+  Out << "extern \"" << L << "\" {\n";
+  VisitDeclContext(D);
+  Indent() << "}";
 }
 
 void DeclPrinter::printTemplateParameters(const TemplateParameterList *Params,
diff --git a/clang/test/AST/ast-print-language-linkage.cpp b/clang/test/AST/ast-print-language-linkage.cpp
new file mode 100644
index 0000000000000..7e4dc3f25f062
--- /dev/null
+++ b/clang/test/AST/ast-print-language-linkage.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -ast-print %s -o - | FileCheck %s
+
+// CHECK: extern "C" int printf(const char *, ...);
+extern "C" int printf(const char *...);
+
+// CHECK: extern "C++" int f(int);
+// CHECK-NEXT: extern "C++" int g(int);
+extern "C++" int f(int), g(int);
+
+// CHECK: extern "C" char a;
+// CHECK-NEXT: extern "C" char b;
+extern "C" char a, b;
+
+// CHECK: extern "C" {
+// CHECK-NEXT:  void foo();
+// CHECK-NEXT:  int x;
+// CHECK-NEXT:  int y;
+// CHECK-NEXT:  extern short z;
+// CHECK-NEXT: }
+extern "C" {
+  void foo(void);
+  int x, y;
+  extern short z;
+}
+
+// CHECK: extern "C" {
+// CHECK-NEXT: }
+extern "C" {}
+
+// CHECK: extern "C++";
+extern "C++";

>From b9fa7fec59bc27ef32d738f78046525f4118e53d Mon Sep 17 00:00:00 2001
From: Artem Yurchenko <artemyurchenko at zoho.com>
Date: Thu, 6 Jun 2024 20:20:19 -0400
Subject: [PATCH 2/3] [clang][AST] enforce the unbraced `extern <lang>`
 invariant

Quoting 9.11.8:
> A declaration directly contained in a linkage-specification is
> treated as if it contains the extern specifier (9.2.2) for the
> purpose of determining the linkage of the declared name and whether
> it is a definition. Such a declaration shall not specify a storage
> class.

So, the invariant is that function and variable declarations within an
unbraced language linkage specification have `StorageClass` set to
`SC_None`.

Problem: in several places the invariant was broken.

Solution: remove the explicit `SC_Extern` specification. Note, that my
changes result in the `extern` being implicit in some functions
that are not contained in a language linkage specification.
---
 clang/lib/AST/Decl.cpp                        | 27 ++++++++++++++-----
 clang/lib/Sema/SemaDecl.cpp                   |  2 +-
 clang/lib/Sema/SemaExpr.cpp                   |  2 +-
 .../Clang/ClangExpressionParser.cpp           |  2 +-
 .../Clang/NameSearchContext.cpp               |  3 ++-
 5 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 1f19dadafa44e..35973d99f906d 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -576,13 +576,16 @@ template <typename T> static bool isFirstInExternCContext(T *D) {
   return First->isInExternCContext();
 }
 
-static bool isSingleLineLanguageLinkage(const Decl &D) {
-  if (const auto *SD = dyn_cast<LinkageSpecDecl>(D.getDeclContext()))
-    if (!SD->hasBraces())
-      return true;
+static bool isUnbracedLanguageLinkage(const DeclContext *DC) {
+  if (const auto *SD = dyn_cast_if_present<LinkageSpecDecl>(DC))
+    return !SD->hasBraces();
   return false;
 }
 
+static bool hasUnbracedLanguageLinkage(const Decl &D) {
+  return isUnbracedLanguageLinkage(D.getDeclContext());
+}
+
 static bool isDeclaredInModuleInterfaceOrPartition(const NamedDecl *D) {
   if (auto *M = D->getOwningModule())
     return M->isInterfaceOrPartition();
@@ -644,7 +647,7 @@ LinkageComputer::getLVForNamespaceScopeDecl(const NamedDecl *D,
 
       if (Var->getStorageClass() != SC_Extern &&
           Var->getStorageClass() != SC_PrivateExtern &&
-          !isSingleLineLanguageLinkage(*Var))
+          !hasUnbracedLanguageLinkage(*Var))
         return LinkageInfo::internal();
     }
 
@@ -2133,6 +2136,12 @@ VarDecl::VarDecl(Kind DK, ASTContext &C, DeclContext *DC,
                 "ParmVarDeclBitfields too large!");
   static_assert(sizeof(NonParmVarDeclBitfields) <= sizeof(unsigned),
                 "NonParmVarDeclBitfields too large!");
+
+  // The unbraced `extern "C"` invariant is that the storage class
+  // specifier is omitted in the source code, i.e. SC_None (but is,
+  // implicitly, `extern`).
+  assert(!isUnbracedLanguageLinkage(DC) || SC == SC_None);
+
   AllBits = 0;
   VarDeclBits.SClass = SC;
   // Everything else is implicitly initialized to false.
@@ -2316,7 +2325,7 @@ VarDecl::isThisDeclarationADefinition(ASTContext &C) const {
   //   A declaration directly contained in a linkage-specification is treated
   //   as if it contains the extern specifier for the purpose of determining
   //   the linkage of the declared name and whether it is a definition.
-  if (isSingleLineLanguageLinkage(*this))
+  if (hasUnbracedLanguageLinkage(*this))
     return DeclarationOnly;
 
   // C99 6.9.2p2:
@@ -3041,6 +3050,12 @@ FunctionDecl::FunctionDecl(Kind DK, ASTContext &C, DeclContext *DC,
       DeclContext(DK), redeclarable_base(C), Body(), ODRHash(0),
       EndRangeLoc(NameInfo.getEndLoc()), DNLoc(NameInfo.getInfo()) {
   assert(T.isNull() || T->isFunctionType());
+
+  // The unbraced `extern "C"` invariant is that the storage class
+  // specifier is omitted in the source code, i.e. SC_None (but is,
+  // implicitly, `extern`).
+  assert(!isUnbracedLanguageLinkage(DC) || S == SC_None);
+
   FunctionDeclBits.SClass = S;
   FunctionDeclBits.IsInline = isInlineSpecified;
   FunctionDeclBits.IsInlineSpecified = isInlineSpecified;
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 4b9b735f1cfb4..4e2f02ba7007a 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -2380,7 +2380,7 @@ FunctionDecl *Sema::CreateBuiltin(IdentifierInfo *II, QualType Type,
   }
 
   FunctionDecl *New = FunctionDecl::Create(Context, Parent, Loc, Loc, II, Type,
-                                           /*TInfo=*/nullptr, SC_Extern,
+                                           /*TInfo=*/nullptr, SC_None,
                                            getCurFPFeatures().isFPConstrained(),
                                            false, Type->isFunctionProtoType());
   New->setImplicit();
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 76145f291887c..60075120fe030 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -6286,7 +6286,7 @@ static FunctionDecl *rewriteBuiltinFunctionDecl(Sema *Sema, ASTContext &Context,
   FunctionDecl *OverloadDecl = FunctionDecl::Create(
       Context, Parent, FDecl->getLocation(), FDecl->getLocation(),
       FDecl->getIdentifier(), OverloadTy,
-      /*TInfo=*/nullptr, SC_Extern, Sema->getCurFPFeatures().isFPConstrained(),
+      /*TInfo=*/nullptr, SC_None, Sema->getCurFPFeatures().isFPConstrained(),
       false,
       /*hasPrototype=*/true);
   SmallVector<ParmVarDecl*, 16> Params;
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index 1dd98567f8d69..12aa1bd60bcb7 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -881,7 +881,7 @@ class CodeComplete : public CodeCompleteConsumer {
         else
           ToInsert += "(";
         raw_string_ostream OS(Description);
-        F->print(OS, m_desc_policy, false);
+        F->print(OS, m_desc_policy);
         OS.flush();
       } else if (const VarDecl *V = dyn_cast<VarDecl>(D)) {
         Description = V->getType().getAsString(m_desc_policy);
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.cpp b/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.cpp
index da59855a9f162..e02b418b1e793 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.cpp
@@ -77,7 +77,8 @@ clang::NamedDecl *NameSearchContext::AddFunDecl(const CompilerType &type,
 
   clang::FunctionDecl *func_decl = FunctionDecl::Create(
       ast, context, SourceLocation(), SourceLocation(), decl_name, qual_type,
-      nullptr, SC_Extern, /*UsesFPIntrin=*/false, isInlineSpecified, hasWrittenPrototype,
+      nullptr, SC_None, /*UsesFPIntrin=*/false, isInlineSpecified,
+      hasWrittenPrototype,
       isConstexprSpecified ? ConstexprSpecKind::Constexpr
                            : ConstexprSpecKind::Unspecified);
 

>From 26f45d30cf82fce9e1dc1d048f09b45090d47112 Mon Sep 17 00:00:00 2001
From: Artem Yurchenko <artemyurchenko at zoho.com>
Date: Sat, 13 Jul 2024 21:12:56 -0700
Subject: [PATCH 3/3] [lldb][AST] fix declaration context passed to parameter
 vars

It needs to be the function declaration that the parameter belongs to,
   not the context of that function.
---
 .../Plugins/ExpressionParser/Clang/NameSearchContext.cpp   | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.cpp b/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.cpp
index e02b418b1e793..da02e4c72cd19 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.cpp
@@ -98,10 +98,9 @@ clang::NamedDecl *NameSearchContext::AddFunDecl(const CompilerType &type,
     for (ArgIndex = 0; ArgIndex < NumArgs; ++ArgIndex) {
       QualType arg_qual_type(func_proto_type->getParamType(ArgIndex));
 
-      parm_var_decls.push_back(
-          ParmVarDecl::Create(ast, const_cast<DeclContext *>(context),
-                              SourceLocation(), SourceLocation(), nullptr,
-                              arg_qual_type, nullptr, SC_Static, nullptr));
+      parm_var_decls.push_back(ParmVarDecl::Create(
+          ast, func_decl, SourceLocation(), SourceLocation(), nullptr,
+          arg_qual_type, nullptr, SC_Static, nullptr));
     }
 
     func_decl->setParams(ArrayRef<ParmVarDecl *>(parm_var_decls));



More information about the lldb-commits mailing list