[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: master b2416d2c029 4/6: Don't load comp when installing an existing
From: |
Andrea Corallo |
Subject: |
Re: master b2416d2c029 4/6: Don't load comp when installing an existing trampoline |
Date: |
Thu, 16 Nov 2023 04:34:25 -0500 |
User-agent: |
Gnus/5.13 (Gnus v5.13) |
Andrea Corallo <acorallo@gnu.org> writes:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>>>> - The `declare-function` should be moved to right after (require
>>>> 'comp-run), i.e. when we do know that the function should be available
>>>> and it will thus silence only spurious warnings.
>>>
>>> Ack will do, out of curiosity what is the downside of having the
>>> declare-function at top level? I thought is there to silence a compile
>>> time warning (and thus the branch is inserted in should not play a
>>> role).
>>
>> If you call that function from elsewhere in the file, there will be no
>> compilation warning, even though the call may error at run time because
>> `comp-run` wasn't loaded.
>>
>> By moving the `declare-function` you make sure only those warnings that
>> you know for sure are spurious (because we just did `(require
>> 'comp-run)`) will get silenced.
>
> Thanks for the explaination, done.
>
>>> As mentioned in the other thread I fear I don't remeber the answer to
>>> those quesitons. The best we can do is to remove the call and test
>>> Emacs to find if there's still a good reason.
>>
>> Then let's remove it in `master`. This way, if it's needed we'll get to
>> see why/when and we'll be able to at the very least add a good comment,
>> or maybe replace it with a better solution.
>
> Ack, I'll try it before in my machine and push it if I see no
> regressions here.
Okay doing some git archeology I discovered that at the time during
development when I added the code into 'advice--add-function' we had no
handling in Fset. This makes me more confident at thinking that the
special handling is not necessary. Tests are passing so I pushed the
clean-up.
Thanks!
Andrea