[PATCH] D136717: [clang] Move getenv call for SOURCE_DATE_EPOCH out of frontend NFC

[Ben Langmuir via Phabricator via cfe-commits]

benlangmuir added inline comments.


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:4324
+    if (Epoch.getAsInteger(10, V) || V > MaxTimestamp) {
       Diags.Report(diag::err_fe_invalid_source_date_epoch)
           << Epoch << MaxTimestamp;
----------------
benlangmuir wrote:
> MaskRay wrote:
> > Is it worth making `err_fe_invalid_source_date_epoch` a driver diagnostic? I think driver validation is more common.
> I wasn't sure if we could just move the validation to the driver or if we would end up duplicating it -- how bad is it to have a value that's too large? If it could cause UB or something I wouldn't want to remove the check from the frontend.
I kept the validation in the frontend for now but happy to iterate on this if you'd like.


================
Comment at: clang/test/Driver/SOURCE_DATE_EPOCH.c:2
+// RUN: %clang -E %s -### 2>&1 | FileCheck %s -check-prefix=NO_EPOCH
+// NO_EPOCH-NOT: source-date-epoch
+
----------------
MaskRay wrote:
> In case someone put the build directory under a directory with the string `-source-date-epoch`, even if it is highly unlikely.
Fair point, done!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136717