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.

Sign up for GitHub

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

Jump to bottom

言語DLLのMakefileのMYDEFINESの指定方法をシンプルに戻す #1389

Merged
merged 1 commit into from Sep 5, 2020
Merged

言語DLLのMakefileのMYDEFINESの指定方法をシンプルに戻す #1389

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 5 additions & 5 deletions sakura_lang_en_US/Makefile
View file
Open in desktop
Expand Up @@ -13,7 +13,7 @@
#
# Debug build:
# $ cd sakura_lang_en_US
# $ mingw32-make MYDEFINES="_DEBUG" -j4
# $ mingw32-make MYDEFINES="-D_DEBUG" -j4

# Path of "sakura_lang_en_US" directory. Compute it from the path of Makefile.
SRCDIR = $(patsubst %/,%,$(subst \,/,$(dir $(firstword $(MAKEFILE_LIST)))))
Expand Down Expand Up @@ -61,18 +61,18 @@ endif
CC= $(PREFIX)gcc
RC= $(RCPREFIX)windres

ifeq (_DEBUG,$(findstring _DEBUG,$(MYDEFINES)))
else ifeq (NDEBUG,$(findstring NDEBUG,$(MYDEFINES)))
ifeq (-D_DEBUG,$(findstring -D_DEBUG,$(MYDEFINES)))
else ifeq (-DNDEBUG,$(findstring -DNDEBUG,$(MYDEFINES)))
else
MYDEFINES += NDEBUG
MYDEFINES += -DNDEBUG
endif
Comment on lines -64 to 68
Copy link
Member

Choose a reason for hiding this comment

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

NDEBUG ってリソースファイルの中では使っていないような気がするのですが、このブロック必要ですか？

Copy link
Contributor Author

Choose a reason for hiding this comment

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

使ってませんが、あったほうがより良いと思います。

Copy link
Member

Choose a reason for hiding this comment

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

本体の Makefile にもこの部分はないですが、追加する予定でしょうか。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

あったほうが良い理由は、MSVC側のReleaseビルドで指定されるフラグだからです。
パターンは4つあって、実質的に使われるのは2つですが NDEBUG を指定しなくてよい理由はない気がします。

-- _DEBUG NDEBUG
Debugビルド 定義あり 定義なし
Releaseビルド 定義なし 定義あり
ありえない1 定義なし 定義なし
ありえない2 定義あり 定義あり

このブロックにより ありえない1 の発生を排除することができます。
ありえない2 の排除は無理だと思うので放置します。

Copy link
Member

Choose a reason for hiding this comment

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

本体の Makefile にはこの部分がなくて、言語 DLL の方にだけあるのは統一が取れていないので、両方から削除するか、両方に追加するかどちらかにした方がいいのでは？

Copy link
Contributor Author

Choose a reason for hiding this comment

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

本体の Makefile にはこの部分がなくて、言語 DLL の方にだけあるのは統一が取れていないので、両方から削除するか、両方に追加するかどちらかにした方がいいのでは？

本体に追加したいです。
それはいつか別PRで対処したいです。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

いまやっときます？

Copy link
Member

Choose a reason for hiding this comment

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

別PRでいいでしょう。
方針が決まらなければ、このブロックが必要かどうかの判断ができないので聞いただけのことです。


DEFINES= \
-DWIN32 \
-D_WIN32_WINNT=_WIN32_WINNT_WIN7 \
-D_UNICODE \
-DUNICODE \
$(addprefix -D,$(MYDEFINES))
$(MYDEFINES)
LDFLAGS= \
-static \
-shared \
Expand Down