[PATCH] D19551: Handle Windows drive letters and ".." for --reproduce.

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 26 13:21:46 PDT 2016


On Tue, Apr 26, 2016 at 1:20 PM, Davide Italiano <dccitaliano at gmail.com>
wrote:

> davide added inline comments.
>
> ================
> Comment at: ELF/Driver.cpp:258
> @@ -243,6 +257,3 @@
>
> -static void dumpLinkerInvocation(ArrayRef<const char *> Args) {
> -  std::error_code EC = sys::fs::create_directories(Config->Reproduce,
> -    false /* IgnoreExisting */);
> -  if (EC) {
> -    error(EC, "--reproduce: can't create directory");
> +static void logCommandline(ArrayRef<const char *> Args) {
> +  if (std::error_code EC = sys::fs::create_directories(
> ----------------
> ruiu wrote:
> > davide wrote:
> > > I prefer dumpLinkerInvocation still.
> > To me this name is more plain and therefore good. It just logs command
> line -- I can tell it would obviously have no side effect (except writing a
> log somewhere) just by looking at the name. But dumpLinkerInvocation sounds
> something more complex.
> As you wish. Discussing this is probably not worth more time.
>
> ================
> Comment at: ELF/Driver.cpp:272
> @@ -257,3 +271,3 @@
>    OS << Args[0];
> -  for (unsigned I = 1, E = Args.size(); I < E; ++I)
> +  for (size_t I = 1, E = Args.size(); I < E; ++I)
>      OS << " " << Args[I];
> ----------------
> ruiu wrote:
> > davide wrote:
> > > We use unsigned everywhere else in the code base, try to be consistent
> or change all the other occurrences.
> > I don't think we do. I actually use size_t where it means the size of an
> array. This is also technically accurate (even though it is unrealistic
> that we have more than 2^32 arguments).
> I think these should be changed then:
>
> Writer.cpp:  for (unsigned I = 0; I < OutputSections.size(); ++I) {
> OutputSections.cpp:    for (unsigned I = 0, N = Offsets.size(); I != N;
> ++I) {
>
> I don't care which one we use just try to use the same everywhere. Feel
> free to change in another patch.
>

Sure, I'll probably do in a follow-up patch.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160426/1ecc37bf/attachment.html>


More information about the llvm-commits mailing list