Rework the visibility computation algorithm in preparation

for distinguishing type vs. value visibility.

The changes to the visibility of explicit specializations
are intentional.  The change to the "ugly" test case is
a consequence of a sensible implementation, and I am happy
to argue that this is better behavior.  Other changes may
or may not be intended;  it is quite difficult to divine
intent from some of the code I altered.

I've left behind a comment which I hope explains the
philosophy behind visibility computation.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@175326 91177308-0d34-0410-b5e6-96231b3b80d8

1 files changed, 25 insertions, 43 deletions

diff --git a/include/clang/AST/Decl.h b/include/clang/AST/Decl.h
index 117ec7eef3..e9643d2dc7 100644
--- a/include/clang/AST/Decl.h
+++ b/include/clang/AST/Decl.h
@@ -247,63 +247,45 @@ public:
     bool visibilityExplicit() const { return explicit_; }
 
     void setLinkage(Linkage L) { linkage_ = L; }
+
     void mergeLinkage(Linkage L) {
       setLinkage(minLinkage(linkage(), L));
     }
-    void mergeLinkage(LinkageInfo Other) {
-      mergeLinkage(Other.linkage());
+    void mergeLinkage(LinkageInfo other) {
+      mergeLinkage(other.linkage());
     }
 
-    // Merge the visibility V giving preference to explicit ones.
-    // This is used, for example, when merging the visibility of a class
-    // down to one of its members. If the member has no explicit visibility,
-    // the class visibility wins.
-    void mergeVisibility(Visibility V, bool E = false) {
-      // Never increase the visibility
-      if (visibility() < V)
-        return;
-
-      // If we have an explicit visibility, keep it
-      if (visibilityExplicit())
-        return;
+    /// Merge in the visibility 'newVis'.
+    void mergeVisibility(Visibility newVis, bool newExplicit) {
+      Visibility oldVis = visibility();
 
-      setVisibility(V, E);
-    }
-    // Merge the visibility V, keeping the most restrictive one.
-    // This is used for cases like merging the visibility of a template
-    // argument to an instantiation. If we already have a hidden class,
-    // no argument should give it default visibility.
-    void mergeVisibilityWithMin(Visibility V, bool E = false) {
-      // Never increase the visibility
-      if (visibility() < V)
+      // Never increase visibility.
+      if (oldVis < newVis)
         return;
 
-      // FIXME: this
-      // If this visibility is explicit, keep it.
-      if (visibilityExplicit() && !E)
+      // If the new visibility is the same as the old and the new
+      // visibility isn't explicit, we have nothing to add.
+      if (oldVis == newVis && !newExplicit)
         return;
 
-      // should be replaced with this
-      // Don't lose the explicit bit for nothing
-      //      if (visibility() == V && visibilityExplicit())
-      //        return;
-
-      setVisibility(V, E);
+      // Otherwise, we're either decreasing visibility or making our
+      // existing visibility explicit.
+      setVisibility(newVis, newExplicit);
     }
-    void mergeVisibility(LinkageInfo Other) {
-      mergeVisibility(Other.visibility(), Other.visibilityExplicit());
-    }
-    void mergeVisibilityWithMin(LinkageInfo Other) {
-      mergeVisibilityWithMin(Other.visibility(), Other.visibilityExplicit());
+    void mergeVisibility(LinkageInfo other) {
+      mergeVisibility(other.visibility(), other.visibilityExplicit());
     }
 
-    void merge(LinkageInfo Other) {
-      mergeLinkage(Other);
-      mergeVisibility(Other);
+    /// Merge both linkage and visibility.
+    void merge(LinkageInfo other) {
+      mergeLinkage(other);
+      mergeVisibility(other);
     }
-    void mergeWithMin(LinkageInfo Other) {
-      mergeLinkage(Other);
-      mergeVisibilityWithMin(Other);
+
+    /// Merge linkage and conditionally merge visibility.
+    void mergeMaybeWithVisibility(LinkageInfo other, bool withVis) {
+      mergeLinkage(other);
+      if (withVis) mergeVisibility(other);
     }
   };