Skip to content

Make use of IGL_DEPRECATED#1380

Merged
jdumas merged 6 commits intolibigl:devfrom
BruegelN:use-IGL_DEPRECATED
Dec 10, 2019
Merged

Make use of IGL_DEPRECATED#1380
jdumas merged 6 commits intolibigl:devfrom
BruegelN:use-IGL_DEPRECATED

Conversation

@BruegelN
Copy link
Copy Markdown
Contributor

@BruegelN BruegelN commented Dec 10, 2019

  • Use the macro in igl::all_edges, igl::internal_angles and igl::is_border_vertex.
    CTRL+F for "deprecated" to find functions where the comment says they're deprecated.

  • Removed #include deprecated.h for igl::nchoosek and igl::readOBJ because it looks like the functions which have been marked as deprecated are already gone. See igl::nchoosek and igl::readOBJ

The compiler warning will look sth. like below if you use a deprecated func.

std::vector<bool> igl::is_border_vertex(const Eigen::MatrixBase<Derived>&, const Eigen::MatrixBase<U>&) [with DerivedV = Eigen::Matrix<double, -1, -1>; DerivedF = Eigen::Matrix<int, -1, -1>]’ is deprecated [-Wdeprecated-declarations]
   15 |   igl::is_border_vertex(V,F);
      |                            ^
In file included from include/igl/is_border_vertex.h:11,
                 from main.cpp:2:
include/igl/is_border_vertex.h:32:32: note: declared here
   32 |   IGL_INLINE std::vector<bool> IGL_DEPRECATED(is_border_vertex(
      |                                ^~~~~~~~~~~~~~
main.cpp:15:28: warning: ‘std::vector<bool> igl::is_border_vertex(const Eigen::MatrixBase<Derived>&, const Eigen::MatrixBase<U>&) [with DerivedV = Eigen::Matrix<double, -1, -1>; DerivedF = Eigen::Matrix<int, -1, -1>]’ is deprecated [-Wdeprecated-declarations]
   15 |   igl::is_border_vertex(V,F);
      |                            ^
In file included from include/igl/is_border_vertex.h:11,
                 from main.cpp:2:
include/igl/is_border_vertex.h:32:32: note: declared here
   32 |   IGL_INLINE std::vector<bool> IGL_DEPRECATED(is_border_vertex(

Check all that apply (change to [x])

  • All changes meet libigl style-guidelines.
  • Adds new .cpp file.
  • Adds corresponding unit test.
  • This is a minor change.

Nico Brügel added 2 commits December 10, 2019 13:12
Make use of IGL_DEPRECATED() macro in igl::all_edges, igl::internal_a…
92a39ff 
…ngles and igl::is_border_vertex.

CTRL+F for "deprecated" find functions where the comment says that the functioin is deprecated.
Removed #include deprecated.h for igl::nchoosek and igl::readOBJ beca…
6f0aa32 
…use it looks like the functions which have been marked as deprecated are already gone.
@jdumas
Copy link
Copy Markdown
Collaborator

jdumas commented Dec 10, 2019 •
edited
Loading

Thanks! I would also vote for marking igl::remove_duplicates() as deprecated in favor of igl::remove_duplicate_vertices() (or vice-versa, @alecjacobson I never know which one I should use?).

Additionally I think we should revise the IGL_DEPRECATED macro. I don't see the need to make it take an argument (since we are mostly deprecating functions). Also, in C++14, we can make use of the official [[deprecated]] macro which should be compatible with more compilers. Taking example from fmt, I think we can revise the macro as follows:

#ifdef __has_cpp_attribute
#  define IGL_HAS_CPP_ATTRIBUTE(x) __has_cpp_attribute(x)
#else
#  define IGL_HAS_CPP_ATTRIBUTE(x) 0
#endif

#ifdef _MSC_VER
#  define IGL_MSC_VER _MSC_VER
#else
#  define IGL_MSC_VER 0
#endif

#ifndef IGL_DEPRECATED
#  if (IGL_HAS_CPP_ATTRIBUTE(deprecated) && __cplusplus >= 201402L) || \
      IGL_MSC_VER >= 1900
#    define IGL_DEPRECATED [[deprecated]]
#  else
#    if defined(__GNUC__) || defined(__clang__)
#      define IGL_DEPRECATED __attribute__((deprecated))
#    elif IGL_MSC_VER
#      define IGL_DEPRECATED __declspec(deprecated)
#    else
#      pragma message("WARNING: You need to implement IGL_DEPRECATED for this compiler")
#      define IGL_DEPRECATED /* deprecated */
#    endif
#  endif
#endif

@BruegelN
Copy link
Copy Markdown
Contributor Author

BruegelN commented Dec 10, 2019

Thank you for your feedback!
I like the suggested rework of IGL_DEPRECATED. Good point!
As far as I know igl::remove_duplicates() is not used by other libigl functions, I only found internal usage of igl::remove_duplicate_vertices() on quick searching for the functions names. So i think igl::remove_duplicates() is the deprecated one. If @alecjacobson confirms this I'll add a IGL_DEPRECATED to igl::remove_duplicates().

@jdumas
Copy link
Copy Markdown
Collaborator

jdumas commented Dec 10, 2019

Awesome. I've edited my code snippet compared to the msg you might have received by email, just FWI (adding pragma message + macro for MSVC).

@jdumas
Copy link
Copy Markdown
Collaborator

jdumas commented Dec 10, 2019

I just got the ok from Alec to mark igl::remove_duplicates as deprecated @BruegelN, please go ahead.

@jdumas jdumas merged commit 5d9d9c8 into libigl:dev Dec 10, 2019
@BruegelN BruegelN deleted the use-IGL_DEPRECATED branch October 1, 2020 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jdumas jdumas jdumas approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@BruegelN @jdumas