tra added a comment.
A better description of the problem would help. PR itself is somewhat short on
details.
If I understand it correctly, the problem is that if we create multiple
definitions with the same mangled name, clang does not always report it as an
error and only emits one of those instances.
================
Comment at: lib/CodeGen/CodeGenModule.cpp:1235-1236
@@ -1235,7 +1234,4 @@
// different type.
- // FIXME: Support for variables is not implemented yet.
- if (isa<FunctionDecl>(D.getDecl()))
- GV = cast<llvm::GlobalValue>(GetAddrOfGlobal(D,
/*IsForDefinition=*/true));
- else
- if (!GV)
- GV = GetGlobalValue(getMangledName(D));
+ llvm::Constant *GVC = GetAddrOfGlobal(D, /*IsForDefinition=*/true);
+ llvm::GlobalValue *GV = dyn_cast<llvm::GlobalValue>(GVC);
+
----------------
andreybokhanko wrote:
> You are right -- I missed this case. Fixed.
>
> Patch updated.
Minor nit -- you could just do dyn_cast<...>(GetAddrOfGlobal()), considering
that GVC is not used anywhere else.
================
Comment at: lib/CodeGen/CodeGenModule.cpp:1250
@@ -1248,3 +1249,3 @@
// ignore these cases.
- if (GV && !GV->isDeclaration())
+ if (!GV->isDeclaration())
continue;
----------------
GetGlobalValue() may return nullptr and existing code did check for that.
I'd add an assert(GV).
================
Comment at: lib/CodeGen/CodeGenModule.cpp:2184-2188
@@ -2140,7 +2183,7 @@
+
if (!MustBeEmitted(D)) {
// If we have not seen a reference to this variable yet, place it
// into the deferred declarations table to be emitted if needed
// later.
- StringRef MangledName = getMangledName(D);
- if (!GetGlobalValue(MangledName)) {
+ if (!GV) {
DeferredDecls[MangledName] = D;
----------------
This may be collapsed into a single if() now.
================
Comment at: lib/CodeGen/CodeGenModule.cpp:2197-2198
@@ +2196,4 @@
+ // definition).
+ if (GV && !GV->isDeclaration())
+ return;
+
----------------
This can be moved above if (!MustBeEmitted()) -- no point calling
MustBeEmitted() in this case if GV != nullptr.
================
Comment at: lib/CodeGen/CodeGenModule.h:704
@@ -704,1 +703,3 @@
+ llvm::Type *Ty = nullptr,
+ bool IsForDefinition = false);
----------------
It would be good to document what IsForDefinition does.
================
Comment at: lib/CodeGen/CodeGenModule.h:1140
@@ -1139,1 +1139,3 @@
+ const VarDecl *D,
+ bool IsForDefinition = false);
----------------
Ditto here.
================
Comment at: lib/CodeGen/CodeGenModule.h:1151
@@ -1148,3 +1150,3 @@
void EmitGlobalFunctionDefinition(GlobalDecl GD, llvm::GlobalValue *GV);
- void EmitGlobalVarDefinition(const VarDecl *D);
+ void EmitGlobalVarDefinition(const VarDecl *D, bool IsTentative = false);
void EmitAliasDefinition(GlobalDecl GD);
----------------
And for IsTentative, too.
http://reviews.llvm.org/D15686
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
