[PATCH] D75545: [dsymutil] Fix template stripping in getDIENames(...) to account for overloaded operators

Shafik Yaghmour via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 3 17:16:47 PST 2020


shafik added inline comments.


================
Comment at: llvm/lib/DWARFLinker/DWARFLinker.cpp:154
+      // as well.
+      num_left_angles_to_count += name.count("<=>");
+
----------------
JDevlieghere wrote:
> Can this operator occur more than once?
No


================
Comment at: llvm/lib/DWARFLinker/DWARFLinker.cpp:161
+      // or shift operators overloaded.
+      if (left_angle_count > right_angle_count)
+        num_left_angles_to_count += left_angle_count - right_angle_count;
----------------
JDevlieghere wrote:
> The comment says not balanced, but you use `>` instead of `!=`. Does that matter? 
I clarified the comment (hopefully)


================
Comment at: llvm/lib/DWARFLinker/DWARFLinker.cpp:169
+      Info.NameWithoutTemplate =
+          StringPool.getEntry(name.substr(0, start_of_template - 1));
+    }
----------------
JDevlieghere wrote:
> JDevlieghere wrote:
> > Given that we always strip the end, can we simplify the algorithm to something like
> > 
> > ```
> > size_t RightAngleCount = name.count('>');
> > size_t LeftAngleCount = name.count('<');
> > if (RightAngleCount != LeftAngleCount) {
> > 	size_t AngleDifference = std::abs(RightAngleCount - LeftAngleCount);
> > 	while (AngleDifference) {
> > 		name = name.take_front(name.rfind('<'));
> > 	}
> > } 
> > ```
> > 
> > That way we don't have to hardcode the `<=>` stuff. 
> Forgot to decrement AngleDifference: `while (AngleDifference--) {`
As discussed in person I am ignoring this comment.


================
Comment at: llvm/test/tools/dsymutil/X86/template_operators.test:23
+}
+
+struct B{};
----------------
JDevlieghere wrote:
> Please test `operator<=>` as well. 
Good call, I caught a bug where I was setting `NameWithoutTemplate` incorrectly in this case.


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

https://reviews.llvm.org/D75545





More information about the llvm-commits mailing list