erichkeane added inline comments.

================
Comment at: clang/lib/Basic/Targets/X86.cpp:1071
+    assert(llvm::is_contained(Priorities, Priority) &&
+           "Priorites don't form consequtive range!");
+  }
----------------
craig.topper wrote:
> erichkeane wrote:
> > If all you care about is whether they are a consecutive range, why not just 
> > use `std::is_sorted`?
> The Priorities array isn't sorted. It's just whatever order the 
> X86_FEATURE_COMPAT lists them.
> 
> The values need to be unique and in a contiguous range.
Then I'd suggest something like: `llvm::sort`, then `assert *(end - 1) - *begin 
== std::distance(begin, end) && llvm::adjacent_find` or something.

I definitely didn't get that point out of this odd for-loop and is_contained.  
There is perhaps at trick with std::min and std::max too.  Though, it looks 
like this is perhaps trying to prove that the range is 0 to the the array size, 
right?  In that case, perhaps there is something easier.

Also a nit, it is `consecutive` in that case.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108151/new/

https://reviews.llvm.org/D108151

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to