Re: [PATCH] D15686: PR25910: clang allows two var definitions with the same mangled name

2016-01-14 Thread Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL257754: PR25910: clang allows two var definitions with the same mangled name (authored by asbokhan). Changed prior to commit: http://reviews.llvm.org/D15686?vs=44425=44845#toc Repository: rL LLVM

Re: [PATCH] D15686: PR25910: clang allows two var definitions with the same mangled name

2016-01-14 Thread Andrey Bokhanko via cfe-commits
andreybokhanko added a comment. @tra, @rnk, @rjmccall, thanks for the review! Yours, Andrey Repository: rL LLVM http://reviews.llvm.org/D15686 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D15686: PR25910: clang allows two var definitions with the same mangled name

2016-01-13 Thread Andrey Bokhanko via cfe-commits
andreybokhanko added a comment. In http://reviews.llvm.org/D15686#325266, @rnk wrote: > I thought we already addressed this issue with @rjmccall and decided that, if > the user intentionally declares extern "C" variables with an _Z prefix, then > we know they are intentionally attempting to

Re: [PATCH] D15686: PR25910: clang allows two var definitions with the same mangled name

2016-01-13 Thread John McCall via cfe-commits
rjmccall added a comment. Yes, this seems to be exactly what I wanted, thanks! LGTM. http://reviews.llvm.org/D15686 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15686: PR25910: clang allows two var definitions with the same mangled name

2016-01-12 Thread Reid Kleckner via cfe-commits
rnk added a subscriber: rnk. rnk added a comment. I thought we already addressed this issue with @rjmccall and decided that, if the user intentionally declares extern "C" variables with an _Z prefix, then we know they are intentionally attempting to name some C++ global variable. I'd rather

Re: [PATCH] D15686: PR25910: clang allows two var definitions with the same mangled name

2016-01-10 Thread Andrey Bokhanko via cfe-commits
andreybokhanko updated the summary for this revision. andreybokhanko updated this revision to Diff 44425. andreybokhanko marked 7 inline comments as done. andreybokhanko added a comment. Fixed tra's notes. All the fixes are local, logic of the patch is not changed.

Re: [PATCH] D15686: PR25910: clang allows two var definitions with the same mangled name

2016-01-05 Thread Artem Belevich via cfe-commits
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

Re: [PATCH] D15686: PR25910: clang allows two var definitions with the same mangled name

2016-01-05 Thread Andrey Bokhanko via cfe-commits
andreybokhanko added inline comments. Comment at: lib/CodeGen/CodeGenModule.cpp:1235-1243 @@ -1235,9 +1234,11 @@ // different type. -// FIXME: Support for variables is not implemented yet. -if (isa(D.getDecl())) - GV = cast(GetAddrOfGlobal(D,

Re: [PATCH] D15686: PR25910: clang allows two var definitions with the same mangled name

2016-01-04 Thread Artem Belevich via cfe-commits
tra added inline comments. Comment at: lib/CodeGen/CodeGenModule.cpp:1235-1236 @@ -1235,8 +1234,4 @@ // different type. -// FIXME: Support for variables is not implemented yet. -if (isa(D.getDecl())) - GV = cast(GetAddrOfGlobal(D, /*IsForDefinition=*/true)); -

Re: [PATCH] D15686: PR25910: clang allows two var definitions with the same mangled name

2016-01-04 Thread Andrey Bokhanko via cfe-commits
andreybokhanko added a comment. Ping! http://reviews.llvm.org/D15686 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15686: PR25910: clang allows two var definitions with the same mangled name

2015-12-21 Thread Andrey Bokhanko via cfe-commits
andreybokhanko added inline comments. Comment at: lib/CodeGen/CodeGenModule.cpp:1235-1236 @@ -1235,8 +1234,4 @@ // different type. -// FIXME: Support for variables is not implemented yet. -if (isa(D.getDecl())) - GV = cast(GetAddrOfGlobal(D,

[PATCH] D15686: PR25910: clang allows two var definitions with the same mangled name

2015-12-21 Thread Andrey Bokhanko via cfe-commits
andreybokhanko created this revision. andreybokhanko added reviewers: rjmccall, tra. andreybokhanko added a subscriber: cfe-commits. This patch fixes incorrect behavior described in PR25910. It is essentially the same stuff as in http://reviews.llvm.org/D11297, just for variables, not