text: Use cbindgen for text-overflow
  .
  Categories
  (Core :: CSS Parsing and Computation, defect)
  Tracking
  ()
  People
  (Reporter: emilio, Assigned: violet.bugreport)
  References
  Details
  Attachments
  (2 files)
  The way to fix this would be:
  - Mark
  TextOverflowSidewith
  #[repr(C, u8)], and change the
  Box<str>for
  crate::OwnedStr:
  - Mark
  computed::TextOverflowwith
  #[repr(C)]:
  - Add
  TextOverflowto the list here:
  - ... And a line like
  { gecko = "StyleTextOverflow", servo = "values::computed::TextOverflow" },here:
  - Then change the style struct to have a
  mozilla::StyleTextOverflowinstead of
  nsStyleTextOverflow
  - Rewrite the code that uses
  nsStyleTextOverflowto use
  mozilla::StyleTextOverflow. You could put the helpers that
  nsStyleTextOverflowhas in a section like this:, or inline them as static functions in the only file that uses them.
  - Remove
  text-overflowand the text-overflow-specific code from:
  - Remove
  nsStyleTextOverflowand co.
  - Profit?
  Violet, would you be interested in taking this since you mentioned some interest in it?
  This is the one I found that wasn't either too trivial or too complex, and you get to remove a good chunk of code :)
  I outlined what has to be done above, but I may have goofed somewhere, definitely let me know if you get stuck or what not.
  Thanks!
  Thanks, that's very helpful!
  I think we'll need to add default c'tor support for
  union in
  cbindgen first, because this seems to be the first time a non-POD member is inside a union (
  OwnedStr in
  TextOverflowSide). C++ will delete the default c'tor for union if it has a member with nontrivial default c'tor, the generated code won't compile:
  static StyleTextOverflowSide String(const StyleOwnedStr &a0) { StyleTextOverflowSide result; ::new (&result.string._0) (StyleOwnedStr)(a0); result.tag = Tag::String; return result; }
  This patch to
  cbindgen should be ok.
  No need to, see:
  Your patch looks ok, but probably the destructor should be private to avoid footgunning somebody. That's why I've been reluctant to add that functionality to cbindgen.
  Err, the constructor, I mean.
  Oh, I see. So the patch isn't necessary. Do you think I should submit a PR to
  cbindgen?
  A probably trivial question: what's the recommended way to modify a third-party rust package and see how it works in gecko locally? My current approach looks quite awkward. I have to modify this line to use the path of my local
  cbindgen repo in order to test whether it works in gecko.
  You can put
  CBINDGEN=/path/to/cbindgen in your mozconfig and that should do the trick. If you need to push to try with your changes, you need to also change: and point it to your repo as well.
  For other third-party rust packages it's a bit easier, since they're not expected to be built before the export phase. For example, if you wanted to modify
  cssparser, you could add
  cssparser = {path = "third_party/rust/cssparser"} to:
  I think submitting a PR to cbindgen is not really necessary, though if this pattern becomes really frequent we could consider adding it.
  Pushed by violet.bugreport@gmail.com: Use cbindgen for text-overflow r=emilio
  Backed out changeset 3304277f5bde (bug 1553252) for build bustages on Windows
  Push that started the failures:
  Failure log:
  Backout:
  Pushed by violet.bugreport@gmail.com: Use cbindgen for text-overflow r=emilio
  That
  using namespace.
  (In reply to violet.bugreport from comment #14)
  That
  using namespace.
  We use unified builds, so if different cpp files are built, they may change how the final compilation unit looks like. That's probably what's happening.