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

fix: change hello_world cmake compiler use -std=c++17 #116

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 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
5 changes: 4 additions & 1 deletion examples/hello_world/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@

cmake_minimum_required(VERSION 3.11)
project(hello_world)
set(CMAKE_CXX_STANDARD 17)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
#set(CMAKE_CXX_FLAGS "-Wall -Wextra -fPIC -Wno-unused-parameter")

include(FetchContent)
FetchContent_Declare(highway GIT_REPOSITORY https://github.com/google/highway.git GIT_TAG da250571a45826b21eebbddc1e50d0c1137dee5f)
Expand All @@ -33,7 +36,7 @@ if (BUILD_MODE STREQUAL "local")
# Relative path to gemma.cpp from examples/hello_world/build/
FetchContent_Declare(gemma SOURCE_DIR ../../..)
else()
FetchContent_Declare(gemma GIT_REPOSITORY https://github.com/google/gemma.cpp.git GIT_TAG a9aa63fd2ea6b786ed0706d619588bfe2d43370e)
FetchContent_Declare(gemma GIT_REPOSITORY https://github.com/google/gemma.cpp.git GIT_TAG 73db33580b90f55042285b99139056c632f159bd)
endif()
FetchContent_MakeAvailable(gemma)

Expand Down
2 changes: 1 addition & 1 deletion examples/hello_world/run.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ std::vector<int> tokenize(
return tokens;
}

int main(int argc, char** argv) {
int main(int argc, const char** argv) {
Copy link
Member

Choose a reason for hiding this comment

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

Why do we use const here? char** or char*[] is standard, see 3.6.1 in https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2012/n3337.pdf.

Copy link
Author

@weedge weedge Mar 21, 2024

Choose a reason for hiding this comment

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

argv from command params just read, so want use const char**, want use libgemma util/app.h this function gcpp::LoaderArgs loader(argc, argv) i want to cast const char** to char** , like this char** nonConstPtr = const_cast<char*>(constPtr);; but this command argv is immutable

Copy link
Author

Choose a reason for hiding this comment

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

Why do we use const here? char** or char*[] is standard, see 3.6.1 in https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2012/n3337.pdf.

use main(int argc, char** argv) it's ok~

gcpp::LoaderArgs loader(argc, argv);

// Rough heuristic for the number of threads to use
Expand Down
2 changes: 1 addition & 1 deletion run.cc
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ void Run(LoaderArgs& loader, InferenceArgs& inference, AppArgs& app) {

} // namespace gcpp

int main(int argc, char** argv) {
int main(int argc, const char** argv) {
{
PROFILER_ZONE("Startup.misc");

Expand Down
6 changes: 3 additions & 3 deletions util/app.h
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ class AppArgs : public ArgsBase<AppArgs> {
}

public:
AppArgs(int argc, char* argv[]) {
AppArgs(int argc, const char* argv[]) {
InitAndParse(argc, argv);
ChooseNumThreads();
}
Expand Down Expand Up @@ -128,7 +128,7 @@ class AppArgs : public ArgsBase<AppArgs> {
};

struct LoaderArgs : public ArgsBase<LoaderArgs> {
LoaderArgs(int argc, char* argv[]) { InitAndParse(argc, argv); }
LoaderArgs(int argc, const char* argv[]) { InitAndParse(argc, argv); }

static std::string ToLower(const std::string& text) {
std::string result = text;
Expand Down Expand Up @@ -205,7 +205,7 @@ struct LoaderArgs : public ArgsBase<LoaderArgs> {
};

struct InferenceArgs : public ArgsBase<InferenceArgs> {
InferenceArgs(int argc, char* argv[]) { InitAndParse(argc, argv); }
InferenceArgs(int argc, const char* argv[]) { InitAndParse(argc, argv); }

size_t max_tokens;
size_t max_generated_tokens;
Expand Down
10 changes: 5 additions & 5 deletions util/args.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ class ArgsBase {
// consider adding a hash-map to speed this up.
class ParseVisitor {
public:
ParseVisitor(int argc, char* argv[]) : argc_(argc), argv_(argv) {}
ParseVisitor(int argc, const char* argv[]) : argc_(argc), argv_(argv) {}

template <typename T>
void operator()(T& t, const char* name, const T& /*init*/,
Expand Down Expand Up @@ -167,7 +167,7 @@ class ArgsBase {
}

int argc_;
char** argv_;
const char** argv_;
}; // ParseVisitor

template <class Visitor>
Expand All @@ -192,19 +192,19 @@ class ArgsBase {
ForEach(visitor);
}

void Parse(int argc, char* argv[]) {
void Parse(int argc, const char* argv[]) {
ParseVisitor visitor(argc, argv);
ForEach(visitor);
}

// For convenience, enables single-line constructor.
void InitAndParse(int argc, char* argv[]) {
void InitAndParse(int argc, const char* argv[]) {
Init();
Parse(argc, argv);
}
};

static inline HWY_MAYBE_UNUSED bool HasHelp(int argc, char* argv[]) {
static inline HWY_MAYBE_UNUSED bool HasHelp(int argc, const char* argv[]) {
// TODO(austinvhuang): handle case insensitivity
if (argc == 1) {
// no arguments - print help
Expand Down