Skip to content
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.

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

Code safety/modernization #234

Merged
merged 6 commits into from
Oct 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
66 changes: 42 additions & 24 deletions API/fleece/CompilerSupport.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,22 @@
#ifndef _FLEECE_COMPILER_SUPPORT_H
#define _FLEECE_COMPILER_SUPPORT_H

#ifdef __APPLE__
#include <sys/cdefs.h> // include this first to avoid conflict with our definition of __printflike
#endif


// The __has_xxx() macros are supported by [at least] Clang and GCC.
// Define them to return 0 on other compilers.
// https://clang.llvm.org/docs/AttributeReference.html
// https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html

#ifndef __has_attribute
#define __has_attribute(x) 0
#define __has_attribute(x) 0
#endif

#ifndef __has_cpp_attribute
#define __has_cpp_attribute(x) 0
#endif

#ifndef __has_builtin
Expand All @@ -43,24 +52,22 @@
# define RETURNS_NONNULL
#endif

// deprecated; use NODISCARD instead
#if __has_attribute(returns_nonnull)
# define MUST_USE_RESULT __attribute__((warn_unused_result))
#else
# define MUST_USE_RESULT
#endif

// NODISCARD expands to the C++17/C23 `[[nodiscard]]` attribute, or else MUST_USE_RESULT.
// (We can't just redefine MUST_USE_RESULT as `[[nodiscard]]` unfortunately, because the former is
// already in use in positions where `[[nodiscard]]` isn't valid, like at the end of a declaration.)
// NODISCARD expands to the C++17/C23 `[[nodiscard]]` attribute.
// Use it before a function declaration when it's a mistake to ignore the function's result.
#if (__cplusplus >= 201700L) || (__STDC_VERSION__ >= 202000)
# define NODISCARD [[nodiscard]]
#elif __has_attribute(warn_unused_result)
# define NODISCARD __attribute__((warn_unused_result))
#else
# define NODISCARD MUST_USE_RESULT
# define NODISCARD
#endif


// These have no effect on behavior, but they hint to the optimizer which branch of an 'if'
// statement to make faster.
// Note: In C++20 the standard attributes `[[likely]]` and `[[unlikely]]` can be used instead,
// but they're not syntactically identical.
#if __has_builtin(__builtin_expect)
#define _usuallyTrue(VAL) __builtin_expect(VAL, true)
#define _usuallyFalse(VAL) __builtin_expect(VAL, false)
Expand All @@ -75,7 +82,7 @@
// disallow NULL values, unless annotated with FL_NULLABLE (which must come after the `*`.)
// (FL_NONNULL is occasionally necessary when there are multiple levels of pointers.)
// NOTE: Only supported in Clang, so far.
#if defined(__clang__)
#if __has_feature(nullability)
# define FL_ASSUME_NONNULL_BEGIN _Pragma("clang assume_nonnull begin")
# define FL_ASSUME_NONNULL_END _Pragma("clang assume_nonnull end")
# define FL_NULLABLE _Nullable
Expand Down Expand Up @@ -146,18 +153,6 @@
#endif


// `constexpr14` is for uses of `constexpr` that are valid in C++14 but not earlier.
// In constexpr functions this includes `if`, `for`, `while` statements; or multiple `return`s.
// The macro expands to `constexpr` in C++14 or later, otherwise to nothing.
#ifdef __cplusplus
#if __cplusplus >= 201400L || _MSVC_LANG >= 201400L
#define constexpr14 constexpr
#else
#define constexpr14
#endif
#endif // __cplusplus


// STEPOVER is for trivial little glue functions that are annoying to step into in the debugger
// on the way to the function you _do_ want to step into. Examples are RefCounted's operator->,
// or slice constructors. Suppressing debug info for those functions means the debugger
Expand Down Expand Up @@ -279,6 +274,29 @@
#define FLAPI
#endif

