This revision was automatically updated to reflect the committed changes.
Closed by commit rG2c090162746a: [Frontend] Recognize environment variable
SOURCE_DATE_EPOCH (authored by MaskRay).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135045/new/
h
ychen accepted this revision as: ychen.
ychen added a comment.
This revision is now accepted and ready to land.
LGTM.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135045/new/
https://reviews.llvm.org/D135045
__
MaskRay updated this revision to Diff 466891.
MaskRay marked an inline comment as done.
MaskRay added a comment.
Don't test SOURCE_DATE_EPOCH=253402300799 on Windows (which only supports upto
32536850399)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.or
MaskRay marked 2 inline comments as done.
MaskRay added inline comments.
Comment at: clang/lib/Lex/PPMacroExpansion.cpp:1612
+ time_t TT = *getPreprocessorOpts().SourceDateEpoch;
+ std::tm *TM = std::gmtime(&TT);
Result = asctime(TM);
ychen wrote
ychen added inline comments.
Comment at: clang/lib/Lex/PPMacroExpansion.cpp:1612
+ time_t TT = *getPreprocessorOpts().SourceDateEpoch;
+ std::tm *TM = std::gmtime(&TT);
Result = asctime(TM);
MaskRay wrote:
> ychen wrote:
> > To be consistent with
MaskRay marked an inline comment as done.
MaskRay added inline comments.
Comment at: clang/lib/Lex/PPMacroExpansion.cpp:1612
+ time_t TT = *getPreprocessorOpts().SourceDateEpoch;
+ std::tm *TM = std::gmtime(&TT);
Result = asctime(TM);
ychen wrote
ychen added a comment.
Looks good to me except for one nit. And the test still fails on Windows for
some reason.
Comment at: clang/lib/Lex/PPMacroExpansion.cpp:1612
+ time_t TT = *getPreprocessorOpts().SourceDateEpoch;
+ std::tm *TM = std::gmtime(&TT);
Result
emaste added a comment.
> The time_t value may be negative
True, but we do not need to allow a negative `SOURCE_DATE_EPOCH` though. The
spec implies that it must be nonnegative, although doesn't say so explicitly.
https://reproducible-builds.org/specs/source-date-epoch/
Repository:
rG LLVM
MaskRay marked an inline comment as done.
MaskRay added inline comments.
Comment at: clang/lib/Lex/PPMacroExpansion.cpp:1092
+TT = *PP.getPreprocessorOpts().SourceDateEpoch;
+TM = std::gmtime(&TT);
+ } else {
ychen wrote:
> MaskRay wrote:
> > ychen wrote
MaskRay updated this revision to Diff 465649.
MaskRay marked an inline comment as done.
MaskRay edited the summary of this revision.
MaskRay added a comment.
Use `??? ?? ` or `??:??:??` is gmtime/localtime returns null
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https:
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:
> > MaskRay wrote:
> > > ychen wrote:
> > > > Why not use l
MaskRay marked 2 inline comments as done.
MaskRay added inline comments.
Comment at: clang/lib/Lex/PPMacroExpansion.cpp:1092
+TT = *PP.getPreprocessorOpts().SourceDateEpoch;
+TM = std::gmtime(&TT);
+ } else {
ychen wrote:
> MaskRay wrote:
> > ychen wrote
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 m
MaskRay marked an inline comment as done.
MaskRay added inline comments.
Comment at: clang/lib/Lex/PPMacroExpansion.cpp:1092
+TT = *PP.getPreprocessorOpts().SourceDateEpoch;
+TM = std::gmtime(&TT);
+ } else {
ychen wrote:
> Why not use localtime as the e
MaskRay updated this revision to Diff 465501.
MaskRay marked an inline comment as done.
MaskRay added a comment.
Make error message precise
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135045/new/
https://reviews.llvm.org/D135045
Files:
clang/d
ychen added inline comments.
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:4313
+// 253402300799 is the UNIX timestamp of -12-31T23:59:59Z.
+if (StringRef(Epoch).getAsInteger(10, V) || V > 253402300799)
+ Diags.Report(diag::err_fe_invalid_source_date_epoc
MaskRay updated this revision to Diff 464815.
MaskRay added a comment.
Use `%if clang-target-64-bits`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135045/new/
https://reviews.llvm.org/D135045
Files:
clang/docs/ReleaseNotes.rst
clang/include/c
ychen added a comment.
The Windows premerge failure looks relevant.
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@lis
MaskRay updated this revision to Diff 464581.
MaskRay edited the summary of this revision.
MaskRay added a comment.
Herald added a reviewer: ctetreau.
https://wiki.debian.org/ReproducibleBuilds/TimestampsFromCPPMacros says
> We missed out __TIMESTAMP__ by mistake, but we will probably send in ano
MaskRay created this revision.
MaskRay added reviewers: clang-language-wg, aaron.ballman, appcs, efriedma,
emaste.
Herald added a subscriber: StephenFan.
Herald added a project: All.
MaskRay requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
20 matches
Mail list logo
