Closed pthom closed 3 years ago
Oh! :grimacing: Thanks a lot for noticing and reporting. :slightly_smiling_face:
I wonder if it would make sense to not only do this auto-generation step automatically when building, but also the fwd/curry stuff. WDYT?
I wonder if it would make sense to not only do this auto-generation step automatically when building, but also the fwd/curry stuff. WDYT?
I think it would be a good idea! I would be interested in giving this a try. Would you like me to propose something around this in this PR?
Sure, I'd be happy if you give it a go. :+1:
Maybe @friendlyanon would like to also have a look at it then, since it will happen close to the recent CMake adjustments? :slightly_smiling_face:
@friendlyanon : I took into account your suggestions. I also added a call to generate_derived_functions.sh
with the same philosophy.
@Dobiasd :
generate_derived_functions.sh
is now called at each build. However this will work under linux and MacOS, but not windows.
If you look at its code, you will see that I had to adapt it for MacOS (where sed is sadly not GNU compatible). Also, in the case of MacOS, I had to install gsed via brew install gnu-sed
in the github ci file.
You will also see that I tried to adapt the contributing.md doc.
Perfect, thanks a lot! (Also for adjusting the docs. :heart:) I've just checked out your branch locally, and the auto-generation during build works fine here too. :+1:
So I guess we're ready to merge, right?
For the long-term, we could think about if we'd like to replace my weird cat | grep | perl | sed
chain with something less insane. :grin:
Does the shell script use the compiler just for resolving the #include
s? If yes, then you can do the same with just a Python script and that's be cross-platform as well.
@friendlyanon
Does the shell script use the compiler just for resolving the
#include
s?
Yes, it does:
g++ -E -CC -std=c++14 -I../include ../include/fplus/fplus.hpp > temp_preprocessor_output.txt
cat temp_preprocessor_output.txt | ...
If yes, then you can do the same with just a Python script and that's be cross-platform as well.
Yeah, that would be better. :+1:
I added some more commits, which summarize to:
we do not use 'gcc -E' anymore for derived function script; instead we use the result of the make_all_in_one.py script
Timing results:
After this commit (only one "auto_generate" target)
> time make
Built target auto_generate
make 0.15s user 0.13s system 116% cpu 0.329 total
Before this commit (two targets)
> time make
Built target generate_derived_functions
-- Generating include_all_in_one
Built target include_all_in_one
make 0.41s user 0.13s system 111% cpu 0.488 total
Here, I refactored the bash code generation part, in order to prepare a possible migration to python.
The heart of the algorithm is here:
function perform_code_generation () {
if [ "$#" -ne 3 ]; then
echo "perform_code_generation: illegal number of parameters"
exit 1
fi
dst_filename=$1
search_token=$2
replace_token=$3
cat $ALL_IN_ONE_FILE \
| grep -B 1 -e "// $search_token" \
| perl -pe "s/^...//g" \
| $SED_CMD ':a;N;$!ba;s/\n/ /g' \
| perl -pe "s/ -- /\n/g" \
| perl -pe "s/API search type: ([^ ]*).*fwd bind count: ([0-9])/$replace_token\2(\1)/g" \
>> $dst_filename
}
However, I did not try to decipher the sed part; and I stopped here.
Ok, give me some time to finish the port to python :-)
For the record the sed script is just:
:a # create label a
N # append next input line to pattern space
$!ba # branch to label a on every line but the last one
s/\n/ /g # replace \n with a space everywhere in the pattern space
This could've just been paste -s -d " "
.
Ok, the generate part is now full python. The code is heavily documented and should be (I hope) quite readable. See here
I also updated the doc.
Do you want me to rewrite this PR history in one or two commits?
Ok, the generate part is now full python.
Awesome! :rocket: :rocket: :rocket:
The code is heavily documented and should be (I hope) quite readable.
Yes, I think so too. :+1:
The sed
version was very terse, but unreadable "write-only code". :grimacing:
I also updated the doc.
:heart:
Do you want me to rewrite this PR history in one or two commits?
I don't have a strict policy regarding the commit history for this repository. But if you'd like your contributions to look neater, feel free to rewrite the history. :slightly_smiling_face:
The
sed
version was [...] "write-only code"
I wouldn't say that, took me one or two minutes cross-referencing the man pages to turn it back into english. It just comes down to familiarity, someone who does some more sed scripting will know it right away, as sed doesn't have a huge number of commands, so it's relatively easy to keep in mind.
OK, I rewrote the history into one commit. If everyone agrees, it is ok to merge IMHO.
Hello,
I saw that the generation of the file
include_all_in_one/include/fplus/fplus.hpp
is not included anymore in the make stepThis file is part of the repository so that it is easily accessible via an URL, for easy install via only one file, or for use on tools such as compiler explorer.
I suspect this happened during the merge of the PR "Improve CMake integration", since this PR vastly refactored the cmake scripts.
So, I open this PR as a way to discuss what can be done on this subject:
This PR proposes to restore the original status: this file is generated automatically at each build. The advantage is that developers will not forget to commit it, since it is refreshed automatically for them
However, we could imagine less intrusive ways to do it: either by mentioning inside CONTRIBUTING the need to call manually
cd include_all_in_one && python3 make_all_in_one.py && cd ..
for each PR, or by calling this script during other deploy scripts (such ascompile_all_and_deploy.sh
orgenerate_derived_functions.sh
)What do you think?
Note: this PR also update the
include_all_in_one/include/fplus/fplus.hpp
with the latest modifications.