Code safety/modernization

API/fleece/CompilerSupport.h

@@ -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
@@ -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)
@@ -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
@@ -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
@@ -271,6 +266,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

API/fleece/FLDoc.h

@@ -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;

API/fleece/FLEncoder.h

@@ -206,7 +206,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;
 
     /** @} */
@@ -219,7 +218,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;
 
     /** @} */
     /** @} */

API/fleece/FLSlice.h

@@ -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;
 
@@ -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
 };
@@ -123,7 +129,7 @@ static inline void FLMemCpy(void* FL_NULLABLE dst, const void* FL_NULLABLE src,
     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`. */
-static inline FLSlice FLStr(const char* FL_NULLABLE str) FLAPI {
+static inline FLSlice FLStr(const char* FL_NULLABLE str LIFETIMEBOUND) FLAPI {
     FLSlice foo = { str, str ? strlen(str) : 0 };
     return foo;
 }
@@ -185,7 +191,7 @@ static inline void FLSliceResult_Release(FLSliceResult s) FLAPI {
 }
 
 /** Type-casts a FLSliceResult to FLSlice, since C doesn't know it's a subclass. */
-static inline FLSlice FLSliceResult_AsSlice(FLSliceResult sr) {
+static inline FLSlice FLSliceResult_AsSlice(FLSliceResult sr LIFETIMEBOUND) {
     FLSlice ret;
     memcpy(&ret, &sr, sizeof(ret));
     return ret;

API/fleece/Fleece.hh

@@ -78,7 +78,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;
@@ -128,9 +127,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;
 
@@ -148,7 +146,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;
@@ -186,7 +184,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;
 
@@ -260,8 +257,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) { }
@@ -333,7 +330,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
@@ -355,23 +352,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;}
 
@@ -414,7 +411,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;}
 
@@ -433,7 +430,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);
@@ -528,6 +525,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);}

API/fleece/PlatformCompat.hh

@@ -12,16 +12,11 @@
 
 #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 LITECORE_UNUSED
     #define __typeof                        decltype
 
     #define __func__                        __FUNCTION__
@@ -38,11 +33,6 @@
 
 #else
 
-    // Suppresses "unused function" warnings
-    #if __has_attribute(unused)
-    #  define LITECORE_UNUSED __attribute__((unused))
-    #endif
-
     // Disables inlining a function. Use when the space savings are worth more than speed.
     #if __has_attribute(noinline)
     #  define NOINLINE                      __attribute((noinline))

API/fleece/RefCounted.hh

@@ -12,7 +12,6 @@
 
 #pragma once
 #include "fleece/PlatformCompat.hh"
-#include <algorithm>
 #include <atomic>
 #include <utility>
 
@@ -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);}
 
@@ -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:
@@ -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;