// `LIFETIMEBOUND` helps Clang detect value lifetime errors, where one value's lifetime is tied to another and the
// dependent value is used after the value it depends on exits scope. For examples of usage, see slice.hh.
// "The `lifetimebound` attribute on a function parameter or implicit object parameter indicates that objects that are
// referred to by that parameter may also be referred to by the return value of the annotated function (or, for a
// parameter of a constructor, by the value of the constructed object)."
// -- https://clang.llvm.org/docs/AttributeReference.html#lifetimebound
#if __has_cpp_attribute(clang::lifetimebound)
# define LIFETIMEBOUND [[clang::lifetimebound]]
#else
# define LIFETIMEBOUND
#endif


// Type-checking for printf-style vararg functions:
#ifndef __printflike
# if __has_attribute(__format__)
# define __printflike(fmtarg, firstvararg) __attribute__((__format__(__printf__, fmtarg, firstvararg)))
# else
# define __printflike(A, B)
# endif
#endif


#else // _FLEECE_COMPILER_SUPPORT_H
#warn "Compiler is not honoring #pragma once"
#endif
4 changes: 2 additions & 2 deletions API/fleece/FLDoc.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,13 @@ extern "C" {
FLEECE_PUBLIC FLDoc FLDoc_Retain(FLDoc FL_NULLABLE) FLAPI;

/** Returns the encoded Fleece data backing the document. */
FLEECE_PUBLIC FLSlice FLDoc_GetData(FLDoc FL_NULLABLE) FLAPI FLPURE;
FLEECE_PUBLIC FLSlice FLDoc_GetData(FLDoc FL_NULLABLE doc LIFETIMEBOUND) FLAPI FLPURE;

/** Returns the FLSliceResult data owned by the document, if any, else a null slice. */
FLEECE_PUBLIC FLSliceResult FLDoc_GetAllocedData(FLDoc FL_NULLABLE) FLAPI FLPURE;

/** Returns the root value in the FLDoc, usually an FLDict. */
FLEECE_PUBLIC FLValue FLDoc_GetRoot(FLDoc FL_NULLABLE) FLAPI FLPURE;
FLEECE_PUBLIC FLValue FLDoc_GetRoot(FLDoc FL_NULLABLE doc LIFETIMEBOUND) FLAPI FLPURE;

/** Returns the FLSharedKeys used by this FLDoc, as specified when it was created. */
FLEECE_PUBLIC FLSharedKeys FLDoc_GetSharedKeys(FLDoc FL_NULLABLE) FLAPI FLPURE;
Expand Down
3 changes: 1 addition & 2 deletions API/fleece/FLEncoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,6 @@ extern "C" {

/** Ends encoding; if there has been no error, it returns the encoded data, else null.
This does not free the FLEncoder; call FLEncoder_Free (or FLEncoder_Reset) next. */
NODISCARD
FLEECE_PUBLIC FLSliceResult FLEncoder_Finish(FLEncoder, FLError* FL_NULLABLE outError) FLAPI;

/** @} */
Expand All @@ -226,7 +225,7 @@ extern "C" {
FLEECE_PUBLIC FLError FLEncoder_GetError(FLEncoder) FLAPI;

/** Returns the error message of an encoder, or NULL if there's no error. */
FLEECE_PUBLIC const char* FL_NULLABLE FLEncoder_GetErrorMessage(FLEncoder) FLAPI;
FLEECE_PUBLIC const char* FL_NULLABLE FLEncoder_GetErrorMessage(FLEncoder e LIFETIMEBOUND) FLAPI;

/** @} */
/** @} */
Expand Down
18 changes: 12 additions & 6 deletions API/fleece/FLSlice.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,11 @@ typedef struct FLSlice {
size_t size;

#ifdef __cplusplus
explicit operator bool() const noexcept FLPURE {return buf != nullptr;}
explicit operator std::string() const {return std::string((char*)buf, size);}
constexpr const void* FL_NULLABLE data() const noexcept FLPURE {return buf;}
constexpr size_t size_bytes() const noexcept FLPURE {return size;} // compatibility with std::span
constexpr bool empty() const noexcept FLPURE {return size == 0;}
constexpr explicit operator bool() const noexcept FLPURE {return buf != nullptr;}
explicit operator std::string() const {return std::string((char*)buf, size);}
#endif
} FLSlice;

Expand All @@ -65,8 +68,11 @@ struct NODISCARD FLSliceResult {
size_t size;

#ifdef __cplusplus
explicit operator bool() const noexcept FLPURE {return buf != nullptr;}
explicit operator FLSlice () const {return {buf, size};}
constexpr const void* FL_NULLABLE data() const noexcept FLPURE {return buf;}
constexpr size_t size_bytes() const noexcept FLPURE {return size;} // compatibility with std::span
constexpr bool empty() const noexcept FLPURE {return size == 0;}
constexpr explicit operator bool() const noexcept FLPURE {return buf != nullptr;}
constexpr explicit operator FLSlice () const {return {buf, size};}
inline explicit operator std::string() const;
#endif
};
Expand Down Expand Up @@ -123,7 +129,7 @@ inline void FLMemCpy(void* FL_NULLABLE dst, const void* FL_NULLABLE src, size_t
It's OK to pass NULL; this returns an empty slice.
\note If the string is a literal, it's more efficient to use \ref FLSTR instead.
\note Performance is O(n) with the length of the string, since it has to call `strlen`. */
inline FLSlice FLStr(const char* FL_NULLABLE str) FLAPI {
inline FLSlice FLStr(const char* FL_NULLABLE str LIFETIMEBOUND) FLAPI {
FLSlice foo = { str, str ? strlen(str) : 0 };
return foo;
}
Expand Down Expand Up @@ -185,7 +191,7 @@ inline void FLSliceResult_Release(FLSliceResult s) FLAPI {
}

/** Type-casts a FLSliceResult to FLSlice, since C doesn't know it's a subclass. */
inline FLSlice FLSliceResult_AsSlice(FLSliceResult sr) {
inline FLSlice FLSliceResult_AsSlice(FLSliceResult sr LIFETIMEBOUND) {
FLSlice ret;
memcpy(&ret, &sr, sizeof(ret));
return ret;
Expand Down
41 changes: 21 additions & 20 deletions API/fleece/Fleece.hh
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ namespace fleece {

bool isEqual(Value v) const {return FLValue_IsEqual(_val, v);}

Value& operator= (Value v) & {_val = v._val; return *this;}
Value& operator= (std::nullptr_t) & {_val = nullptr; return *this;}

inline Value operator[] (const KeyPath &kp) const;
Expand Down Expand Up @@ -129,9 +128,8 @@ namespace fleece {
inline Value operator[] (int index) const {return get(index);}
inline Value operator[] (const KeyPath &kp) const {return Value::operator[](kp);}

Array& operator= (Array a) & {_val = a._val; return *this;}
Array& operator= (std::nullptr_t) & {_val = nullptr; return *this;}
Value& operator= (Value v) =delete;
Array& operator= (std::nullptr_t) & {_val = nullptr; return *this;}
Value& operator= (Value v) =delete;

[[nodiscard]] inline MutableArray asMutable() const;

Expand All @@ -149,7 +147,7 @@ namespace fleece {
inline Value operator * () const {return value();}
inline explicit operator bool() const {return (bool)value();}
inline iterator& operator++ () {next(); return *this;}
inline bool operator!= (const iterator&) {return value() != nullptr;}
inline bool operator!= (const iterator&) const {return value() != nullptr;}
inline Value operator[] (unsigned n) const {return FLArrayIterator_GetValueAt(this,n);}
private:
iterator() =default;
Expand Down Expand Up @@ -187,7 +185,6 @@ namespace fleece {
inline Value operator[] (const char *key) const {return get(key);}
inline Value operator[] (const KeyPath &kp) const {return Value::operator[](kp);}

Dict& operator= (Dict d) & {_val = d._val; return *this;}
Dict& operator= (std::nullptr_t) & {_val = nullptr; return *this;}
Value& operator= (Value v) =delete;

Expand Down Expand Up @@ -261,8 +258,8 @@ namespace fleece {
KeyPath(slice_NONNULL spec, FLError* FL_NULLABLE err) :_path(FLKeyPath_New(spec, err)) { }
~KeyPath() {FLKeyPath_Free(_path);}

KeyPath(KeyPath &&kp) :_path(kp._path) {kp._path = nullptr;}
KeyPath& operator=(KeyPath &&kp) & {FLKeyPath_Free(_path); _path = kp._path;
KeyPath(KeyPath &&kp) noexcept :_path(kp._path) {kp._path = nullptr;}
KeyPath& operator=(KeyPath &&kp) & noexcept {FLKeyPath_Free(_path); _path = kp._path;
kp._path = nullptr; return *this;}

KeyPath(const KeyPath &kp) :KeyPath(std::string(kp), nullptr) { }
Expand Down Expand Up @@ -334,7 +331,7 @@ namespace fleece {
external pointers to. */
class Doc {
public:
Doc(alloc_slice fleeceData,
Doc(const alloc_slice& fleeceData,
FLTrust trust =kFLUntrusted,
FLSharedKeys FL_NULLABLE sk =nullptr,
slice externDest =nullslice) noexcept
Expand All @@ -356,23 +353,23 @@ namespace fleece {
Doc& operator=(Doc &&other) noexcept;
~Doc() {FLDoc_Release(_doc);}

slice data() const {return FLDoc_GetData(_doc);}
slice data() const LIFETIMEBOUND {return FLDoc_GetData(_doc);}
alloc_slice allocedData() const {return FLDoc_GetAllocedData(_doc);}
FLSharedKeys sharedKeys() const {return FLDoc_GetSharedKeys(_doc);}

Value root() const {return FLDoc_GetRoot(_doc);}
Value root() const LIFETIMEBOUND {return FLDoc_GetRoot(_doc);}
explicit operator bool () const {return root() != nullptr;}
Array asArray() const {return root().asArray();}
Dict asDict() const {return root().asDict();}

operator Value () const {return root();}
operator Dict () const {return asDict();}
operator FLDict FL_NULLABLE () const {return asDict();}
operator Value () const LIFETIMEBOUND {return root();}
operator Dict () const LIFETIMEBOUND {return asDict();}
operator FLDict FL_NULLABLE () const LIFETIMEBOUND {return asDict();}

Value operator[] (int index) const {return asArray().get(index);}
Value operator[] (slice key) const {return asDict().get(key);}
Value operator[] (const char *key) const {return asDict().get(key);}
Value operator[] (const KeyPath &kp) const {return root().operator[](kp);}
Value operator[] (int index) const LIFETIMEBOUND {return asArray().get(index);}
Value operator[] (slice key) const LIFETIMEBOUND {return asDict().get(key);}
Value operator[] (const char *key) const LIFETIMEBOUND {return asDict().get(key);}
Value operator[] (const KeyPath &kp) const LIFETIMEBOUND {return root().operator[](kp);}

bool operator== (const Doc &d) const {return _doc == d._doc;}

Expand Down Expand Up @@ -415,7 +412,7 @@ namespace fleece {
explicit Encoder(FLSharedKeys FL_NULLABLE sk) :Encoder() {setSharedKeys(sk);}

explicit Encoder(FLEncoder enc) :_enc(enc) { }
Encoder(Encoder&& enc) :_enc(enc._enc) {enc._enc = nullptr;}
Encoder(Encoder&& enc) noexcept :_enc(enc._enc) {enc._enc = nullptr;}

void detach() {_enc = nullptr;}

Expand All @@ -434,7 +431,7 @@ namespace fleece {
inline bool writeDouble(double);
inline bool writeString(slice);
inline bool writeString(const char *s) {return writeString(slice(s));}
inline bool writeString(std::string s) {return writeString(slice(s));}
inline bool writeString(const std::string& s) {return writeString(slice(s));}
inline bool writeDateString(FLTimestamp, bool asUTC =true);
inline bool writeData(slice);
inline bool writeValue(Value);
Expand Down Expand Up @@ -537,6 +534,10 @@ namespace fleece {

//====== IMPLEMENTATION GUNK:

static_assert(std::is_trivially_copyable_v<Value>);
static_assert(std::is_trivially_copyable_v<Array>);
static_assert(std::is_trivially_copyable_v<Dict>);

inline FLValueType Value::type() const {return FLValue_GetType(_val);}
inline bool Value::isInteger() const {return FLValue_IsInteger(_val);}
inline bool Value::isUnsigned() const {return FLValue_IsUnsigned(_val);}
Expand Down
5 changes: 1 addition & 4 deletions API/fleece/PlatformCompat.hh
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,12 @@

#pragma once
#include "fleece/CompilerSupport.h"
#ifdef __APPLE__
#include <sys/cdefs.h>
#include "TargetConditionals.h"
#endif

#ifdef _MSC_VER
#define NOINLINE __declspec(noinline)
#define ALWAYS_INLINE inline
#define ASSUME(cond) __assume(cond)
#define __typeof decltype

#define __func__ __FUNCTION__

Expand Down
18 changes: 11 additions & 7 deletions API/fleece/RefCounted.hh
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@

#pragma once
#include "fleece/PlatformCompat.hh"
#include <algorithm>
#include <atomic>
#include <utility>

Expand Down Expand Up @@ -108,9 +107,9 @@ namespace fleece {

~Retained() {release(_ref);}

operator T* () const & noexcept FLPURE STEPOVER {return _ref;}
T* operator-> () const noexcept FLPURE STEPOVER {return _ref;}
T* get() const noexcept FLPURE STEPOVER {return _ref;}
operator T* () const & noexcept LIFETIMEBOUND FLPURE STEPOVER {return _ref;}
T* operator-> () const noexcept LIFETIMEBOUND FLPURE STEPOVER {return _ref;}
T* get() const noexcept LIFETIMEBOUND FLPURE STEPOVER {return _ref;}

explicit operator bool () const FLPURE {return (_ref != nullptr);}

Expand Down Expand Up @@ -150,6 +149,11 @@ namespace fleece {
[[nodiscard]]
T* detach() && noexcept {auto r = _ref; _ref = nullptr; return r;}

/// Equivalent to `get` but without the `LIFETIMEBOUND` attribute. For use in rare cases where you have
/// a `Retained<T>` and need to return it as a `T*`, which is normally illegal, but you know that there's
/// another `Retained` value keeping the object alive even after this function returns.
T* unsafe_get() const noexcept {return _ref;}

// The operator below is often a dangerous mistake, so it's deliberately made impossible.
// It happens in these sorts of contexts, where it can produce a dangling pointer to a
// deleted object:
Expand Down Expand Up @@ -198,9 +202,9 @@ namespace fleece {
RetainedConst(Retained<T> &&r) noexcept :_ref(std::move(r).detach()) { }
ALWAYS_INLINE ~RetainedConst() {release(_ref);}

operator const T* () const & noexcept FLPURE STEPOVER {return _ref;}
const T* operator-> () const noexcept FLPURE STEPOVER {return _ref;}
const T* get() const noexcept FLPURE STEPOVER {return _ref;}
operator const T* () const & noexcept LIFETIMEBOUND FLPURE STEPOVER {return _ref;}
const T* operator-> () const noexcept LIFETIMEBOUND FLPURE STEPOVER {return _ref;}
const T* get() const noexcept LIFETIMEBOUND FLPURE STEPOVER {return _ref;}

RetainedConst& operator=(const T *t) & noexcept {
auto oldRef = _ref;
Expand Down
Loading
Loading