[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added inline comments.


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:4313
+    // 253402300799 is the UNIX timestamp of 9999-12-31T23:59:59Z.
+    if (StringRef(Epoch).getAsInteger(10, V) || V > 253402300799)
+      Diags.Report(diag::err_fe_invalid_source_date_epoch) << Epoch;
----------------
The time_t value may be negative and its value range is platform-dependent. It is better to be as transparent as possible. gmtime/localtime should be able to do the check by returning nullptr.

getAsInteger could handle V as time_t directly.


================
Comment at: clang/lib/Lex/PPMacroExpansion.cpp:1092
+    TT = *PP.getPreprocessorOpts().SourceDateEpoch;
+    TM = std::gmtime(&TT);
+  } else {
----------------
Why not use localtime as the else branch?

As mentioned above, diagnose null TM here or when parsing the user-specifed value. A small trade-off to make, up to you.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135045/new/

https://reviews.llvm.org/D135045