search

epics-modules / asyn

EPICS module for driver and device support
http://epics-modules.github.io/asyn/
Other
37 stars 74 forks source link

Remove paramTableSize argument from asynPortDriver constructor #52

Closed mark0n closed 7 years ago

mark0n commented 7 years ago

I like the improvements @mdavidsaver made recently (see #35 and #36) and I would like to propose to take this to the next level by removing the unused paramTableSize argument completely. Of course this means that existing code needs to be converted at some point. I would like to suggest the following approach to make this process as painless as possible:

  1. Add a new constructor without the paramTableSize parameter. This one would be the recommended one for new applications. Mark the old one as deprecated (e.g. using GCC's __attribute__((deprecated))). Thus users that are building their code and have not converted their constructor calls, yet, would see a warning. Document this in the change log for the next release.
  2. At some point in the future remove the deprecated constructor.
mark0n commented 7 years ago

I just realized that some unrelated changes managed to sneak into my branch. I just cleaned things up.

I recently also came across the EPICS_DEPRECATED macro. It gets replaced by nothing if the compiler does not support this feature (GCC and clang both support it). Unfortunately it doesn't allow us to provide detailed instructions to the users. Let me know how you prefer to handle this.

Engbretson commented 7 years ago

Is there any way that the EPICS_DEPRECATED macro could be configured to work like CHECK_RELEASE? So instead of just code or no code, there is an option to . . . I don’t know . . . what is a portable way to #PRAGMA WARNING or #PRAGMA ERROR? Something that would at least stop compilation and force someone to look at what code is not going to be included?

If there is no problem removing code if the compiler doesn’t support it . . . . and everything still works afterwards . . . . then why put in that complier specific code in the first place?

From: Martin Konrad [mailto:notifications@github.com] Sent: Tuesday, April 11, 2017 11:43 AM To: epics-modules/asyn asyn@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Subject: Re: [epics-modules/asyn] Remove paramTableSize argument from asynPortDriver constructor (#52)

I just realized that some unrelated changes managed to sneak into my branch. I just cleaned things up.

I recently also came across the EPICS_DEPRECATED macro. It gets replaced by nothing if the compiler does not support this feature (GCC and clang both support it). Unfortunately it doesn't allow us to provide detailed instructions to the users. Let me know how you prefer to handle this.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/epics-modules/asyn/issues/52#issuecomment-293323497 , or mute the thread https://github.com/notifications/unsubscribe-auth/AM66ixIlsEHIvFJ3xQbPimDvHLdeABlkks5ru62vgaJpZM4MYtkz . https://github.com/notifications/beacon/AM66i0ibF_azvuLlWu2TCVAkznRRHGvhks5ru62vgaJpZM4MYtkz.gif

ralphlange commented 7 years ago

IMHO, using deprecated functions must not stop compilation. The whole idea of deprecation (see https://en.wikipedia.org/wiki/Deprecation) is to warn the user without prohibiting the use of the deprecated feature.

The macro is not removing code. It expands to __attribute__((deprecated)) or something equivalent on compilers that support this, and expands to nothing on compilers that don't.

MarkRivers commented 7 years ago

I recently also came across the EPICS_DEPRECATED macro. It gets replaced by nothing if the compiler does not support this feature (GCC and clang both support it). Unfortunately it doesn't allow us to provide detailed instructions to the users. Let me know how you prefer to handle this.

I merged your pull request, but found that it would not compile on many platforms, including vxWorks (gcc) and Windows (VS2010), because of the attribute(deprecated) with an argument. I replaced it with EPICS_DEPRECATED. You are correct that it lacks the detailed instructions, but it does print a message.