[llvm] r209227 - GlobalValue: Automatically reset visibility when setting local linkage

[Juergen Ributzka]

Hi Duncan,

sorry for catching this so late. As it turns out, clang is not the only client that sets the visibility before the linkage. These methods are exposed via the C API and I didn’t see any documentation that there is an ordering constrained on these two methods. Since the C API has to be backward compatible it is possible that these recent changes can break existing clients like WebKit.

-Juergen

On May 20, 2014, at 12:00 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:

> Author: dexonsmith
> Date: Tue May 20 14:00:58 2014
> New Revision: 209227
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=209227&view=rev
> Log:
> GlobalValue: Automatically reset visibility when setting local linkage
> 
> r208264 started asserting in `setLinkage()` and `setVisibility()` that
> visibility and linkage are compatible.  There are a few places in clang
> where visibility is set first, and then linkage later, so the assert
> fires.  In `setLinkage()`, it's clear what the visibility *should* be,
> so rather than updating all the call sites just automatically fix the
> visibility.
> 
> The testcase for this is for *clang*, so it'll follow separately in cfe.
> 
> PR19760
> 
> Modified:
>    llvm/trunk/include/llvm/IR/GlobalValue.h
> 
> Modified: llvm/trunk/include/llvm/IR/GlobalValue.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/GlobalValue.h?rev=209227&r1=209226&r2=209227&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/IR/GlobalValue.h (original)
> +++ llvm/trunk/include/llvm/IR/GlobalValue.h Tue May 20 14:00:58 2014
> @@ -222,8 +222,8 @@ public:
>   bool hasCommonLinkage() const { return isCommonLinkage(Linkage); }
> 
>   void setLinkage(LinkageTypes LT) {
> -    assert((!isLocalLinkage(LT) || hasDefaultVisibility()) &&
> -           "local linkage requires default visibility");
> +    if (isLocalLinkage(LT))
> +      Visibility = DefaultVisibility;
>     Linkage = LT;
>   }
>   LinkageTypes getLinkage() const { return Linkage; }
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits