Skip to content

Core: Add LV_NODISCARD for C API functions with non-discardable results #265

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kaixiong merged 6 commits into from
Feb 13, 2023
Merged

Core: Add LV_NODISCARD for C API functions with non-discardable results #265

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
140583d
Core: Add LV_NODISCARD to mark C API functions whose return results a…
kaixiong Feb 8, 2023
8096ffe
Core: Define LV_NODISCARD as [[nodiscard]] when building with a C++ c…
kaixiong Feb 8, 2023
4428cb2
Core: Add MSVC and GCC version checks for LV_NODISCARD.
kaixiong Feb 8, 2023
32ebde4
Core: Fix #include of lv_defines.h.
kaixiong Feb 8, 2023
6eb6cf3
Core: Mark 'nodiscard' functions that were missed.
kaixiong Feb 9, 2023
c0f6a3f
Core: Check for C++17 before defining LV_NODISCARD as [[nodiscard]].
kaixiong Feb 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions libvisual/libvisual/lv_actor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,11 +248,11 @@ LV_API const char *visual_actor_get_next_by_name_nogl (const char *name);
LV_API const char *visual_actor_get_prev_by_name (const char *name);
LV_API const char *visual_actor_get_next_by_name (const char *name);

LV_API VisActor *visual_actor_new (const char *name);
LV_API int visual_actor_realize (VisActor *actor);
LV_API void visual_actor_run (VisActor *actor, VisAudio *audio);
LV_API void visual_actor_ref (VisActor *actor);
LV_API void visual_actor_unref (VisActor *actor);
LV_NODISCARD LV_API VisActor *visual_actor_new (const char *name);
LV_API int visual_actor_realize (VisActor *actor);
LV_API void visual_actor_run (VisActor *actor, VisAudio *audio);
LV_API void visual_actor_ref (VisActor *actor);
LV_API void visual_actor_unref (VisActor *actor);

LV_API VisSongInfo *visual_actor_get_songinfo (VisActor *actor);
LV_API VisPalette *visual_actor_get_palette (VisActor *actor);
Expand Down
3 changes: 2 additions & 1 deletion libvisual/libvisual/lv_audio.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,8 @@ struct _VisAudio;

LV_BEGIN_DECLS

LV_API VisAudio *visual_audio_new (void);
LV_NODISCARD LV_API VisAudio *visual_audio_new (void);

LV_API void visual_audio_free (VisAudio *audio);

LV_API int visual_audio_get_sample (VisAudio *audio, VisBuffer *buffer, const char *channelid);
Expand Down
5 changes: 3 additions & 2 deletions libvisual/libvisual/lv_bin.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,9 @@ struct _VisBin;

LV_BEGIN_DECLS

LV_API VisBin *visual_bin_new (void);
LV_API void visual_bin_free (VisBin *bin);
LV_NODISCARD LV_API VisBin *visual_bin_new (void);

LV_API void visual_bin_free (VisBin *bin);

LV_API void visual_bin_realize (VisBin *bin);

Expand Down
6 changes: 3 additions & 3 deletions libvisual/libvisual/lv_buffer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,9 +250,9 @@ struct _VisBuffer;

LV_BEGIN_DECLS

LV_API VisBuffer *visual_buffer_new_wrap_data (void *data, visual_size_t size, int own);
LV_API VisBuffer *visual_buffer_new_allocate (visual_size_t size);
LV_API VisBuffer *visual_buffer_clone (VisBuffer *source);
LV_NODISCARD LV_API VisBuffer *visual_buffer_new_wrap_data (void *data, visual_size_t size, int own);
LV_NODISCARD LV_API VisBuffer *visual_buffer_new_allocate (visual_size_t size);
LV_NODISCARD LV_API VisBuffer *visual_buffer_clone (VisBuffer *source);

LV_API void *visual_buffer_get_data (VisBuffer *buffer);
LV_API void *visual_buffer_get_data_offset (VisBuffer *buffer, visual_size_t offset);
Expand Down
7 changes: 4 additions & 3 deletions libvisual/libvisual/lv_color.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,10 @@ struct _VisColor
};
#endif

LV_API VisColor *visual_color_new (void);
LV_API VisColor *visual_color_clone (VisColor *color);
LV_API void visual_color_free (VisColor *color);
LV_NODISCARD LV_API VisColor *visual_color_new (void);
LV_NODISCARD LV_API VisColor *visual_color_clone (VisColor *color);

LV_API void visual_color_free (VisColor *color);

LV_API int visual_color_compare (VisColor *src1, VisColor *src2);

Expand Down
11 changes: 11 additions & 0 deletions libvisual/libvisual/lv_defines.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,19 @@
#define LV_PLUGIN_EXPORT LV_DLL_EXPORT

/* Utility macros */

#ifdef _MSC_VER
#define __PRETTY_FUNCTION__ __FUNCTION__
#endif

#if __cplusplus >= 201703L
Copy link
Member

@hartwork hartwork Feb 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kaixiong could it be that this needs addition of defined(__cplusplus) && ?

Copy link
Member Author

@kaixiong kaixiong Feb 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hartwork, I was wondering about this. GCC and Clang compile it but I'm not sure if this is standard. Any idea?

Copy link
Member

@hartwork hartwork Feb 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kaixiong let me first spell out the questions that I think we need to answer and then try to give a potential answer. For the questions:

  1. Is this code used with a C compiler or only C++? Does need it to work from plain C?
  2. Can we safely compare an undefined macro against an integer value?
  3. Does the __cplusplus value reflect reality (more below)?

Potential answers:

  1. Seems like yes, we need to support both C and C++ here.
  2. It is defined according to https://stackoverflow.com/questions/5085392/what-is-the-value-of-an-undefined-constant-used-in-if so it seems to become a question of style and/or clarity.
  3. MSVC is said to not have reality in __cplusplus. The post lays out some options. Another one. I think we do want to support MSVC?

PS: https://sourceforge.net/p/predef/wiki/Standards/ does not seem to have any additional insights.

What do you think?

Copy link
Member Author

@kaixiong kaixiong Feb 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hartwork Looks like Question 1 and 2 are effectively the same question as the C/C++ preprocessor both default to 0 for undefined constants (apart from the handling of true and false).

Question 3 is addressed in #269 but does not ultimately matter for this PR because the code falls back to compiler extensions (_Check_return_) immediately below this.

#define LV_NODISCARD [[nodiscard]]
#elif defined(_MSC_VER) && _MSC_VER >= 1700
#define LV_NODISCARD _Check_return_
#elif defined(__GNUC__) && __GNUC__ >= 4
#define LV_NODISCARD __attribute__ ((warn_unused_result))
#else
#define LV_NODISCARD
#endif

#endif /* _LV_DEFINES_H */
23 changes: 12 additions & 11 deletions libvisual/libvisual/lv_event.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ LV_BEGIN_DECLS
* @param keymod Key modifier used
* @param state State of key i.e. pressed or released
*/
LV_API VisEvent *visual_event_new_keyboard (VisKey keysym, VisKeyMod keymod, VisKeyState state);
LV_NODISCARD LV_API VisEvent *visual_event_new_keyboard (VisKey keysym, VisKeyMod keymod, VisKeyState state);

/**
* Creates a new mouse movement event.
Expand All @@ -247,7 +247,7 @@ LV_API VisEvent *visual_event_new_keyboard (VisKey keysym, VisKeyMod keymod, Vis
*
* @return New event object
*/
LV_API VisEvent *visual_event_new_mousemotion (int dx, int dy);
LV_NODISCARD LV_API VisEvent *visual_event_new_mousemotion (int dx, int dy);

/**
* Creates a new mouse button event
Expand All @@ -259,7 +259,7 @@ LV_API VisEvent *visual_event_new_mousemotion (int dx, int dy);
*
* @return New event object
*/
LV_API VisEvent *visual_event_new_mousebutton (int button, VisMouseState state, int x, int y);
LV_NODISCARD LV_API VisEvent *visual_event_new_mousebutton (int button, VisMouseState state, int x, int y);

/**
* Creates a resize event.
Expand All @@ -269,7 +269,7 @@ LV_API VisEvent *visual_event_new_mousebutton (int button, VisMouseState state,
*
* @return New event object
*/
LV_API VisEvent *visual_event_new_resize (int width, int height);
LV_NODISCARD LV_API VisEvent *visual_event_new_resize (int width, int height);

/**
* Creates a new song change event.
Expand All @@ -278,7 +278,7 @@ LV_API VisEvent *visual_event_new_resize (int width, int height);
*
* @return New event object
*/
LV_API VisEvent *visual_event_new_newsong (VisSongInfo *songinfo);
LV_NODISCARD LV_API VisEvent *visual_event_new_newsong (VisSongInfo *songinfo);

/**
* Creates a new parameter change event.
Expand All @@ -287,14 +287,14 @@ LV_API VisEvent *visual_event_new_newsong (VisSongInfo *songinfo);
*
* @return New event object
*/
LV_API VisEvent *visual_event_new_param (void *param);
LV_NODISCARD LV_API VisEvent *visual_event_new_param (void *param);

/**
* Creates a quit event
*
* @return New event object
*/
LV_API VisEvent *visual_event_new_quit (void);
LV_NODISCARD LV_API VisEvent *visual_event_new_quit (void);

/**
* Creates a new visibility event.
Expand All @@ -303,7 +303,7 @@ LV_API VisEvent *visual_event_new_quit (void);
*
* @return New event object
*/
LV_API VisEvent *visual_event_new_visibility (int is_visible);
LV_NODISCARD LV_API VisEvent *visual_event_new_visibility (int is_visible);

/**
* Copies a VisEvent.
Expand All @@ -329,10 +329,11 @@ LV_API void visual_event_free (VisEvent* event);
*
* @return New event object
*/
LV_API VisEvent *visual_event_new_custom (int eid, int param_int, void *param_ptr);
LV_NODISCARD LV_API VisEvent *visual_event_new_custom (int eid, int param_int, void *param_ptr);

LV_API VisEventQueue *visual_event_queue_new (void);
LV_API void visual_event_queue_free (VisEventQueue *eventqueue);
LV_NODISCARD LV_API VisEventQueue *visual_event_queue_new (void);

LV_API void visual_event_queue_free (VisEventQueue *eventqueue);

LV_API void visual_event_queue_add (VisEventQueue *eventqueue, VisEvent *event);
LV_API int visual_event_queue_poll (VisEventQueue *eventqueue, VisEvent *event);
Expand Down
5 changes: 3 additions & 2 deletions libvisual/libvisual/lv_fourier.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,9 @@ struct _VisDFT;

LV_BEGIN_DECLS

LV_API VisDFT *visual_dft_new (unsigned int samples_out, unsigned int samples_in);
LV_API void visual_dft_free (VisDFT *dft);
LV_NODISCARD LV_API VisDFT *visual_dft_new (unsigned int samples_out, unsigned int samples_in);

LV_API void visual_dft_free (VisDFT *dft);

LV_API void visual_dft_perform (VisDFT *dft, float *output, float const *input);

