aaron.ballman added a comment. In D70553#1773046 <https://reviews.llvm.org/D70553#1773046>, @zturner wrote:
> In D70553#1757862 <https://reviews.llvm.org/D70553#1757862>, @aaron.ballman > wrote: > > > Can you add a test case for this functionality? > > > The reason there's no test case is because it seems like that wouldn't really > be any different than testing the functionality of `llvm::sys::fs`, which > should already be handled at the llvm/Support layer. There's not really any > interesting logic here. I can still do one if you think it's important but > that was my reasoning anyway. I have a slight preference for a test because it's a user-facing feature and we want to be alerted if we ever regress it. The unit tests in llvm/Support will catch if we break the underlying get/set permissions, but not the user feature (though it would be hard to break this user feature except through get/set permissions, lol). If it's hard to test for some reason, I don't insist on it. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70553/new/ https://reviews.llvm.org/D70553 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
