[PATCH 3/5] scanner: introduce --object-type option

[Emil Velikov]

On 26 January 2018 at 02:44, Jonas Ådahl <jadahl at gmail.com> wrote:
> On Thu, Jan 25, 2018 at 05:56:45PM +0000, Emil Velikov wrote:
>> On 25 January 2018 at 02:01, Jonas Ådahl <jadahl at gmail.com> wrote:
>> > On Wed, Jan 24, 2018 at 07:15:09PM +0000, Emil Velikov wrote:
>> >> On 24 January 2018 at 18:20, Derek Foreman <derekf at osg.samsung.com> wrote:
>> >> > On 2018-01-22 09:30 AM, Emil Velikov wrote:
>> >> >>
>> >> >> On 22 August 2017 at 14:02, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> >> >>>
>> >> >>> On 18 August 2017 at 13:05, Pekka Paalanen <ppaalanen at gmail.com> wrote:
>> >> >>>
>> >> >>>>>>
>> >> >>>>>> The exported configuration would then be:
>> >> >>>>>> LOCAL_INTERFACE_DECL=extern
>> >> >>>>>> EXTERN_INTERFACE_DECL=extern
>> >> >>>>>> LOCAL_INTERFACE_DEF=WL_EXPORT
>> >> >>>>>>
>> >> >>>>>> That would be far too flexible and no-one would use it right, right?
>> >> >>>>>>
>> >> >>>>> I did not introduce separate tokens, since those are (and should be)
>> >> >>>>> used _only_ in the .c file.
>> >> >>>>> Personally then do not seem necessary, If you prefer we can add them
>> >> >>>>> though.
>> >> >>>>
>> >> >>>>
>> >> >>>> Ah, no, that was just a wild idea of something completely different. I
>> >> >>>> meant that the user project would be setting those macros before using
>> >> >>>> scanner-generated files, and if unset, the scanner-emitted code would
>> >> >>>> default to the legacy behaviour. That way there would be no visibility
>> >> >>>> modes in scanner itself. If it's not obviously better, then nevermind.
>> >> >>>> It certainly has a lot more room to go wrong than your proposal.
>> >> >>>>
>> >> >>>>
>> >> >>> I see.
>> >> >>>
>> >> >>> Personally I'd lean towards with my approach for now since it is
>> >> >>> simpler, despite that it provides less flexibility.
>> >> >>> As you pointed out the proposal is a bit more fragile, so might be
>> >> >>> better to avoid until there's a real need for it.
>> >> >>>
>> >> >>>
>> >> >>>> ...
>> >> >>>>
>> >> >>>>>> The patch looks pretty much correct to me, if we choose to go this
>> >> >>>>>> way.
>> >> >>>>>>
>> >> >>>>> Glad to hear.
>> >> >>>>>
>> >> >>>>> I'll let me know once you guys are settled in on the approach, and
>> >> >>>>> I'll respin the series with all the comments addressed.
>> >> >>>>
>> >> >>>>
>> >> >>>> Cool, let's see if we can get the name conflict issue solved, and then
>> >> >>>> I'll try to remember to ping you.
>> >> >>>>
>> >> >>> Ack, I'll keep an eye open, just in case.
>> >> >>>
>> >> >> Considering the status of the the name conflict series, should I
>> >> >> re-spin this lot?
>> >> >> I'm more than happy to tweak things - say rename the toggle, etc.
>> >> >
>> >> >
>> >> > I see there were two series proposed to control symbol visibility, yours and
>> >> > Jonas'?
>> >> >
>> >> > Assuming that once we drop the symbol collision issue they both solve the
>> >> > same problems, it would be good if we could focus on one going forward.
>> >> >
>> >> > Is this the chosen one?
>> >> >
>> >> Right, the cover letter [1] covers some of the high-lights/differences.
>> >> As a TL;DR: using static/shared is more common and gives us more
>> >> flexibility for the future.
>> >>
>> >> So far Pekka is the only person who commented on the series/approach
>> >> and seemed happy.
>> >> I was expecting others to weight in - say Jonas ;-) I'll respin the
>> >> series tomorrow.
>> >>
>> >> In hindsight --object-type={shared,static} is too much of a mouthful,
>> >> I'll opt for --{static,shared} for v2.
>> >
>> > I have no opinion of what variant to land (I'm slightly too lazy to
>> > search for my own, and this is high up the inbox thanks to the replies,
>> > so lets focus on this one).
>> >
>> > My only nit is using the term "object-type". I think refering to it as
>> > "visibility" ("symbol visibility" if wanting to be extra verbose) where
>> > one can say 'export', 'static' or 'private' is more accurate.
>> >
>> > "objects" are a Wayland protocol thing and that is not what we are
>> > poking at here.
>> >
>> Right using "object" might be misleading. On the other hand,
>> --shared/--static are well known and dead-obvious.
>> Plus it gives us flexibility to sweep more things under it, if we get
>> some funky stuff in the future.
>>
>> Can I buy you on the different name?
>
> --static is pretty clear, but --shared? Both WL_EXPORT and WL_PRIVATE
> are "shared" but just different audiences.
>
The actual symbol notation WL_EXPORT/WL_PRIVATE/other(?) is an
implementation detail.
That is "hidden" within the C files and never exposed outside.

There are two reasons for that:
 - used internally, thus no point in polluting the namespace
 - some compilers (older clang or was it the oracle/sun one?) are
unhappy if the header is annotates, while the C file isn't

-Emil