Expand Down
11 changes: 6 additions & 5 deletions libvisual/libvisual/lv_input.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,12 @@ struct _VisInputPlugin {

LV_BEGIN_DECLS

LV_API VisInput *visual_input_new (const char *name);
LV_API void visual_input_ref (VisInput *input);
LV_API void visual_input_unref (VisInput *input);
LV_API int visual_input_realize (VisInput *input);
LV_API int visual_input_run (VisInput *input);
LV_NODISCARD LV_API VisInput *visual_input_new (const char *name);

LV_API void visual_input_ref (VisInput *input);
LV_API void visual_input_unref (VisInput *input);
LV_API int visual_input_realize (VisInput *input);
LV_API int visual_input_run (VisInput *input);

LV_API VisPluginData *visual_input_get_plugin (VisInput *input);
LV_API VisAudio *visual_input_get_audio (VisInput *audio);
Expand Down
8 changes: 4 additions & 4 deletions libvisual/libvisual/lv_mem.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ LV_BEGIN_DECLS
*
* @return Pointer to newly allocated memory block, or NULL on failure
*/
LV_API void *visual_mem_malloc (visual_size_t size) LV_ATTR_MALLOC;
LV_NODISCARD LV_API void *visual_mem_malloc (visual_size_t size) LV_ATTR_MALLOC;

/**
* Allocates a block of memory with its content zeroed.
Expand All @@ -108,7 +108,7 @@ LV_API void *visual_mem_malloc (visual_size_t size) LV_ATTR_MALLOC;
* @return Pointer to newly allocated memory block with its contents
* zeroed, or NULL on failure
*/
LV_API void *visual_mem_malloc0 (visual_size_t size) LV_ATTR_MALLOC;
LV_NODISCARD LV_API void *visual_mem_malloc0 (visual_size_t size) LV_ATTR_MALLOC;

/**
* Reallocates memory, can be used to grow a buffer.
Expand All @@ -118,7 +118,7 @@ LV_API void *visual_mem_malloc0 (visual_size_t size) LV_ATTR_MALLOC;
*
* @return pointer to the reallocated memory block, or NULL on failure
*/
LV_API void *visual_mem_realloc (void *ptr, visual_size_t size) LV_ATTR_MALLOC;
LV_NODISCARD LV_API void *visual_mem_realloc (void *ptr, visual_size_t size) LV_ATTR_MALLOC;

/**
* Frees a memory block allocated by visual_mem_malloc() and visual_mem_realloc().
Expand All @@ -139,7 +139,7 @@ LV_API void visual_mem_free (void *ptr);
*
* @note Memory allocated by this function must be fred by visual_mem_free_aligned().
*/
LV_API void *visual_mem_malloc_aligned (visual_size_t size, visual_size_t alignment);
LV_NODISCARD LV_API void *visual_mem_malloc_aligned (visual_size_t size, visual_size_t alignment);

/**
* Frees a memory block allocated by visual_mem_alloc_aligned().
Expand Down
7 changes: 4 additions & 3 deletions libvisual/libvisual/lv_morph.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,9 +262,10 @@ LV_API const char *visual_morph_get_next_by_name (const char *name);
*/
LV_API const char *visual_morph_get_prev_by_name (const char *name);

LV_API VisMorph *visual_morph_new (const char *name);
LV_API void visual_morph_ref (VisMorph *morph);
LV_API void visual_morph_unref (VisMorph *morph);
LV_NODISCARD LV_API VisMorph *visual_morph_new (const char *name);

LV_API void visual_morph_ref (VisMorph *morph);
LV_API void visual_morph_unref (VisMorph *morph);

LV_API VisPluginData *visual_morph_get_plugin (VisMorph *morph);
LV_API VisVideoDepth visual_morph_get_supported_depths (VisMorph *morph);
Expand Down
6 changes: 3 additions & 3 deletions libvisual/libvisual/lv_palette.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,11 @@ struct _VisPalette;

LV_BEGIN_DECLS

LV_API VisPalette *visual_palette_new (unsigned int ncolors);
LV_NODISCARD LV_API VisPalette *visual_palette_new (unsigned int ncolors);

LV_API void visual_palette_free (VisPalette *palette);
LV_NODISCARD LV_API VisPalette *visual_palette_clone (VisPalette *self);

LV_API VisPalette *visual_palette_clone (VisPalette *self);
LV_API void visual_palette_free (VisPalette *palette);

LV_API void visual_palette_copy (VisPalette *dest, VisPalette *src);

Expand Down
20 changes: 11 additions & 9 deletions libvisual/libvisual/lv_param.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,13 +159,15 @@ LV_BEGIN_DECLS

/* VisClosure API */

LV_API VisClosure *visual_closure_new (void *func, void *data, VisDestroyFunc destroy_func);
LV_API void visual_closure_free (VisClosure *self);
LV_NODISCARD LV_API VisClosure *visual_closure_new (void *func, void *data, VisDestroyFunc destroy_func);

LV_API void visual_closure_free (VisClosure *self);

/* VisParamList API */

LV_API VisParamList *visual_param_list_new (void);
LV_API void visual_param_list_free (VisParamList *self);
LV_NODISCARD LV_API VisParamList *visual_param_list_new (void);

LV_API void visual_param_list_free (VisParamList *self);

LV_API void visual_param_list_add (VisParamList *list, VisParam *param);
LV_API void visual_param_list_add_array (VisParamList *list, VisParam **params, unsigned int nparams);
Expand All @@ -190,11 +192,11 @@ LV_API VisEventQueue *visual_param_list_get_event_queue (VisParamList *list);
*
* @return A newly allocated VisParam
*/
LV_API VisParam *visual_param_new (const char * name,
const char * description,
VisParamType type,
void * default_value,
VisClosure * validator);
LV_NODISCARD LV_API VisParam *visual_param_new (const char * name,
const char * description,
VisParamType type,
void * default_value,
VisClosure * validator);

/**
* Frees a parameter entry.
Expand Down
2 changes: 1 addition & 1 deletion libvisual/libvisual/lv_param_validators.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ typedef struct _VisClosure VisClosure;

LV_BEGIN_DECLS

LV_API VisClosure *visual_param_in_range (VisParamType type, void *lower, void *upper);
LV_NODISCARD LV_API VisClosure *visual_param_in_range (VisParamType type, void *lower, void *upper);

#define _LV_DEFINE_PARAM_IN_RANGE(func,ctype,type,marshal) \
static inline VisClosure *visual_param_in_range_##func (ctype lower, ctype upper) { \
Expand Down
15 changes: 8 additions & 7 deletions libvisual/libvisual/lv_param_value.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,14 @@ struct _VisParamValue

LV_BEGIN_DECLS

LV_API VisParamValue *visual_param_value_new (VisParamType type, void *value);
LV_API void visual_param_value_init (VisParamValue *self, VisParamType type, void *value);
LV_API void visual_param_value_copy (VisParamValue *value, VisParamValue *src);
LV_API void visual_param_value_set (VisParamValue *value, VisParamType type, void *new_value);
LV_API int visual_param_value_compare (VisParamValue *lhs, VisParamValue *rhs);
LV_API void visual_param_value_free (VisParamValue *value);
LV_API void visual_param_value_free_value (VisParamValue *value);
LV_NODISCARD LV_API VisParamValue *visual_param_value_new (VisParamType type, void *value);

LV_API void visual_param_value_init (VisParamValue *self, VisParamType type, void *value);
LV_API void visual_param_value_copy (VisParamValue *value, VisParamValue *src);
LV_API void visual_param_value_set (VisParamValue *value, VisParamType type, void *new_value);
LV_API int visual_param_value_compare (VisParamValue *lhs, VisParamValue *rhs);
LV_API void visual_param_value_free (VisParamValue *value);
LV_API void visual_param_value_free_value (VisParamValue *value);

#define _LV_PARAM_MARSHAL_INTEGER(x) ((void *) (intptr_t) (x))
#define _LV_PARAM_MARSHAL_FLOAT(x) ((void *) (&x))
Expand Down
5 changes: 3 additions & 2 deletions libvisual/libvisual/lv_random.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,9 @@ typedef ::LV::RandomSeed VisRandomSeed;

LV_BEGIN_DECLS

LV_API VisRandomContext *visual_random_context_new (VisRandomSeed seed);
LV_API void visual_random_context_free (VisRandomContext *rcontext);
LV_NODISCARD LV_API VisRandomContext *visual_random_context_new (VisRandomSeed seed);

LV_API void visual_random_context_free (VisRandomContext *rcontext);

LV_API void visual_random_context_set_seed (VisRandomContext *rcontext, VisRandomSeed seed);
LV_API uint32_t visual_random_context_int (VisRandomContext *rcontext);
Expand Down
6 changes: 3 additions & 3 deletions libvisual/libvisual/lv_rectangle.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,15 +251,15 @@ struct _VisRectangle;

LV_BEGIN_DECLS

LV_API VisRectangle *visual_rectangle_new (int x, int y, int width, int height);
LV_API VisRectangle *visual_rectangle_new_empty (void);
LV_NODISCARD LV_API VisRectangle *visual_rectangle_new (int x, int y, int width, int height);
LV_NODISCARD LV_API VisRectangle *visual_rectangle_new_empty (void);

LV_API void visual_rectangle_free (VisRectangle *rect);

LV_API void visual_rectangle_set (VisRectangle *rect, int x, int y, int width, int height);
LV_API void visual_rectangle_copy (VisRectangle *dest, VisRectangle *src);

LV_API VisRectangle *visual_rectangle_clone (VisRectangle *rect);
LV_NODISCARD LV_API VisRectangle *visual_rectangle_clone (VisRectangle *rect);

LV_API void visual_rectangle_set_x (VisRectangle *rect, int x);
LV_API int visual_rectangle_get_x (VisRectangle *rect);
Expand Down
7 changes: 4 additions & 3 deletions libvisual/libvisual/lv_songinfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,9 +214,10 @@ struct _VisSongInfo;

LV_BEGIN_DECLS

LV_API VisSongInfo *visual_songinfo_new (VisSongInfoType type);
LV_API VisSongInfo *visual_songinfo_clone (VisSongInfo const *src);
LV_API void visual_songinfo_free (VisSongInfo *songinfo);
LV_NODISCARD LV_API VisSongInfo *visual_songinfo_new (VisSongInfoType type);
LV_NODISCARD LV_API VisSongInfo *visual_songinfo_clone (VisSongInfo const *src);

LV_API void visual_songinfo_free (VisSongInfo *songinfo);

LV_API void visual_songinfo_copy (VisSongInfo *dest, VisSongInfo const *src);
LV_API int visual_songinfo_compare (VisSongInfo const *s1, VisSongInfo const *s2);
Expand Down
Loading