sasagawa888 / eisl

ISLisp interpreter/compiler
Other
267 stars 22 forks source link

Fixes #287 compilation on macOS with clang 14 fails #289

Closed informatimago closed 1 year ago

informatimago commented 1 year ago

Note, review the corrections, you may want to edit some of them (eg. I changed the result type of:

    int set_dyn_env(int sym, int val);
    int add_dyn_env(int sym, int val);

but you may want to change the type of the fields where they're assigned instead.)

Also, in opengl.lsp, on macOS, we'd have to use frameworks instead of libraries. Unfortunately, I've found to way to test for the OS in eisl, so I commented out two lines for linux and added the two lines for macOS. You'll want to add a test to key both alternatives.

Some unused variables have been removed, and unused parameters have been declared unused with __attribute__((unused)), but you may want to use a macro for this instead.

I kept the:

     static const char __attribute__((unused)) rcsid[] = "$";

lines, but with git they've become useless. In the makefile, you may use the command:

    git rev-parse HEAD

to get the current git commit ID, and insert a single occurence of it in the binary. You may do it in a C variable, or using linker options to add a specific segment to the binary. See LDFLAGS in makefile. Then you may retrieve it with:

$ otool -s .data .gitCommitID eisl
eisl:
Contents of (.data,.gitCommitID) section
0000000130b14000    32 31 35 33 31 31 39 39 34 37 31 35 32 39 33 35 
0000000130b14010    38 66 38 61 34 36 33 65 66 65 37 61 65 63 34 35 
0000000130b14020    35 64 33 63 30 35 37 63

$ otool -s .data .gitCommitID eisl|sed -e s'/^[0-9a-f]* //'|xxd -r -p 
21531199471529358f8a463efe7aec455d3c057c

Note: if you were to use the ld.lld linker that comes with llvm with clang, instead of the system ld linker, you'd have to change the LDFLAGS option.

sasagawa888 commented 1 year ago

Thank you. But error.

cii/src/mem.c:36:63: warning: unused parameter ‘file’ [-Wunused-parameter]
   36 | void Mem_free(void *ptr, const char * __attribute__((unused)) file , int __attribute__((unused)) line ) {
      |                          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
cc -Wl,-sectcreate,.data,.gitCommitID,"/tmp/tmp.Ks56P5X7bT" -flto -s main.o function.o extension.o syntax.o data.o gbc.o cell.o error.o bignum.o compute.o edit.o syn_highlight.o long.o link.o cii/src/except.o cii/src/fmt.o cii/src/str.o cii/src/text.o cii/src/mem.o -o eisl -lm -ldl -Wl,-Bsymbolic-functions -lncursesw -ltinfo -lpthread
/usr/bin/ld: cannot find .data: No such file or directory
/usr/bin/ld: cannot find .gitCommitID: No such file or directory
/usr/bin/ld:/tmp/tmp.Ks56P5X7bT: file format not recognized; treating as linker script
/usr/bin/ld:/tmp/tmp.Ks56P5X7bT:0: syntax error
collect2: error: ld returned 1 exit status
make: *** [makefile:142: eisl] Error 1
sasagawa888 commented 1 year ago

I merged your pull request, but it caused the code to no longer work on Linux. I have reverted it back to its original state. I apologize for any inconvenience caused

informatimago commented 1 year ago

Obviously, you need to select clang instead of gcc.

You can use the xcode-select command to switch between clang and gcc in macOS.

To switch to gcc, run:

sudo xcode-select --switch /Library/Developer/CommandLineTools

To switch back to clang, run:

sudo xcode-select --switch /Applications/Xcode.app

Note that Xcode needs to be installed for either compiler to work.

Normally, attribute((unused)) is gcc syntax that clang accepts. But if you have a compiler that rejects it, you would want to use a macro instead, as mentioned in the message, that expands what is correct for that compiler.

#if defined(__clang__) || defined(__gnu__)
#define UNUSED __attribute__((unused))
#else
#define UNUSED
#endif

and use UNUSED instead of attribute((unused)) in the source.

Similarly for LDFLAGS in the makefile, you would want to test the linker ld and determine if it's gcc's ld or clang ld.lld or something else, and use an option specific to the linker. (clang takes a --use-ld option to specify which linker to use; by default it uses ld like gcc).

You could put something like this in the makefile:

if $(eq,$(shell $(CC) --version | grep -q  -s "clang"; echo $$?),0))
LDFLAGS += -Wl,-sectcreate,.data,.gitCommitID,"$(shell f=$$(mktemp) ; git rev-parse HEAD|tr -d '\012' > $$f ; echo $$f)"
elif $(eq,$(shell $(CC) --version | grep -q  -s "gcc"; echo $$?),0))
LDFLAGS += -Wl,-sectcreate,.data,.gitCommitID,"$(shell f=$$(mktemp) ; git rev-parse HEAD|tr -d '\012' > $$f ; echo $$f)"
else
$(info "I don't know how to add a section with the linker for this compiler.")
endif
informatimago commented 1 year ago

Also, perhaps you have a clang, but not the same version, and perhaps it handles attribute((unused)) differently? You could try to put it before the const char* type or after the file parameter name.

And you should mention the Xcode-select stuff in the README.

Also, on linux (or even on macOS and other OSes), user may want to select a compiler and linker just setting CC and LD.

CC=clang LD=ld.lld make install vs. CC=gcc LD=ld make install

You may also add a suffix or a prefix on the executable name to indicate with which compiler it was generated, so you may compile both and compare them (you should get the exact same results).

CC=clang LD=ld make install EXESUFFIX=-clang-ld && ./eisl-clang-ld

This is an option that should work, and be documented.