njames93 marked an inline comment as done. njames93 added inline comments.
================ Comment at: clang/lib/Tooling/NodeIntrospection.cpp:60 + if (LHS.first == RHS.first) + return LHS.second->name() < RHS.second->name(); + return LHS.first < RHS.first; ---------------- dblaikie wrote: > njames93 wrote: > > dblaikie wrote: > > > njames93 wrote: > > > > This is a slight change in behaviour, The old implementation used the > > > > operator< on a `std::shared_ptr`. > > > > That in turns boils down to a comparison on the raw pointer, which is > > > > less than ideal. > > > Why is that less than ideal? Does the ordering need to be stable? > > It's not strictly necessary that the ordering is stable. but when printing > > the contents of the map it would be nice if there was some obvious > > ordering. This also (partially) mimics the behaviour of the comparison for > > `std::pair<SourceRange, SharedLocationCall>`. > seems like a strange tradeoff to me for the LLVM codebase - we have lots of > unstable maps & don't try to make them nicer for printing - especially if > that would come at any performance cost, like doing string comparisons. > > > Does the SourceRange+SharedLocationCall need stability? The purpose behind this functionality is partly based on getting clang-query to show how to get display what source location accessors can be used for specific nodes(http://ce.steveire.com/z/V3k6Rl). As this is for a user facing and not a particularly hot path, I think its worth a couple extra cycles to give stability. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100378/new/ https://reviews.llvm.org/D100378 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
