supranational / blst

Multilingual BLS12-381 signature library
Apache License 2.0
467 stars 177 forks source link

Replace duplicate assembly.S with include #171

Closed jtraglia closed 1 year ago

jtraglia commented 1 year ago

The contents of this file can be replaced with a single include. I think this is simpler and less error prone.

dot-asm commented 1 year ago

Relative paths in #include-s are a no-go in my book. Because the standard is kind of ambiguous in that respect. Well, it says "implementation dependent" and formally speaking one can make a case that in this particular case the implementation is effectively known. In the sense that cgo makes certain assumptions about the C compiler it calls, and relative paths in #include are among those assumptions... At some point I've considered alternative names for the assembly.S and server.c in the go directory that would include their counterparts through -I${SRCDIR}. I'd still argue that it would be more appropriate. The original reason for why it wasn't done from the get-go was the desire to maintain a degree of freedom in the face of uncertainty. One can argue that we now know enough to realize that there is no uncertainty to "fear" :-)

dot-asm commented 1 year ago

alternative names for the assembly.S and server.c in the go directory

Just in case, yes, both. If you're concerned about rb_tree.c, then one can note that it is not actually called, but if unused code is a concern, it can be #ifndef __BLST_CGO__-ed.

jtraglia commented 1 year ago

Yes, I like that idea; renaming both files & doing non-relative includes.