octave-patch-tracker
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Octave-patch-tracker] [patch #8919] Start of patch to enable visibility


From: John W. Eaton
Subject: [Octave-patch-tracker] [patch #8919] Start of patch to enable visibility attributes for GCC in build system
Date: Wed, 16 Dec 2020 13:06:16 -0500 (EST)
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Firefox/78.0

Follow-up Comment #3, patch #8919 (project octave):

I started looking at visibility.  I'm attaching two changesets.  The first
allows me to almost build Octave using separate -DLIBNAME_DLL macros for each
shared library that we create.  But in the end it fails with the error:


/usr/bin/ld: libgui/.libs/liboctgui.so: undefined reference to
`Array<octave_value>::resize_fill_value() const'


That seems to be happening because the Array<T> class is marked as hidden when
we try to instantiate Array<octave_value> when building liboctinterp.  To work
around that problem, the original API tags that were added for MSVC were
applied to the template instantiations themselves, not the template
declarations.  But that doesn't seem to work with GCC, which issues messages
like


In file included from /home/jwe/src/octave/liboctave/array/Array-voidp.cc:35:
/home/jwe/src/octave/liboctave/array/Array.cc:2769:22: warning: attribute
ignored in explicit instantiation ‘class Array<void*>’ [-Wattributes]
 2769 |   template API class Array<T>
      |                      ^~~~~~~~
/home/jwe/src/octave/liboctave/array/Array-voidp.cc:45:1: note: in expansion
of macro ‘INSTANTIATE_ARRAY’
   45 | INSTANTIATE_ARRAY (void *, OCTAVE_API);
      | ^~~~~~~~~~~~~~~~~
/home/jwe/src/octave/liboctave/array/Array.cc:2769:22: note: no attribute can
be applied to an explicit instantiation
 2769 |   template API class Array<T>
      |                      ^~~~~~~~
/home/jwe/src/octave/liboctave/array/Array-voidp.cc:45:1: note: in expansion
of macro ‘INSTANTIATE_ARRAY’
   45 | INSTANTIATE_ARRAY (void *, OCTAVE_API);
      | ^~~~~~~~~~~~~~~~~


and doesn't change the instantiation to be visible.

So next I tried to just have one API macro (OCTAVE_API) instead of one for
each library.  The second changeset attached here does that (apply it in
addition to the first one).

This experiment raises a few questions for me:

Do we really need separate API macros for each library or can we just use
one?

In some public header files, we mark entire classes as visible but in others
we only mark a few functions as visible.

Marking only some functions in a public header file as visible seems bad to
me.  I would find it weird if I could compile something using Octave header
files without warning but not link my program because of symbol visibility. 
So it seems to me that we should mark everything in public header files as
visible.

We should only need visibility=hidden for functions that need to be shared
between object files in the same shared library (so they can't be declared
static within one file) but that are not considered part of the public
interface for the shared library.  Is that correct?

If we want to hide more of the Octave internals, then we need to create more
private header files in addition to applying visibility attributes.  The hard
part is deciding what interfaces should be public. 

(file #50492, file #50493)
    _______________________________________________________

Additional Item Attachment:

File name: visibility-patch-1.txt         Size:127 KB
    <https://file.savannah.gnu.org/file/visibility-patch-1.txt?file_id=50492>

File name: visibility-patch-2.txt         Size:154 KB
    <https://file.savannah.gnu.org/file/visibility-patch-2.txt?file_id=50493>



    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/patch/?8919>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/




reply via email to

[Prev in Thread] Current Thread [Next in Thread]