kromanova created this revision.
kromanova added reviewers: rsmith, dblaikie, echristo, probinson, aprantl.
kromanova added a subscriber: cfe-commits.
I had a review opened for anonymous namespaces bug in llvm-commits (see
http://reviews.llvm.org/D7895). It was opened in llvm-commits is because the
first patch that I submitted was in the backend. However it seems that the
frontend patch will be more appropriate in this case. David Blaikie advised to
get cfe developers opinion, specifically Richard Smith's.
Rather than just adding cfe-dev to the reviewers list, I thought it will be
better to open a brand new review.
There was quite a long discussion in llvm-commits list about this bug and
several re-implementations of a fix, so it gets confusing after a while.
I will write a short summary of what was discussed in
http://reviews.llvm.org/D7895
Problem #1:
DW_TAG_imported_module is missing from the compile unit's root scope. Without
this tag, anonymous namespace is not imported into the root scope by PS4
debugger.
Consider the following testcase. Lack of DW_TAG_imported_module prevents our
debugger from displaying the value of ‘a’.
#include <stdio.h>
namespace
{
int a = 5;
}
int main()
{
printf("%d\n", a);
return 0;
}
Problem #2:
Another (more general) problem is that while the clang compiler doesn't
generate DW_TAG_imported_module for the top-level anonymous namespace, it
generates this tag for nested anonymous namespaces. So, we have an asymmetrical
situation.
namespace
{
namespace {
int a1 = 5;
}
int a2 = 7;
}
int *b1 = &a1;
int *b2 = &a2;
DWARF:
<1><54>: Abbrev Number: 5 (DW_TAG_namespace)
<55> DW_AT_decl_file : 1
<56> DW_AT_decl_line : 2
<2><57>: Abbrev Number: 5 (DW_TAG_namespace)
<58> DW_AT_decl_file : 1
<59> DW_AT_decl_line : 3
....
<2><8d>: Abbrev Number: 7 (DW_TAG_imported_module)
<8e> DW_AT_import : <0x57> [Abbrev Number: 5 (DW_TAG_namespace)]
Note that DW_TAG_import is not generated for the top-level anonymous namespace.
There was a long discussion if we want to generate explicit imports marked by
DW_AT_artificial attribute for anonymous namespaces in Clang or not. Both GCC
and ICC compilers generate explicit imports in this case. However, it's an
arguable gray area between the compiler and the debugger. The final decision
was *not* to generate DW_TAG_imported_module for all the platforms except PS4.
On PS4 we definitely want it, since our debugger doesn't import the anonymous
namespace automatically.
There were several different patches for this fix. For this review, I'm
providing the latest FE patch only. It takes care of both problem #1 and
problem #2.
See http://reviews.llvm.org/D7895 for more details/earlier patches.
Richard, could you please give your opinion on how that problem needs to get
taken care of in the FE. I don't mind re-writing patch based on your comments,
if necessary.
Thank you!
http://reviews.llvm.org/D12624
Files:
include/clang/Parse/Parser.h
include/clang/Sema/Sema.h
lib/CodeGen/CGDebugInfo.cpp
lib/Parse/ParseDecl.cpp
lib/Parse/ParseDeclCXX.cpp
lib/Sema/SemaDeclCXX.cpp
test/CodeGenCXX/debug-info-anon-namespace.cpp
Index: test/CodeGenCXX/debug-info-anon-namespace.cpp
===================================================================
--- test/CodeGenCXX/debug-info-anon-namespace.cpp
+++ test/CodeGenCXX/debug-info-anon-namespace.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -emit-llvm -g -triple x86_64-scei-ps4 -O0 %s -o - | FileCheck --check-prefix=PS4 %s
+// RUN: %clang_cc1 -emit-llvm -g -triple x86_64-unknown-linux-gnu -O0 %s -o - | FileCheck --check-prefix=NON-PS4 %s
+
+namespace
+{
+ int a = 5;
+}
+int *b = &a;
+
+namespace
+{
+ namespace {
+ int a1 = 5;
+ }
+ int a2 = 7;
+}
+int *b1 = &a1;
+int *b2 = &a2;
+
+
+// PS4: [[NS:![0-9]+]] = !DINamespace
+// PS4: [[NS2:![0-9]+]] = !DINamespace
+// PS4: !DIImportedEntity(tag: DW_TAG_imported_module, scope: !0, entity: [[NS]])
+// PS4: !DIImportedEntity(tag: DW_TAG_imported_module, scope: [[NS]], entity: [[NS2]], line: {{[0-9]+}})
+// NON-PS4-NOT: !DIImportedEntity
+
Index: lib/Sema/SemaDeclCXX.cpp
===================================================================
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -7185,7 +7185,8 @@
SourceLocation IdentLoc,
IdentifierInfo *II,
SourceLocation LBrace,
- AttributeList *AttrList) {
+ AttributeList *AttrList,
+ UsingDirectiveDecl *& UD) {
SourceLocation StartLoc = InlineLoc.isValid() ? InlineLoc : NamespaceLoc;
// For anonymous namespace, take the location of the left brace.
SourceLocation Loc = II ? IdentLoc : LBrace;
@@ -7309,14 +7310,13 @@
// namespace internal linkage.
if (!PrevNS) {
- UsingDirectiveDecl* UD
- = UsingDirectiveDecl::Create(Context, Parent,
- /* 'using' */ LBrace,
- /* 'namespace' */ SourceLocation(),
- /* qualifier */ NestedNameSpecifierLoc(),
- /* identifier */ SourceLocation(),
- Namespc,
- /* Ancestor */ Parent);
+ UD = UsingDirectiveDecl::Create(Context, Parent,
+ /* 'using' */ LBrace,
+ /* 'namespace' */ SourceLocation(),
+ /* qualifier */ NestedNameSpecifierLoc(),
+ /* identifier */ SourceLocation(),
+ Namespc,
+ /* Ancestor */ Parent);
UD->setImplicit();
Parent->addDecl(UD);
}
Index: lib/Parse/ParseDeclCXX.cpp
===================================================================
--- lib/Parse/ParseDeclCXX.cpp
+++ lib/Parse/ParseDeclCXX.cpp
@@ -54,9 +54,9 @@
/// namespace-alias-definition: [C++ 7.3.2: namespace.alias]
/// 'namespace' identifier '=' qualified-namespace-specifier ';'
///
-Decl *Parser::ParseNamespace(unsigned Context,
- SourceLocation &DeclEnd,
- SourceLocation InlineLoc) {
+Parser::DeclGroupPtrTy Parser::ParseNamespace(unsigned Context,
+ SourceLocation &DeclEnd,
+ SourceLocation InlineLoc) {
assert(Tok.is(tok::kw_namespace) && "Not a namespace!");
SourceLocation NamespaceLoc = ConsumeToken(); // eat the 'namespace'.
ObjCDeclContextSwitch ObjCDC(*this);
@@ -64,7 +64,7 @@
if (Tok.is(tok::code_completion)) {
Actions.CodeCompleteNamespaceDecl(getCurScope());
cutOffParsing();
- return nullptr;
+ return DeclGroupPtrTy();;
}
SourceLocation IdentLoc;
@@ -108,7 +108,7 @@
Diag(Tok, diag::err_expected) << tok::identifier;
// Skip to end of the definition and eat the ';'.
SkipUntil(tok::semi);
- return nullptr;
+ return DeclGroupPtrTy();
}
if (attrLoc.isValid())
Diag(attrLoc, diag::err_unexpected_namespace_attributes_alias);
@@ -115,8 +115,9 @@
if (InlineLoc.isValid())
Diag(InlineLoc, diag::err_inline_namespace_alias)
<< FixItHint::CreateRemoval(InlineLoc);
- return ParseNamespaceAlias(NamespaceLoc, IdentLoc, Ident, DeclEnd);
- }
+ Decl *NSAlias = ParseNamespaceAlias(NamespaceLoc, IdentLoc, Ident, DeclEnd);
+ return Actions.ConvertDeclToDeclGroup(NSAlias);
+}
BalancedDelimiterTracker T(*this, tok::l_brace);
@@ -125,7 +126,7 @@
Diag(Tok, diag::err_expected) << tok::l_brace;
else
Diag(Tok, diag::err_expected_either) << tok::identifier << tok::l_brace;
- return nullptr;
+ return DeclGroupPtrTy();
}
if (getCurScope()->isClassScope() || getCurScope()->isTemplateParamScope() ||
@@ -133,7 +134,7 @@
getCurScope()->getFnParent()) {
Diag(T.getOpenLocation(), diag::err_namespace_nonnamespace_scope);
SkipUntil(tok::r_brace);
- return nullptr;
+ return DeclGroupPtrTy();
}
if (ExtraIdent.empty()) {
@@ -180,10 +181,11 @@
// Enter a scope for the namespace.
ParseScope NamespaceScope(this, Scope::DeclScope);
+ UsingDirectiveDecl *ImplicitUsingDirectiveDecl = nullptr;
Decl *NamespcDecl =
Actions.ActOnStartNamespaceDef(getCurScope(), InlineLoc, NamespaceLoc,
IdentLoc, Ident, T.getOpenLocation(),
- attrs.getList());
+ attrs.getList(), ImplicitUsingDirectiveDecl);
PrettyDeclStackTraceEntry CrashInfo(Actions, NamespcDecl, NamespaceLoc,
"parsing namespace");
@@ -198,8 +200,10 @@
DeclEnd = T.getCloseLocation();
Actions.ActOnFinishNamespaceDef(NamespcDecl, DeclEnd);
-
- return NamespcDecl;
+
+ // Return NamespaceDecl + ImportDecl here.
+ return Actions.ConvertDeclToDeclGroup(NamespcDecl,
+ ImplicitUsingDirectiveDecl);
}
/// ParseInnerNamespace - Parse the contents of a namespace.
@@ -228,17 +232,17 @@
// FIXME: Preserve the source information through to the AST rather than
// desugaring it here.
ParseScope NamespaceScope(this, Scope::DeclScope);
+ UsingDirectiveDecl *ImplicitUsingDirectiveDecl = NULL;
Decl *NamespcDecl =
Actions.ActOnStartNamespaceDef(getCurScope(), SourceLocation(),
NamespaceLoc[index], IdentLoc[index],
Ident[index], Tracker.getOpenLocation(),
- attrs.getList());
+ attrs.getList(), ImplicitUsingDirectiveDecl);
ParseInnerNamespace(IdentLoc, Ident, NamespaceLoc, ++index, InlineLoc,
attrs, Tracker);
NamespaceScope.Exit();
-
Actions.ActOnFinishNamespaceDef(NamespcDecl, Tracker.getCloseLocation());
}
Index: lib/Parse/ParseDecl.cpp
===================================================================
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/ParseDecl.cpp
@@ -1468,15 +1468,13 @@
if (getLangOpts().CPlusPlus && NextToken().is(tok::kw_namespace)) {
ProhibitAttributes(attrs);
SourceLocation InlineLoc = ConsumeToken();
- SingleDecl = ParseNamespace(Context, DeclEnd, InlineLoc);
- break;
+ return ParseNamespace(Context, DeclEnd, InlineLoc);
}
return ParseSimpleDeclaration(Context, DeclEnd, attrs,
true);
case tok::kw_namespace:
ProhibitAttributes(attrs);
- SingleDecl = ParseNamespace(Context, DeclEnd);
- break;
+ return ParseNamespace(Context, DeclEnd);
case tok::kw_using:
SingleDecl = ParseUsingDirectiveOrDeclaration(Context, ParsedTemplateInfo(),
DeclEnd, attrs, &OwnedType);
Index: lib/CodeGen/CGDebugInfo.cpp
===================================================================
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -3259,10 +3259,14 @@
void CGDebugInfo::EmitUsingDirective(const UsingDirectiveDecl &UD) {
if (CGM.getCodeGenOpts().getDebugInfo() < CodeGenOptions::LimitedDebugInfo)
return;
- DBuilder.createImportedModule(
- getCurrentContextDescriptor(cast<Decl>(UD.getDeclContext())),
- getOrCreateNameSpace(UD.getNominatedNamespace()),
- getLineNumber(UD.getLocation()));
+ const NamespaceDecl *NSDecl = UD.getNominatedNamespace();
+ if (!NSDecl->isAnonymousNamespace() ||
+ CGM.getTarget().getTriple().isPS4CPU()) {
+ DBuilder.createImportedModule(
+ getCurrentContextDescriptor(cast<Decl>(UD.getDeclContext())),
+ getOrCreateNameSpace(NSDecl),
+ getLineNumber(UD.getLocation()));
+ }
}
void CGDebugInfo::EmitUsingDecl(const UsingDecl &UD) {
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -4054,7 +4054,8 @@
SourceLocation IdentLoc,
IdentifierInfo *Ident,
SourceLocation LBrace,
- AttributeList *AttrList);
+ AttributeList *AttrList,
+ UsingDirectiveDecl * &UsingDecl);
void ActOnFinishNamespaceDef(Decl *Dcl, SourceLocation RBrace);
NamespaceDecl *getStdNamespace() const;
Index: include/clang/Parse/Parser.h
===================================================================
--- include/clang/Parse/Parser.h
+++ include/clang/Parse/Parser.h
@@ -2335,8 +2335,8 @@
void DiagnoseUnexpectedNamespace(NamedDecl *Context);
- Decl *ParseNamespace(unsigned Context, SourceLocation &DeclEnd,
- SourceLocation InlineLoc = SourceLocation());
+ DeclGroupPtrTy ParseNamespace(unsigned Context, SourceLocation &DeclEnd,
+ SourceLocation InlineLoc = SourceLocation());
void ParseInnerNamespace(std::vector<SourceLocation>& IdentLoc,
std::vector<IdentifierInfo*>& Ident,
std::vector<SourceLocation>& NamespaceLoc,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
