Remove unnecessary, confusing 'extern' (#12300)

Summary:
In C++, `extern` is redundant in a number of cases:
* "Global" function declarations and definitions
* "Global" variable definitions when already declared `extern`

For consistency and simplicity, I've removed these in code that *we own*. In a couple of cases, I removed obsolete declarations, and for MagicNumber constants, I have consolidated the declarations into a header file (format.h)
as standard best practice would prescribe.

Pull Request resolved: #12300

Test Plan: no functional changes, CI

Reviewed By: ajkr

Differential Revision: D53148629

Pulled By: pdillinger

fbshipit-source-id: fb8d927959892e03af09b0c0d542b0a3b38fd886

db/arena_wrapped_db_iter.h

@@ -118,7 +118,7 @@ class ArenaWrappedDBIter : public Iterator {
 // Generate the arena wrapped iterator class.
 // `db_impl` and `cfd` are used for reneweal. If left null, renewal will not
 // be supported.
-extern ArenaWrappedDBIter* NewArenaWrappedDbIterator(
+ArenaWrappedDBIter* NewArenaWrappedDbIterator(
     Env* env, const ReadOptions& read_options, const ImmutableOptions& ioptions,
     const MutableCFOptions& mutable_cf_options, const Version* version,
     const SequenceNumber& sequence, uint64_t max_sequential_skip_in_iterations,

db/builder.h

@@ -50,7 +50,7 @@ TableBuilder* NewTableBuilder(const TableBuilderOptions& tboptions,
 //
 // @param column_family_name Name of the column family that is also identified
 //    by column_family_id, or empty string if unknown.
-extern Status BuildTable(
+Status BuildTable(
     const std::string& dbname, VersionSet* versions,
     const ImmutableDBOptions& db_options, const TableBuilderOptions& tboptions,
     const FileOptions& file_options, TableCache* table_cache,

db/c.cc

@@ -3144,7 +3144,7 @@ void rocksdb_options_set_enable_blob_files(rocksdb_options_t* opt,
                                            unsigned char val) {
   opt->rep.enable_blob_files = val;
 }
-extern ROCKSDB_LIBRARY_API unsigned char rocksdb_options_get_enable_blob_files(
+ROCKSDB_LIBRARY_API unsigned char rocksdb_options_get_enable_blob_files(
     rocksdb_options_t* opt) {
   return opt->rep.enable_blob_files;
 }
@@ -4636,7 +4636,7 @@ void rocksdb_readoptions_set_io_timeout(rocksdb_readoptions_t* opt,
   opt->rep.io_timeout = std::chrono::microseconds(microseconds);
 }
 
-extern ROCKSDB_LIBRARY_API uint64_t
+ROCKSDB_LIBRARY_API uint64_t
 rocksdb_readoptions_get_io_timeout(rocksdb_readoptions_t* opt) {
   return opt->rep.io_timeout.count();
 }
@@ -5469,9 +5469,7 @@ uint64_t rocksdb_livefiles_deletions(const rocksdb_livefiles_t* lf, int index) {
   return lf->rep[index].num_deletions;
 }
 
-extern void rocksdb_livefiles_destroy(const rocksdb_livefiles_t* lf) {
-  delete lf;
-}
+void rocksdb_livefiles_destroy(const rocksdb_livefiles_t* lf) { delete lf; }
 
 void rocksdb_get_options_from_string(const rocksdb_options_t* base_options,
                                      const char* opts_str,

db/column_family.h

@@ -251,20 +251,19 @@ struct SuperVersion {
   autovector<MemTable*> to_delete;
 };
 
-extern Status CheckCompressionSupported(const ColumnFamilyOptions& cf_options);
+Status CheckCompressionSupported(const ColumnFamilyOptions& cf_options);
 
-extern Status CheckConcurrentWritesSupported(
-    const ColumnFamilyOptions& cf_options);
+Status CheckConcurrentWritesSupported(const ColumnFamilyOptions& cf_options);
 
-extern Status CheckCFPathsSupported(const DBOptions& db_options,
-                                    const ColumnFamilyOptions& cf_options);
+Status CheckCFPathsSupported(const DBOptions& db_options,
+                             const ColumnFamilyOptions& cf_options);
 
-extern ColumnFamilyOptions SanitizeOptions(const ImmutableDBOptions& db_options,
-                                           const ColumnFamilyOptions& src);
+ColumnFamilyOptions SanitizeOptions(const ImmutableDBOptions& db_options,
+                                    const ColumnFamilyOptions& src);
 // Wrap user defined table properties collector factories `from cf_options`
 // into internal ones in int_tbl_prop_collector_factories. Add a system internal
 // one too.
-extern void GetIntTblPropCollectorFactory(
+void GetIntTblPropCollectorFactory(
     const ImmutableCFOptions& ioptions,
     IntTblPropCollectorFactories* int_tbl_prop_collector_factories);
 
@@ -872,12 +871,11 @@ class ColumnFamilyMemTablesImpl : public ColumnFamilyMemTables {
   ColumnFamilyHandleInternal handle_;
 };
 
-extern uint32_t GetColumnFamilyID(ColumnFamilyHandle* column_family);
+uint32_t GetColumnFamilyID(ColumnFamilyHandle* column_family);
 
-extern const Comparator* GetColumnFamilyUserComparator(
+const Comparator* GetColumnFamilyUserComparator(
     ColumnFamilyHandle* column_family);
 
-extern const ImmutableOptions& GetImmutableOptions(
-    ColumnFamilyHandle* column_family);
+const ImmutableOptions& GetImmutableOptions(ColumnFamilyHandle* column_family);
 
 }  // namespace ROCKSDB_NAMESPACE

db/compaction/compaction.h

@@ -595,6 +595,6 @@ struct PerKeyPlacementContext {
 #endif /* !NDEBUG */
 
 // Return sum of sizes of all files in `files`.
-extern uint64_t TotalFileSize(const std::vector<FileMetaData*>& files);
+uint64_t TotalFileSize(const std::vector<FileMetaData*>& files);
 
 }  // namespace ROCKSDB_NAMESPACE

db/db_impl/compacted_db_impl.cc

@@ -13,10 +13,6 @@
 
 namespace ROCKSDB_NAMESPACE {
 
-extern void MarkKeyMayExist(void* arg);
-extern bool SaveValue(void* arg, const ParsedInternalKey& parsed_key,
-                      const Slice& v, bool hit_and_return);
-
 CompactedDBImpl::CompactedDBImpl(const DBOptions& options,
                                  const std::string& dbname)
     : DBImpl(options, dbname, /*seq_per_batch*/ false, +/*batch_per_txn*/ true,

db/db_impl/db_impl.h

@@ -2855,53 +2855,52 @@ class GetWithTimestampReadCallback : public ReadCallback {
   }
 };
 
-extern Options SanitizeOptions(const std::string& db, const Options& src,
-                               bool read_only = false,
-                               Status* logger_creation_s = nullptr);
+Options SanitizeOptions(const std::string& db, const Options& src,
+                        bool read_only = false,
+                        Status* logger_creation_s = nullptr);
 
-extern DBOptions SanitizeOptions(const std::string& db, const DBOptions& src,
-                                 bool read_only = false,
-                                 Status* logger_creation_s = nullptr);
+DBOptions SanitizeOptions(const std::string& db, const DBOptions& src,
+                          bool read_only = false,
+                          Status* logger_creation_s = nullptr);
 
-extern CompressionType GetCompressionFlush(
-    const ImmutableCFOptions& ioptions,
-    const MutableCFOptions& mutable_cf_options);
+CompressionType GetCompressionFlush(const ImmutableCFOptions& ioptions,
+                                    const MutableCFOptions& mutable_cf_options);
 
 // Return the earliest log file to keep after the memtable flush is
 // finalized.
 // `cfd_to_flush` is the column family whose memtable (specified in
 // `memtables_to_flush`) will be flushed and thus will not depend on any WAL
 // file.
 // The function is only applicable to 2pc mode.
-extern uint64_t PrecomputeMinLogNumberToKeep2PC(
+uint64_t PrecomputeMinLogNumberToKeep2PC(
     VersionSet* vset, const ColumnFamilyData& cfd_to_flush,
     const autovector<VersionEdit*>& edit_list,
     const autovector<MemTable*>& memtables_to_flush,
     LogsWithPrepTracker* prep_tracker);
 // For atomic flush.
-extern uint64_t PrecomputeMinLogNumberToKeep2PC(
+uint64_t PrecomputeMinLogNumberToKeep2PC(
     VersionSet* vset, const autovector<ColumnFamilyData*>& cfds_to_flush,
     const autovector<autovector<VersionEdit*>>& edit_lists,
     const autovector<const autovector<MemTable*>*>& memtables_to_flush,
     LogsWithPrepTracker* prep_tracker);
 
 // In non-2PC mode, WALs with log number < the returned number can be
 // deleted after the cfd_to_flush column family is flushed successfully.
-extern uint64_t PrecomputeMinLogNumberToKeepNon2PC(
+uint64_t PrecomputeMinLogNumberToKeepNon2PC(
     VersionSet* vset, const ColumnFamilyData& cfd_to_flush,
     const autovector<VersionEdit*>& edit_list);
 // For atomic flush.
-extern uint64_t PrecomputeMinLogNumberToKeepNon2PC(
+uint64_t PrecomputeMinLogNumberToKeepNon2PC(
     VersionSet* vset, const autovector<ColumnFamilyData*>& cfds_to_flush,
     const autovector<autovector<VersionEdit*>>& edit_lists);
 
 // `cfd_to_flush` is the column family whose memtable will be flushed and thus
 // will not depend on any WAL file. nullptr means no memtable is being flushed.
 // The function is only applicable to 2pc mode.
-extern uint64_t FindMinPrepLogReferencedByMemTable(
+uint64_t FindMinPrepLogReferencedByMemTable(
     VersionSet* vset, const autovector<MemTable*>& memtables_to_flush);
 // For atomic flush.
-extern uint64_t FindMinPrepLogReferencedByMemTable(
+uint64_t FindMinPrepLogReferencedByMemTable(
     VersionSet* vset,
     const autovector<const autovector<MemTable*>*>& memtables_to_flush);
 

db/db_iter.h

@@ -428,13 +428,15 @@ class DBIter final : public Iterator {
 // Return a new iterator that converts internal keys (yielded by
 // "*internal_iter") that were live at the specified `sequence` number
 // into appropriate user keys.
-extern Iterator* NewDBIterator(
-    Env* env, const ReadOptions& read_options, const ImmutableOptions& ioptions,
-    const MutableCFOptions& mutable_cf_options,
-    const Comparator* user_key_comparator, InternalIterator* internal_iter,
-    const Version* version, const SequenceNumber& sequence,
-    uint64_t max_sequential_skip_in_iterations, ReadCallback* read_callback,
-    DBImpl* db_impl = nullptr, ColumnFamilyData* cfd = nullptr,
-    bool expose_blob_index = false);
+Iterator* NewDBIterator(Env* env, const ReadOptions& read_options,
+                        const ImmutableOptions& ioptions,
+                        const MutableCFOptions& mutable_cf_options,
+                        const Comparator* user_key_comparator,
+                        InternalIterator* internal_iter, const Version* version,
+                        const SequenceNumber& sequence,
+                        uint64_t max_sequential_skip_in_iterations,
+                        ReadCallback* read_callback, DBImpl* db_impl = nullptr,
+                        ColumnFamilyData* cfd = nullptr,
+                        bool expose_blob_index = false);
 
 }  // namespace ROCKSDB_NAMESPACE

db/internal_stats.h

@@ -58,7 +58,7 @@ struct DBPropertyInfo {
   bool (DBImpl::*handle_string_dbimpl)(std::string* value);
 };
 
-extern const DBPropertyInfo* GetPropertyInfo(const Slice& property);
+const DBPropertyInfo* GetPropertyInfo(const Slice& property);
 
 #undef SCORE
 enum class LevelStatType {

db/memtable.h

@@ -699,6 +699,6 @@ class MemTable {
   void MaybeUpdateNewestUDT(const Slice& user_key);
 };
 
-extern const char* EncodeKey(std::string* scratch, const Slice& target);
+const char* EncodeKey(std::string* scratch, const Slice& target);
 
 }  // namespace ROCKSDB_NAMESPACE

db/memtable_list.h

@@ -498,7 +498,7 @@ class MemTableList {
 // installs flush results for external immutable memtable lists other than the
 // cfds' own immutable memtable lists, e.g. MemTableLIstTest. In this case,
 // imm_lists parameter is not nullptr.
-extern Status InstallMemtableAtomicFlushResults(
+Status InstallMemtableAtomicFlushResults(
     const autovector<MemTableList*>* imm_lists,
     const autovector<ColumnFamilyData*>& cfds,
     const autovector<const MutableCFOptions*>& mutable_cf_options_list,

db/plain_table_db_test.cc

@@ -252,8 +252,6 @@ TEST_P(PlainTableDBTest, Empty) {
   ASSERT_EQ("NOT_FOUND", Get("0000000000000foo"));
 }
 
-extern const uint64_t kPlainTableMagicNumber;
-
 class TestPlainTableReader : public PlainTableReader {
  public:
   TestPlainTableReader(
@@ -307,7 +305,6 @@ class TestPlainTableReader : public PlainTableReader {
   bool* expect_bloom_not_match_;
 };
 
-extern const uint64_t kPlainTableMagicNumber;
 class TestPlainTableFactory : public PlainTableFactory {
  public:
   explicit TestPlainTableFactory(bool* expect_bloom_not_match,

db/table_properties_collector_test.cc

@@ -242,8 +242,6 @@ class FlushBlockEveryThreePolicyFactory : public FlushBlockPolicyFactory {
   }
 };
 
-extern const uint64_t kBlockBasedTableMagicNumber;
-extern const uint64_t kPlainTableMagicNumber;
 namespace {
 void TestCustomizedTablePropertiesCollector(
     bool backward_mode, uint64_t magic_number, bool test_int_tbl_prop_collector,

db/version_edit.h

@@ -113,7 +113,7 @@ constexpr uint64_t kUnknownEpochNumber = 0;
 // will be dedicated to files ingested behind.
 constexpr uint64_t kReservedEpochNumberForFileIngestedBehind = 1;
 
-extern uint64_t PackFileNumberAndPathId(uint64_t number, uint64_t path_id);
+uint64_t PackFileNumberAndPathId(uint64_t number, uint64_t path_id);
 
 // A copyable structure contains information needed to read data from an SST
 // file. It can contain a pointer to a table reader opened for the file, or

db/version_set.h

@@ -98,27 +98,27 @@ using VersionEditParams = VersionEdit;
 // Return file_level.num_files if there is no such file.
 // REQUIRES: "file_level.files" contains a sorted list of
 // non-overlapping files.
-extern int FindFile(const InternalKeyComparator& icmp,
-                    const LevelFilesBrief& file_level, const Slice& key);
+int FindFile(const InternalKeyComparator& icmp,
+             const LevelFilesBrief& file_level, const Slice& key);
 
 // Returns true iff some file in "files" overlaps the user key range
 // [*smallest,*largest].
 // smallest==nullptr represents a key smaller than all keys in the DB.
 // largest==nullptr represents a key largest than all keys in the DB.
 // REQUIRES: If disjoint_sorted_files, file_level.files[]
 // contains disjoint ranges in sorted order.
-extern bool SomeFileOverlapsRange(const InternalKeyComparator& icmp,
-                                  bool disjoint_sorted_files,
-                                  const LevelFilesBrief& file_level,
-                                  const Slice* smallest_user_key,
-                                  const Slice* largest_user_key);
+bool SomeFileOverlapsRange(const InternalKeyComparator& icmp,
+                           bool disjoint_sorted_files,
+                           const LevelFilesBrief& file_level,
+                           const Slice* smallest_user_key,
+                           const Slice* largest_user_key);
 
 // Generate LevelFilesBrief from vector<FdWithKeyRange*>
 // Would copy smallest_key and largest_key data to sequential memory
 // arena: Arena used to allocate the memory
-extern void DoGenerateLevelFilesBrief(LevelFilesBrief* file_level,
-                                      const std::vector<FileMetaData*>& files,
-                                      Arena* arena);
+void DoGenerateLevelFilesBrief(LevelFilesBrief* file_level,
+                               const std::vector<FileMetaData*>& files,
+                               Arena* arena);
 enum EpochNumberRequirement {
   kMightMissing,
   kMustPresent,

db/write_stall_stats.h

@@ -11,31 +11,31 @@
 #include "rocksdb/types.h"
 
 namespace ROCKSDB_NAMESPACE {
-extern const std::string& InvalidWriteStallHyphenString();
+const std::string& InvalidWriteStallHyphenString();
 
-extern const std::string& WriteStallCauseToHyphenString(WriteStallCause cause);
+const std::string& WriteStallCauseToHyphenString(WriteStallCause cause);
 
-extern const std::string& WriteStallConditionToHyphenString(
+const std::string& WriteStallConditionToHyphenString(
     WriteStallCondition condition);
 
 // REQUIRES:
 // cause` is CF-scope `WriteStallCause`, see `WriteStallCause` for more
 //
 // REQUIRES:
 // `condition` != `WriteStallCondition::kNormal`
-extern InternalStats::InternalCFStatsType InternalCFStat(
+InternalStats::InternalCFStatsType InternalCFStat(
     WriteStallCause cause, WriteStallCondition condition);
 
 // REQUIRES:
 // cause` is DB-scope `WriteStallCause`, see `WriteStallCause` for more
 //
 // REQUIRES:
 // `condition` != `WriteStallCondition::kNormal`
-extern InternalStats::InternalDBStatsType InternalDBStat(
+InternalStats::InternalDBStatsType InternalDBStat(
     WriteStallCause cause, WriteStallCondition condition);
 
-extern bool isCFScopeWriteStallCause(WriteStallCause cause);
-extern bool isDBScopeWriteStallCause(WriteStallCause cause);
+bool isCFScopeWriteStallCause(WriteStallCause cause);
+bool isDBScopeWriteStallCause(WriteStallCause cause);
 
 constexpr uint32_t kNumCFScopeWriteStallCauses =
     static_cast<uint32_t>(WriteStallCause::kCFScopeWriteStallCauseEnumMax) -