kousikk added a comment.

Thanks for the comment @jkorous.

> I think you could've just used CHECK-DAG to fix the tests. It *might* be a 
> bit more robust. Although just reordering checks seems perfectly fine too. 
> https://llvm.org/docs/CommandGuide/FileCheck.html#the-check-dag-directive. 
> Using std::set here sounds reasonable to me but I'd like @arphaman to be 
> aware of this.

Thanks for pointing me to it - I have a minor preference towards a `sorted` 
order of outputs vs `order in which we visit the files`. I think a sorted order 
is much more easy to reason about for a client. Having said that, @arphaman 
what do you think? If you and Jan both feel that we should maintain the current 
order, I'll stick to that and not change `Dependencies` to `set`.

> BTW should we change std::vector<std::string> Dependencies; in 
> DependencyCollector to set too?

I think this would change the output of `*.d` file? If so I'm a little hesitant 
to make that change because of the impact it would have, but once again I'll 
defer to your judgement on this (I'm very new to LLVM codebase).

> Also, I'm wondering why is the behavior different on Windows.

I confirmed that there are duplicate blacklist.txt files without changing 
`vector` to `set`. I'm setting up a Windows machine to try to figure out why 
that is the case. Will share what I find.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69090



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

Reply via email to