Skip to content

[ML] Correct the way internal linkage of constants is achieved #102

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

Merged
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
2 changes: 1 addition & 1 deletion include/core/COsFileFuncs.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ class CORE_EXPORT COsFileFuncs : private CNonInstantiatable {
static const int EXECUTABLE;

//! The name of the magic file that discards everything written to it
static const char* NULL_FILENAME;
static const char* const NULL_FILENAME;

public:
//! Signed size type (to be used instead of ssize_t)
Expand Down
8 changes: 4 additions & 4 deletions include/core/CProcess.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ namespace core {
class CORE_EXPORT CProcess : private CNonCopyable {
public:
//! These messages need to be 100% standard across all services
static const char* STARTING_MSG;
static const char* STARTED_MSG;
static const char* STOPPING_MSG;
static const char* STOPPED_MSG;
static const char* const STARTING_MSG;
static const char* const STARTED_MSG;
static const char* const STOPPING_MSG;
static const char* const STOPPED_MSG;

public:
//! Prototype of the mlMain() function
Expand Down
2 changes: 1 addition & 1 deletion include/core/CWordDictionary.h
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ class CORE_EXPORT CWordDictionary : private CNonCopyable {

private:
//! Name of the file to load that contains the dictionary words.
static const char* DICTIONARY_FILE;
static const char* const DICTIONARY_FILE;

//! The constructor loads a file, and hence may take a while. This
//! mutex prevents the singleton object being constructed simultaneously
Expand Down
2 changes: 1 addition & 1 deletion include/core/CXmlParser.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class CORE_EXPORT CXmlParser : public CXmlParserIntf {
static const std::string ATTRIBUTE_EQUALS;
static const size_t DEFAULT_INDENT_SPACES;
static const size_t MAX_INDENT_SPACES;
static const char* INDENT_SPACE_STR;
static const char* const INDENT_SPACE_STR;

public:
using TStrVec = std::vector<std::string>;
Expand Down
13 changes: 4 additions & 9 deletions include/core/CoreTypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,12 @@ using TTime = time_t;

//! The standard line ending for the platform - DON'T make this std::string as
//! that would cause many strings to be constructed (since the variable is
//! static const at the namespace level, so is internal to each file this
//! header is included in)
//! const at the namespace level, so is internal to each file this header is
//! included in)
#ifdef Windows
static const char* LINE_ENDING = "\r\n";
const char* const LINE_ENDING = "\r\n";
#else
#ifdef __GNUC__
// Tell g++ that it's reasonable that this variable isn't used
__attribute__((unused)) static const char* LINE_ENDING = "\n";
Copy link
Member

Choose a reason for hiding this comment

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

Not needed?

Copy link
Contributor Author

@droberts195 droberts195 May 23, 2018

Choose a reason for hiding this comment

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

No, now this really is a constant with internal linkage it's not needed.

In fact, the warning from g++ was the canary in the mine warning that something was not right. Suppressing the warning was the wrong way to avoid it.

#else
static const char* LINE_ENDING = "\n";
#endif
const char* const LINE_ENDING = "\n";
#endif
}
}
Expand Down
2 changes: 1 addition & 1 deletion include/model/ModelTypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -701,7 +701,7 @@ enum EMetricCategory {
};

//! Must correspond to the number of enumeration values of EMetricCategory
static const size_t NUM_METRIC_CATEGORIES = 9;
const size_t NUM_METRIC_CATEGORIES = 9;

//! Get the metric feature data corresponding to \p feature
//! if there is one.
Expand Down
20 changes: 10 additions & 10 deletions lib/api/unittest/CIoManagerTest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,19 @@ const uint32_t PAUSE_TIME_MS = 10;
const size_t MAX_ATTEMPTS = 100;
const size_t TEST_SIZE = 10000;
const char TEST_CHAR = 'a';
const char* GOOD_INPUT_FILE_NAME = "testfiles/good_input_file";
const char* GOOD_OUTPUT_FILE_NAME = "testfiles/good_output_file";
const char* const GOOD_INPUT_FILE_NAME = "testfiles/good_input_file";
const char* const GOOD_OUTPUT_FILE_NAME = "testfiles/good_output_file";
#ifdef Windows
const char* GOOD_INPUT_PIPE_NAME = "\\\\.\\pipe\\good_input_pipe";
const char* GOOD_OUTPUT_PIPE_NAME = "\\\\.\\pipe\\good_output_pipe";
const char* const GOOD_INPUT_PIPE_NAME = "\\\\.\\pipe\\good_input_pipe";
const char* const GOOD_OUTPUT_PIPE_NAME = "\\\\.\\pipe\\good_output_pipe";
#else
const char* GOOD_INPUT_PIPE_NAME = "testfiles/good_input_pipe";
const char* GOOD_OUTPUT_PIPE_NAME = "testfiles/good_output_pipe";
const char* const GOOD_INPUT_PIPE_NAME = "testfiles/good_input_pipe";
const char* const GOOD_OUTPUT_PIPE_NAME = "testfiles/good_output_pipe";
#endif
const char* BAD_INPUT_FILE_NAME = "can't_create_a_file_here/bad_input_file";
const char* BAD_OUTPUT_FILE_NAME = "can't_create_a_file_here/bad_output_file";
const char* BAD_INPUT_PIPE_NAME = "can't_create_a_pipe_here/bad_input_pipe";
const char* BAD_OUTPUT_PIPE_NAME = "can't_create_a_pipe_here/bad_output_pipe";
const char* const BAD_INPUT_FILE_NAME = "can't_create_a_file_here/bad_input_file";
const char* const BAD_OUTPUT_FILE_NAME = "can't_create_a_file_here/bad_output_file";
const char* const BAD_INPUT_PIPE_NAME = "can't_create_a_pipe_here/bad_input_pipe";
const char* const BAD_OUTPUT_PIPE_NAME = "can't_create_a_pipe_here/bad_output_pipe";

class CThreadDataWriter : public ml::core::CThread {
public:
Expand Down
2 changes: 1 addition & 1 deletion lib/core/CMutex_Windows.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
namespace {
// 4000 is a value that Microsoft uses in some of their code, so it's
// hopefully a reasonably sensible setting
static const DWORD SPIN_COUNT(4000);
const DWORD SPIN_COUNT(4000);
}

namespace ml {
Expand Down
2 changes: 1 addition & 1 deletion lib/core/COsFileFuncs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const int COsFileFuncs::READABLE(R_OK);
const int COsFileFuncs::WRITABLE(W_OK);
const int COsFileFuncs::EXECUTABLE(X_OK);

const char* COsFileFuncs::NULL_FILENAME("/dev/null");
const char* const COsFileFuncs::NULL_FILENAME("/dev/null");

int COsFileFuncs::open(const char* path, int oflag) {
return ::open(path, oflag);
Expand Down
2 changes: 1 addition & 1 deletion lib/core/COsFileFuncs_Windows.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ const int COsFileFuncs::WRITABLE(2);
// For Windows, consider "executable" the same as "readable" for the time being
const int COsFileFuncs::EXECUTABLE(4);

const char* COsFileFuncs::NULL_FILENAME("nul");
const char* const COsFileFuncs::NULL_FILENAME("nul");

int COsFileFuncs::open(const char* path, int oflag) {
return COsFileFuncs::open(path, oflag, 0);
Expand Down
8 changes: 4 additions & 4 deletions lib/core/CProcess.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@
namespace ml {
namespace core {

const char* CProcess::STARTING_MSG("Process Starting.");
const char* CProcess::STARTED_MSG("Process Started.");
const char* CProcess::STOPPING_MSG("Process Shutting Down.");
const char* CProcess::STOPPED_MSG("Process Exiting.");
const char* const CProcess::STARTING_MSG("Process Starting.");
const char* const CProcess::STARTED_MSG("Process Started.");
const char* const CProcess::STOPPING_MSG("Process Shutting Down.");
const char* const CProcess::STOPPED_MSG("Process Exiting.");

CProcess::CProcess()
: m_IsService(false), m_Initialised(false), m_Running(false),
Expand Down
8 changes: 4 additions & 4 deletions lib/core/CProcess_Windows.cc
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,10 @@ const DWORD PPID(findParentProcessId());
namespace ml {
namespace core {

const char* CProcess::STARTING_MSG("Process Starting.");
const char* CProcess::STARTED_MSG("Process Started.");
const char* CProcess::STOPPING_MSG("Process Shutting Down.");
const char* CProcess::STOPPED_MSG("Process Exiting.");
const char* const CProcess::STARTING_MSG("Process Starting.");
const char* const CProcess::STARTED_MSG("Process Started.");
const char* const CProcess::STOPPING_MSG("Process Shutting Down.");
const char* const CProcess::STOPPED_MSG("Process Exiting.");

CProcess::CProcess()
: m_IsService(false), m_Initialised(false), m_Running(false),
Expand Down
2 changes: 1 addition & 1 deletion lib/core/CResourceLocator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
#include <stdlib.h>

namespace {
const char* CPP_SRC_HOME("CPP_SRC_HOME");
const char* const CPP_SRC_HOME("CPP_SRC_HOME");
}

namespace ml {
Expand Down
10 changes: 5 additions & 5 deletions lib/core/CStatistics.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,13 @@ namespace {

using TGenericLineWriter = core::CRapidJsonLineWriter<rapidjson::OStreamWrapper>;

static const std::string NAME_TYPE("name");
static const std::string DESCRIPTION_TYPE("description");
static const std::string STAT_TYPE("value");
const std::string NAME_TYPE("name");
const std::string DESCRIPTION_TYPE("description");
const std::string STAT_TYPE("value");

//! Persistence tags
static const std::string KEY_TAG("a");
static const std::string VALUE_TAG("b");
const std::string KEY_TAG("a");
const std::string VALUE_TAG("b");

//! Helper function to add a string/int pair to JSON writer
void addStringInt(TGenericLineWriter& writer,
Expand Down
2 changes: 1 addition & 1 deletion lib/core/CUname_Windows.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ bool queryKernelVersion(uint16_t& major, uint16_t& minor, uint16_t& build) {
// then distinguish client/server versions of Windows using
// VerifyVersionInfo().

static const char* KERNEL32_DLL("kernel32.dll");
static const char* const KERNEL32_DLL("kernel32.dll");

DWORD handle(0);
DWORD size(GetFileVersionInfoSize(KERNEL32_DLL, &handle));
Expand Down
2 changes: 1 addition & 1 deletion lib/core/CWindowsError_Windows.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
#include <cmath>

namespace {
static const size_t BUFFER_SIZE(1024);
const size_t BUFFER_SIZE(1024);
}

namespace ml {
Expand Down
2 changes: 1 addition & 1 deletion lib/core/CWordDictionary.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ CWordDictionary::EPartOfSpeech partOfSpeechFromCode(char partOfSpeechCode) {
}
}

const char* CWordDictionary::DICTIONARY_FILE("ml-en.dict");
const char* const CWordDictionary::DICTIONARY_FILE("ml-en.dict");

CFastMutex CWordDictionary::ms_LoadMutex;
volatile CWordDictionary* CWordDictionary::ms_Instance(nullptr);
Expand Down
2 changes: 1 addition & 1 deletion lib/core/CXmlParser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const std::string CXmlParser::ATTRIBUTE_EQUALS("=");
const size_t CXmlParser::DEFAULT_INDENT_SPACES(4);
const size_t CXmlParser::MAX_INDENT_SPACES(10);
// The number of spaces in this constant MUST match the maximum above
const char* CXmlParser::INDENT_SPACE_STR(" ");
const char* const CXmlParser::INDENT_SPACE_STR(" ");

CXmlParser::CXmlParser()
: m_Doc(nullptr), m_XPathContext(nullptr), m_NavigatedNode(nullptr) {
Expand Down
2 changes: 1 addition & 1 deletion lib/core/unittest/CDualThreadStreamBufTest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ void CDualThreadStreamBufTest::testPutback() {

buf.signalEndOfFile();

static const char* PUTBACK_CHARS("put this back");
static const char* const PUTBACK_CHARS("put this back");
std::istream strm(&buf);
char c('\0');
CPPUNIT_ASSERT(strm.get(c).good());
Expand Down
4 changes: 2 additions & 2 deletions lib/core/unittest/CLoggerTest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@

namespace {
#ifdef Windows
const char* TEST_PIPE_NAME = "\\\\.\\pipe\\testpipe";
const char* const TEST_PIPE_NAME = "\\\\.\\pipe\\testpipe";
#else
const char* TEST_PIPE_NAME = "testfiles/testpipe";
const char* const TEST_PIPE_NAME = "testfiles/testpipe";
#endif
}

Expand Down
6 changes: 3 additions & 3 deletions lib/core/unittest/CNamedPipeFactoryTest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ const size_t MAX_ATTEMPTS = 100;
const size_t TEST_SIZE = 10000;
const char TEST_CHAR = 'a';
#ifdef Windows
const char* TEST_PIPE_NAME = "\\\\.\\pipe\\testpipe";
const char* const TEST_PIPE_NAME = "\\\\.\\pipe\\testpipe";
#else
const char* TEST_PIPE_NAME = "testfiles/testpipe";
const char* const TEST_PIPE_NAME = "testfiles/testpipe";
#endif

class CThreadDataWriter : public ml::core::CThread {
Expand Down Expand Up @@ -279,7 +279,7 @@ void CNamedPipeFactoryTest::testErrorIfSymlink() {
// the file system
LOG_DEBUG(<< "symlink test not relevant to Windows");
#else
static const char* TEST_SYMLINK_NAME = "test_symlink";
static const char* const TEST_SYMLINK_NAME = "test_symlink";

// Remove any files left behind by a previous failed test, but don't check
// the return codes as these calls will usually fail
Expand Down
2 changes: 1 addition & 1 deletion lib/maths/CSampling.cc
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ void sampleQuantiles(const DISTRIBUTION& distribution, std::size_t n, TDoubleVec
result.push_back(expectation(distribution, a, b));
}

static const std::string RNG_TAG("a");
const std::string RNG_TAG("a");
}

bool CSampling::staticsAcceptRestoreTraverser(core::CStateRestoreTraverser& traverser) {
Expand Down
6 changes: 3 additions & 3 deletions lib/maths/CXMeansOnline1d.cc
Original file line number Diff line number Diff line change
Expand Up @@ -588,9 +588,9 @@ const std::string DECAY_RATE_TAG("j");
const std::string HISTORY_LENGTH_TAG("k");

// CXMeansOnline1d::CCluster
static const std::string INDEX_TAG("a");
static const std::string STRUCTURE_TAG("b");
static const std::string PRIOR_TAG("c");
const std::string INDEX_TAG("a");
const std::string STRUCTURE_TAG("b");
const std::string PRIOR_TAG("c");

const std::string EMPTY_STRING;
}
Expand Down
10 changes: 5 additions & 5 deletions lib/model/CForecastModelPersist.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ namespace ml {
namespace model {

namespace {
static const std::string FORECAST_MODEL_PERSIST_TAG("forecast_persist");
static const std::string FEATURE_TAG("feature");
static const std::string DATA_TYPE_TAG("datatype");
static const std::string MODEL_TAG("model");
static const std::string BY_FIELD_VALUE_TAG("by_field_value");
const std::string FORECAST_MODEL_PERSIST_TAG("forecast_persist");
const std::string FEATURE_TAG("feature");
const std::string DATA_TYPE_TAG("datatype");
const std::string MODEL_TAG("model");
const std::string BY_FIELD_VALUE_TAG("by_field_value");
}

CForecastModelPersist::CPersist::CPersist(const std::string& temporaryPath)
Expand Down