bmahjour added inline comments.
================
Comment at: llvm/include/llvm/ADT/DirectedGraph.h:40
/// Static polymorphism: delegate implementation (via isEqualTo) to the
/// derived class.
+ bool operator==(const DGEdge &E) const {
----------------
jfb wrote:
> That comment, so informative! 😐
It would be better to make these non-member functions as well, to be consistent
with the `DGNode`.
```
friend bool operator==(const EdgeType &Elhs, const EdgeType &Erhs) {
return Elhs.isEqualTo(Erhs);
}
friend bool operator!=(const EdgeType &Elhs, const EdgeType &Erhs) {
return !(Elhs == Erhs);
}
```
================
Comment at: llvm/include/llvm/ADT/DirectedGraph.h:40
/// Static polymorphism: delegate implementation (via isEqualTo) to the
/// derived class.
+ bool operator==(const DGEdge &E) const {
----------------
bmahjour wrote:
> jfb wrote:
> > That comment, so informative! 😐
> It would be better to make these non-member functions as well, to be
> consistent with the `DGNode`.
> ```
> friend bool operator==(const EdgeType &Elhs, const EdgeType &Erhs) {
> return Elhs.isEqualTo(Erhs);
> }
> friend bool operator!=(const EdgeType &Elhs, const EdgeType &Erhs) {
> return !(Elhs == Erhs);
> }
> ```
> That comment, so informative! 😐
Yeah, it's not the best comment. It is trying to say that //the `isEqualTo`
function gets selected, based on the type of the derived class in the CRTP, and
that the selection is done at compile-time using type information, rather than
at runtime using dynamic polymorphism//. Please feel free to update it to say
that or offer any other suggestions you might have.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78938/new/
https://reviews.llvm.org/D78938
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
