[clang] [clang] replaced the usage of `asctime` with `strftime` (PR #99075)
https://github.com/yichi170 updated https://github.com/llvm/llvm-project/pull/99075 >From 0fcbd4a46bb05ad9829fcf33a9829fd5f5269c71 Mon Sep 17 00:00:00 2001 From: yichi170 Date: Wed, 17 Jul 2024 02:15:43 +0800 Subject: [PATCH 1/4] [clang] replaced the usage of `asctime` with `strftime` --- clang/lib/Lex/PPMacroExpansion.cpp | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/clang/lib/Lex/PPMacroExpansion.cpp b/clang/lib/Lex/PPMacroExpansion.cpp index 3913ff08c2eb5..f7e657be87932 100644 --- a/clang/lib/Lex/PPMacroExpansion.cpp +++ b/clang/lib/Lex/PPMacroExpansion.cpp @@ -1722,10 +1722,12 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) { // MSVC, ICC, GCC, VisualAge C++ extension. The generated string should be // of the form "Ddd Mmm dd hh::mm::ss ", which is returned by asctime. const char *Result; +char TimeString[std::size("Ddd Mmm dd hh:mm:ss ")]; if (getPreprocessorOpts().SourceDateEpoch) { time_t TT = *getPreprocessorOpts().SourceDateEpoch; std::tm *TM = std::gmtime(&TT); - Result = asctime(TM); + strftime(TimeString, std::size(TimeString), "%c", TM); + Result = TimeString; } else { // Get the file that we are lexing out of. If we're currently lexing from // a macro, dig into the include stack. @@ -1735,13 +1737,13 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) { if (CurFile) { time_t TT = CurFile->getModificationTime(); struct tm *TM = localtime(&TT); -Result = asctime(TM); +strftime(TimeString, std::size(TimeString), "%c", TM); +Result = TimeString; } else { -Result = "??? ??? ?? ??:??:?? \n"; +Result = "??? ??? ?? ??:??:?? "; } } -// Surround the string with " and strip the trailing newline. -OS << '"' << StringRef(Result).drop_back() << '"'; +OS << '"' << Result << '"'; Tok.setKind(tok::string_literal); } else if (II == Ident__FLT_EVAL_METHOD__) { // __FLT_EVAL_METHOD__ is set to the default value. >From 0352a494b6d0735ad83aa9189d33f2e50a1349d0 Mon Sep 17 00:00:00 2001 From: yichi170 Date: Wed, 17 Jul 2024 22:17:48 +0800 Subject: [PATCH 2/4] [clang] handling zero return of `strgtime` in `clang/lib/Lex/PPMacroExpansion.cpp` --- clang/lib/Lex/PPMacroExpansion.cpp | 14 ++ 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/clang/lib/Lex/PPMacroExpansion.cpp b/clang/lib/Lex/PPMacroExpansion.cpp index f7e657be87932..89697872901d7 100644 --- a/clang/lib/Lex/PPMacroExpansion.cpp +++ b/clang/lib/Lex/PPMacroExpansion.cpp @@ -1721,13 +1721,13 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) { Diag(Tok.getLocation(), diag::warn_pp_date_time); // MSVC, ICC, GCC, VisualAge C++ extension. The generated string should be // of the form "Ddd Mmm dd hh::mm::ss ", which is returned by asctime. -const char *Result; -char TimeString[std::size("Ddd Mmm dd hh:mm:ss ")]; +const char *Result = "??? ??? ?? ??:??:?? "; +char TimeString[64]; if (getPreprocessorOpts().SourceDateEpoch) { time_t TT = *getPreprocessorOpts().SourceDateEpoch; std::tm *TM = std::gmtime(&TT); - strftime(TimeString, std::size(TimeString), "%c", TM); - Result = TimeString; + if (strftime(TimeString, sizeof TimeString, "%a %b %e %T %Y", TM) != 0) +Result = TimeString; } else { // Get the file that we are lexing out of. If we're currently lexing from // a macro, dig into the include stack. @@ -1737,10 +1737,8 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) { if (CurFile) { time_t TT = CurFile->getModificationTime(); struct tm *TM = localtime(&TT); -strftime(TimeString, std::size(TimeString), "%c", TM); -Result = TimeString; - } else { -Result = "??? ??? ?? ??:??:?? "; +if (strftime(TimeString, sizeof TimeString, "%a %b %e %T %Y", TM) != 0) + Result = TimeString; } } OS << '"' << Result << '"'; >From 3764d5de783e42cc3cd15faabe9d4fd29f5b31cd Mon Sep 17 00:00:00 2001 From: yichi170 Date: Tue, 23 Jul 2024 08:35:39 +0800 Subject: [PATCH 3/4] [clang] replace the usage of `asctime` with `std::put_time` --- clang/lib/Lex/PPMacroExpansion.cpp | 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/clang/lib/Lex/PPMacroExpansion.cpp b/clang/lib/Lex/PPMacroExpansion.cpp index 89697872901d7..ea9c8e66d1a3d 100644 --- a/clang/lib/Lex/PPMacroExpansion.cpp +++ b/clang/lib/Lex/PPMacroExpansion.cpp @@ -52,7 +52,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -1721,13 +1723,15 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) { Diag(Tok.getLocation(), diag::warn_pp_date_time); // MSVC, ICC, GCC, VisualAge C++ extension. The generated string should be // of the form "Ddd Mmm dd hh::mm
[clang] [clang] replaced the usage of `asctime` with `strftime` (PR #99075)
@@ -1721,11 +1723,15 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) { Diag(Tok.getLocation(), diag::warn_pp_date_time); // MSVC, ICC, GCC, VisualAge C++ extension. The generated string should be // of the form "Ddd Mmm dd hh::mm::ss ", which is returned by asctime. -const char *Result; +std::string Result = "??? ??? ?? ??:??:?? "; +std::stringstream TmpStream; +TmpStream.imbue(std::locale("C")); +TmpStream.setstate(std::ios_base::badbit); yichi170 wrote: Fixed it. Thanks for the advice. https://github.com/llvm/llvm-project/pull/99075 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] replaced the usage of `asctime` with `std::put_time` (PR #99075)
https://github.com/yichi170 edited https://github.com/llvm/llvm-project/pull/99075 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] replaced the usage of `asctime` with `std::put_time` (PR #99075)
https://github.com/yichi170 edited https://github.com/llvm/llvm-project/pull/99075 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] replaced the usage of `asctime` with `std::put_time` (PR #99075)
yichi170 wrote: I don't have write access, so if the PR is ready, please help me land it! If there is anything that I need to do, please let me know! https://github.com/llvm/llvm-project/pull/99075 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] replaced the usage of `asctime` with `std::put_time` (PR #99075)
https://github.com/yichi170 updated https://github.com/llvm/llvm-project/pull/99075 >From 60a8a36851bcb0e73228ec4941f1906d0e5b7ba9 Mon Sep 17 00:00:00 2001 From: yichi170 Date: Wed, 17 Jul 2024 02:15:43 +0800 Subject: [PATCH] [clang] replaced the usage of `asctime` with `std::put_time` --- clang/lib/Lex/PPMacroExpansion.cpp | 18 +++--- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/clang/lib/Lex/PPMacroExpansion.cpp b/clang/lib/Lex/PPMacroExpansion.cpp index 3913ff08c2eb5..879f01e87806e 100644 --- a/clang/lib/Lex/PPMacroExpansion.cpp +++ b/clang/lib/Lex/PPMacroExpansion.cpp @@ -52,7 +52,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -1721,11 +1723,13 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) { Diag(Tok.getLocation(), diag::warn_pp_date_time); // MSVC, ICC, GCC, VisualAge C++ extension. The generated string should be // of the form "Ddd Mmm dd hh::mm::ss ", which is returned by asctime. -const char *Result; +std::string Result; +std::stringstream TmpStream; +TmpStream.imbue(std::locale("C")); if (getPreprocessorOpts().SourceDateEpoch) { time_t TT = *getPreprocessorOpts().SourceDateEpoch; std::tm *TM = std::gmtime(&TT); - Result = asctime(TM); + TmpStream << std::put_time(TM, "%a %b %e %T %Y"); } else { // Get the file that we are lexing out of. If we're currently lexing from // a macro, dig into the include stack. @@ -1735,13 +1739,13 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) { if (CurFile) { time_t TT = CurFile->getModificationTime(); struct tm *TM = localtime(&TT); -Result = asctime(TM); - } else { -Result = "??? ??? ?? ??:??:?? \n"; +TmpStream << std::put_time(TM, "%a %b %e %T %Y"); } } -// Surround the string with " and strip the trailing newline. -OS << '"' << StringRef(Result).drop_back() << '"'; +Result = TmpStream.str(); +if (Result.empty()) + Result = "??? ??? ?? ??:??:?? "; +OS << '"' << Result << '"'; Tok.setKind(tok::string_literal); } else if (II == Ident__FLT_EVAL_METHOD__) { // __FLT_EVAL_METHOD__ is set to the default value. ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] replaced the usage of `asctime` with `std::put_time` (PR #99075)
yichi170 wrote: Thanks for identifying the problem. @AaronBallman @rorth https://github.com/llvm/llvm-project/pull/99075 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] replaced the usage of `asctime` with `strftime` (PR #99075)
https://github.com/yichi170 created https://github.com/llvm/llvm-project/pull/99075 In `clang/lib/Lex/PPMacroExpansion.cpp`, replaced the usage of the obsolete `asctime` function with `strftime` for generating timestamp strings. This is my first time contributing to an open-source project. If there is anything that I can improve (such as code quality), please let me know! Fixes: https://github.com/llvm/llvm-project/issues/98724 >From 0fcbd4a46bb05ad9829fcf33a9829fd5f5269c71 Mon Sep 17 00:00:00 2001 From: yichi170 Date: Wed, 17 Jul 2024 02:15:43 +0800 Subject: [PATCH] [clang] replaced the usage of `asctime` with `strftime` --- clang/lib/Lex/PPMacroExpansion.cpp | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/clang/lib/Lex/PPMacroExpansion.cpp b/clang/lib/Lex/PPMacroExpansion.cpp index 3913ff08c2eb5..f7e657be87932 100644 --- a/clang/lib/Lex/PPMacroExpansion.cpp +++ b/clang/lib/Lex/PPMacroExpansion.cpp @@ -1722,10 +1722,12 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) { // MSVC, ICC, GCC, VisualAge C++ extension. The generated string should be // of the form "Ddd Mmm dd hh::mm::ss ", which is returned by asctime. const char *Result; +char TimeString[std::size("Ddd Mmm dd hh:mm:ss ")]; if (getPreprocessorOpts().SourceDateEpoch) { time_t TT = *getPreprocessorOpts().SourceDateEpoch; std::tm *TM = std::gmtime(&TT); - Result = asctime(TM); + strftime(TimeString, std::size(TimeString), "%c", TM); + Result = TimeString; } else { // Get the file that we are lexing out of. If we're currently lexing from // a macro, dig into the include stack. @@ -1735,13 +1737,13 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) { if (CurFile) { time_t TT = CurFile->getModificationTime(); struct tm *TM = localtime(&TT); -Result = asctime(TM); +strftime(TimeString, std::size(TimeString), "%c", TM); +Result = TimeString; } else { -Result = "??? ??? ?? ??:??:?? \n"; +Result = "??? ??? ?? ??:??:?? "; } } -// Surround the string with " and strip the trailing newline. -OS << '"' << StringRef(Result).drop_back() << '"'; +OS << '"' << Result << '"'; Tok.setKind(tok::string_literal); } else if (II == Ident__FLT_EVAL_METHOD__) { // __FLT_EVAL_METHOD__ is set to the default value. ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] replaced the usage of `asctime` with `strftime` (PR #99075)
@@ -1722,10 +1722,12 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) { // MSVC, ICC, GCC, VisualAge C++ extension. The generated string should be // of the form "Ddd Mmm dd hh::mm::ss ", which is returned by asctime. const char *Result; +char TimeString[std::size("Ddd Mmm dd hh:mm:ss ")]; if (getPreprocessorOpts().SourceDateEpoch) { time_t TT = *getPreprocessorOpts().SourceDateEpoch; std::tm *TM = std::gmtime(&TT); - Result = asctime(TM); + strftime(TimeString, std::size(TimeString), "%c", TM); yichi170 wrote: I see. Do you mean if the result of `strftime` is zero, `Result` have to be set as "???...?"? https://github.com/llvm/llvm-project/pull/99075 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] replaced the usage of `asctime` with `strftime` (PR #99075)
https://github.com/yichi170 edited https://github.com/llvm/llvm-project/pull/99075 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] replaced the usage of `asctime` with `strftime` (PR #99075)
https://github.com/yichi170 updated https://github.com/llvm/llvm-project/pull/99075 >From 0fcbd4a46bb05ad9829fcf33a9829fd5f5269c71 Mon Sep 17 00:00:00 2001 From: yichi170 Date: Wed, 17 Jul 2024 02:15:43 +0800 Subject: [PATCH 1/2] [clang] replaced the usage of `asctime` with `strftime` --- clang/lib/Lex/PPMacroExpansion.cpp | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/clang/lib/Lex/PPMacroExpansion.cpp b/clang/lib/Lex/PPMacroExpansion.cpp index 3913ff08c2eb5..f7e657be87932 100644 --- a/clang/lib/Lex/PPMacroExpansion.cpp +++ b/clang/lib/Lex/PPMacroExpansion.cpp @@ -1722,10 +1722,12 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) { // MSVC, ICC, GCC, VisualAge C++ extension. The generated string should be // of the form "Ddd Mmm dd hh::mm::ss ", which is returned by asctime. const char *Result; +char TimeString[std::size("Ddd Mmm dd hh:mm:ss ")]; if (getPreprocessorOpts().SourceDateEpoch) { time_t TT = *getPreprocessorOpts().SourceDateEpoch; std::tm *TM = std::gmtime(&TT); - Result = asctime(TM); + strftime(TimeString, std::size(TimeString), "%c", TM); + Result = TimeString; } else { // Get the file that we are lexing out of. If we're currently lexing from // a macro, dig into the include stack. @@ -1735,13 +1737,13 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) { if (CurFile) { time_t TT = CurFile->getModificationTime(); struct tm *TM = localtime(&TT); -Result = asctime(TM); +strftime(TimeString, std::size(TimeString), "%c", TM); +Result = TimeString; } else { -Result = "??? ??? ?? ??:??:?? \n"; +Result = "??? ??? ?? ??:??:?? "; } } -// Surround the string with " and strip the trailing newline. -OS << '"' << StringRef(Result).drop_back() << '"'; +OS << '"' << Result << '"'; Tok.setKind(tok::string_literal); } else if (II == Ident__FLT_EVAL_METHOD__) { // __FLT_EVAL_METHOD__ is set to the default value. >From 0352a494b6d0735ad83aa9189d33f2e50a1349d0 Mon Sep 17 00:00:00 2001 From: yichi170 Date: Wed, 17 Jul 2024 22:17:48 +0800 Subject: [PATCH 2/2] [clang] handling zero return of `strgtime` in `clang/lib/Lex/PPMacroExpansion.cpp` --- clang/lib/Lex/PPMacroExpansion.cpp | 14 ++ 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/clang/lib/Lex/PPMacroExpansion.cpp b/clang/lib/Lex/PPMacroExpansion.cpp index f7e657be87932..89697872901d7 100644 --- a/clang/lib/Lex/PPMacroExpansion.cpp +++ b/clang/lib/Lex/PPMacroExpansion.cpp @@ -1721,13 +1721,13 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) { Diag(Tok.getLocation(), diag::warn_pp_date_time); // MSVC, ICC, GCC, VisualAge C++ extension. The generated string should be // of the form "Ddd Mmm dd hh::mm::ss ", which is returned by asctime. -const char *Result; -char TimeString[std::size("Ddd Mmm dd hh:mm:ss ")]; +const char *Result = "??? ??? ?? ??:??:?? "; +char TimeString[64]; if (getPreprocessorOpts().SourceDateEpoch) { time_t TT = *getPreprocessorOpts().SourceDateEpoch; std::tm *TM = std::gmtime(&TT); - strftime(TimeString, std::size(TimeString), "%c", TM); - Result = TimeString; + if (strftime(TimeString, sizeof TimeString, "%a %b %e %T %Y", TM) != 0) +Result = TimeString; } else { // Get the file that we are lexing out of. If we're currently lexing from // a macro, dig into the include stack. @@ -1737,10 +1737,8 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) { if (CurFile) { time_t TT = CurFile->getModificationTime(); struct tm *TM = localtime(&TT); -strftime(TimeString, std::size(TimeString), "%c", TM); -Result = TimeString; - } else { -Result = "??? ??? ?? ??:??:?? "; +if (strftime(TimeString, sizeof TimeString, "%a %b %e %T %Y", TM) != 0) + Result = TimeString; } } OS << '"' << Result << '"'; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] replaced the usage of `asctime` with `strftime` (PR #99075)
yichi170 wrote: I submitted a commit that used the original way with error handling and fixed-sized TimeString. > (an alternative would be to use the C++ interface > https://compiler-explorer.com/z/WEqTW341b ) Since I'm not familiar with the issue related to locale, I'm not sure which method is better. If the C++ interface implementation mentioned by @cor3ntin is preferable, I can modify the current implementation. https://github.com/llvm/llvm-project/pull/99075 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] replaced the usage of `asctime` with `strftime` (PR #99075)
@@ -1721,11 +1721,13 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) { Diag(Tok.getLocation(), diag::warn_pp_date_time); // MSVC, ICC, GCC, VisualAge C++ extension. The generated string should be // of the form "Ddd Mmm dd hh::mm::ss ", which is returned by asctime. -const char *Result; +const char *Result = "??? ??? ?? ??:??:?? "; +char TimeString[64]; if (getPreprocessorOpts().SourceDateEpoch) { time_t TT = *getPreprocessorOpts().SourceDateEpoch; std::tm *TM = std::gmtime(&TT); - Result = asctime(TM); + if (strftime(TimeString, sizeof TimeString, "%a %b %e %T %Y", TM) != 0) yichi170 wrote: @AaronBallman I adapted your advice and here I use `%e` instead of `%d` since `asctime` returns the space-padded Day of the month. https://github.com/llvm/llvm-project/pull/99075 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] replaced the usage of `asctime` with `strftime` (PR #99075)
@@ -1721,11 +1721,13 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) { Diag(Tok.getLocation(), diag::warn_pp_date_time); // MSVC, ICC, GCC, VisualAge C++ extension. The generated string should be // of the form "Ddd Mmm dd hh::mm::ss ", which is returned by asctime. -const char *Result; +const char *Result = "??? ??? ?? ??:??:?? "; +char TimeString[64]; if (getPreprocessorOpts().SourceDateEpoch) { time_t TT = *getPreprocessorOpts().SourceDateEpoch; std::tm *TM = std::gmtime(&TT); - Result = asctime(TM); + if (strftime(TimeString, sizeof TimeString, "%a %b %e %T %Y", TM) != 0) yichi170 wrote: I found that using `%b` in `put_time` is also locale-dependent. Is it expected? https://github.com/llvm/llvm-project/pull/99075 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] replaced the usage of `asctime` with `strftime` (PR #99075)
https://github.com/yichi170 edited https://github.com/llvm/llvm-project/pull/99075 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits