[PATCH] D49652: Apply -fdebug-prefix-map in reverse of command line order

2018-07-24 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. In https://reviews.llvm.org/D49652#1172498, @alxu wrote: > my general theory is that it's better to have tests that test everything at > once if possible, but I guess it's unlikely enough that the debug info path > will be correct in the IR and then not correct in

[PATCH] D49652: Apply -fdebug-prefix-map in reverse of command line order

2018-07-23 Thread Alex Xu (Hello71) via Phabricator via cfe-commits
alxu added a comment. my general theory is that it's better to have tests that test everything at once if possible, but I guess it's unlikely enough that the debug info path will be correct in the IR and then not correct in the generated file and that that won't be caught by LLVM tests.

[PATCH] D49652: Apply -fdebug-prefix-map in reverse of command line order

2018-07-23 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. In https://reviews.llvm.org/D49652#1172352, @alxu wrote: > I was just going to run llvm-dwarfdump. Well, that's one of those quirks I mentioned. This is a Clang change, and Clang tests should exercise as little of LLVM as possible. That means you don't generate

[PATCH] D49652: Apply -fdebug-prefix-map in reverse of command line order

2018-07-23 Thread Alex Xu (Hello71) via Phabricator via cfe-commits
alxu added a comment. I was just going to run llvm-dwarfdump. Repository: rC Clang https://reviews.llvm.org/D49652 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49652: Apply -fdebug-prefix-map in reverse of command line order

2018-07-23 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. Re. SmallVector versus std::vector, they are functionally similar but have different memory-allocation behaviors. SmallVector includes a vector of N elements (where N is the template parameter) so does no dynamic allocation until you have more than N elements; but

[PATCH] D49652: Apply -fdebug-prefix-map in reverse of command line order

2018-07-23 Thread Alex Xu (Hello71) via Phabricator via cfe-commits
alxu added a comment. Also, is it right to use SmallVector in one place and vector in the other? I just assumed based on the surrounding declarations, but I really have no idea. Repository: rC Clang https://reviews.llvm.org/D49652 ___

[PATCH] D49652: Apply -fdebug-prefix-map in reverse of command line order

2018-07-23 Thread Alex Xu (Hello71) via Phabricator via cfe-commits
alxu planned changes to this revision. alxu added a comment. The test syntax looked complicated, and I assumed that the documentation was terrible, like wine's, but it's actually pretty good, so I'll give it a go. I think the order is not really surprising though. It is run from right to left,

[PATCH] D49652: Apply -fdebug-prefix-map in reverse of command line order

2018-07-23 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. Also, please document the option in clang/docs/UsersManual.rst. It should have been added when the option was first added, but certainly with right-to-left behavior it needs a mention. Nearly all options work left-to-right, so even though it's the same as gcc, not

[PATCH] D49652: Apply -fdebug-prefix-map in reverse of command line order

2018-07-23 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a reviewer: probinson. probinson added a comment. Needs a test. Repository: rC Clang https://reviews.llvm.org/D49652 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D49652: Apply -fdebug-prefix-map in reverse of command line order

2018-07-22 Thread Alex Xu (Hello71) via Phabricator via cfe-commits
alxu added a comment. Oh, I forgot to mention, this is the way that gcc does it. Therefore, I expect that almost everybody either doesn't care about the order, or assumes the gcc behavior. Repository: rC Clang https://reviews.llvm.org/D49652

[PATCH] D49652: Apply -fdebug-prefix-map in reverse of command line order

2018-07-22 Thread Alex Xu (Hello71) via Phabricator via cfe-commits
alxu created this revision. Herald added a subscriber: cfe-commits. Before this patch, it is applied in order of increasing OLD path length. This is not a useful behavior. After this patch, it is applied based on the command line order from right to left, stopping on the first match.