ychen added inline comments.

================
Comment at: clang/lib/Lex/PPMacroExpansion.cpp:1092
+    TT = *PP.getPreprocessorOpts().SourceDateEpoch;
+    TM = std::gmtime(&TT);
+  } else {
----------------
MaskRay wrote:
> ychen wrote:
> > 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.
> Can you elaborate? With the check in Frontend I think a check here is not 
> necessary...
Sorry I was not clear. The time_t value is not portable so it is hard to 
diagnose in a target-independent way. For example, `253402300799` is too big of 
a value for windows that `gmtime` returns null (causes crash at `asctime` in 
the similar case below). We could diagnose when gmtime/localtime returns null, 
which is portable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135045

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to