[PATCH] D21000: [CFLAA] Cleaned up StratifiedAttrs handling

[George Burgess IV via llvm-commits]

george.burgess.iv added a comment.

Thanks for the patch!

A few high-level things:

- This needs tests. Ideally at least one for each logical change that's being made (though I'm not sure how testable the overload-related issue would be). :)
- We generally request that people try to make reviews as targeted as possible. So, if you could please split patches like these out into N different reviews in the future, that would be great!

---

> There are two overloaded version of StratifiedSets::noteAttribute


One is `noteAttribute`, the other is `noteAttributes`, so I don't think overload resolution should be kicking in at all here. :)


================
Comment at: lib/Analysis/CFLAliasAnalysis.cpp:1100
@@ -1098,2 +1099,3 @@
   // argument or global, then we don't have to be as conservative.
-  if (AttrsA.any() && AttrsB.any())
+  if ((AttrsA.none() && AttrsB.any()) || (AttrsA.any() && AttrsB.none()))
+    return NoAlias;
----------------
This looks a bit subtle. I think we can make it less so if we move the index number check above this. If we do that, it seems that we can simplify this entire if/else chain to something like

```
if (a.none() || b.none() || (a == Escaped && b == Escaped))
  return NoAlias;
return MayAlias;
```

Right?

================
Comment at: lib/Analysis/CFLAliasAnalysis.cpp:1102
@@ +1101,3 @@
+    return NoAlias;
+  else if (AttrsA.test(AttrUnknownIndex) || AttrsB.test(AttrUnknownIndex))
+    return MayAlias;
----------------
Style nit:

```
if (foo)
  return bar;
if (baz)
  ...
```

Is preferred over

```
if (foo)
  return bar;
else if (baz)
  ...
```

================
Comment at: lib/Analysis/CFLAliasAnalysis.cpp:1105
@@ +1104,3 @@
+  else if ((AttrsA & StratifiedAttrs(AttrEscaped)).any() &&
+           (AttrsB & StratifiedAttrs(AttrEscaped)).any())
+    return NoAlias;
----------------
`AttrsA.test(AttrEscapedIndex) && AttrsB.test(AttrEscapedIndex)`?

Either way, this looks like it'll hand us NoAlias if I have e.g. two sets marked with every StratifiedAttr except AttrUnknown.

================
Comment at: lib/Analysis/StratifiedSets.h:436
@@ -435,3 @@
-    auto &Link = linksAt(Info->Index);
-    Link.setAttr(AttrNum);
-  }
----------------
Does `Link.setAttr` have any uses aside from this one? If not, we should remove it, too. :)


http://reviews.llvm.org/D21000