tools icon indicating copy to clipboard operation
tools copied to clipboard

Dustmite/polyhash.d: Surround gcc assembly operands in parentheses

Open the-horo opened this issue 1 year ago • 5 comments

Compiling with recent versions of gdc results in errors:

DustMite/polyhash.d-mixin-289:293:13: error: ‘low’ must be surrounded by parentheses
DustMite/polyhash.d-mixin-289:293:23: error: ‘high’ must be surrounded by parentheses
DustMite/polyhash.d-mixin-289:294:12: error: ‘a’ must be surrounded by parentheses
DustMite/polyhash.d-mixin-289:294:20: error: ‘b’ must be surrounded by parentheses
DustMite/polyhash.d:165:37: error: template instance ‘polyhash.longMul!ulong’ error instantiating
  165 |                 this.value = longMul(this.value, operand.value).longDiv(q).remainder;
      |                                     ^
DustMite/polyhash.d:32:35: note: instantiated from here: ‘opOpAssign!"*"’
   32 |                                 v *= v;
      |                                   ^
DustMite/splitter.d:50:28: note: instantiated from here: ‘PolynomialHash!(ModQ!(ulong, 18446744073709551557LU))’
   50 |         alias EntityHash = PolynomialHash!(ModQ!(ulong, largest64bitPrime));
      |                            ^
DustMite/polyhash.d-mixin-362:366:13: error: ‘quotient’ must be surrounded by parentheses
DustMite/polyhash.d-mixin-362:366:28: error: ‘remainder’ must be surrounded by parentheses
DustMite/polyhash.d-mixin-362:367:12: error: ‘low’ must be surrounded by parentheses
DustMite/polyhash.d-mixin-362:367:21: error: ‘high’ must be surrounded by parentheses
DustMite/polyhash.d-mixin-362:367:32: error: ‘b’ must be surrounded by parentheses
DustMite/polyhash.d:165:72: error: template instance ‘polyhash.longDiv!(ulong, LongInt!(64u, false))’ error instantiating
  165 |                this.value = longMul(this.value, operand.value).longDiv(q).remainder;
      |                                                                       ^

DustMite/polyhash.d:32:35: note: instantiated from here: ‘opOpAssign!"*"’
   32 |                                 v *= v;
      |                                   ^
DustMite/splitter.d:50:28: note: instantiated from here: ‘PolynomialHash!(ModQ!(ulong, 18446744073709551557LU))’
   50 |         alias EntityHash = PolynomialHash!(ModQ!(ulong, largest64bitPrime));
      |                            ^

the-horo avatar Apr 25 '24 14:04 the-horo

Thanks for your pull request and interest in making D better, @the-horo! We are looking forward to reviewing it, and you should be hearing from a maintainer soon. Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + tools#468"

dlang-bot avatar Apr 25 '24 14:04 dlang-bot

Thanks, could you please also submit this upstream?

CyberShadow avatar Apr 25 '24 14:04 CyberShadow

Thanks, could you please also submit this upstream?

It seems it has been fixed in https://github.com/CyberShadow/DustMite/commit/b8703eb98e93c063d8c7c75e964f5625b1200f16

the-horo avatar Apr 25 '24 15:04 the-horo

Ah, I guess we're just past due for a sync then.

CyberShadow avatar Apr 25 '24 15:04 CyberShadow

Ah, I guess we're just past due for a sync then.

Here it is: https://github.com/dlang/tools/pull/476

CyberShadow avatar Sep 19 '24 09:09 CyberShadow