On Mon, Nov 12, 2018 at 5:24 PM David Blaikie <dblai...@gmail.com> wrote: > > Thanks!
Thank you for the review feedback, that was a good catch. I've changed to an assert in r346714. ~Aaron > > On Mon, Nov 12, 2018 at 2:14 PM Aaron Ballman <aa...@aaronballman.com> wrote: >> >> On Mon, Nov 12, 2018 at 1:30 PM David Blaikie <dblai...@gmail.com> wrote: >> > >> > The previous code didn't have a conditional for Iter != End - was that a >> > bug? Should there be a test case for that bug? >> > >> > If that's not an actual change in behavior, could it be an assert instead >> > of a condition? >> >> I think an assertion would make the most sense. I can't devise a way >> where there wouldn't be at least a root path and a file name, so >> skipping over the root component should always be okay. I'll fix that >> up. >> >> ~Aaron >> >> > >> > On Tue, Nov 6, 2018 at 1:14 PM Aaron Ballman via cfe-commits >> > <cfe-commits@lists.llvm.org> wrote: >> >> >> >> Author: aaronballman >> >> Date: Tue Nov 6 13:12:44 2018 >> >> New Revision: 346266 >> >> >> >> URL: http://llvm.org/viewvc/llvm-project?rev=346266&view=rev >> >> Log: >> >> Don't use std::next() on an input iterator; NFC. >> >> >> >> Instead, advance the old-fashioned way, as std::next() cannot be used on >> >> an input iterator until C++17. >> >> >> >> Modified: >> >> cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp >> >> >> >> Modified: cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp >> >> URL: >> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp?rev=346266&r1=346265&r2=346266&view=diff >> >> ============================================================================== >> >> --- cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp (original) >> >> +++ cfe/trunk/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp Tue Nov 6 >> >> 13:12:44 2018 >> >> @@ -82,25 +82,27 @@ static std::string fileNameToURI(StringR >> >> Ret += Twine("/" + Root).str(); >> >> } >> >> >> >> - // Add the rest of the path components, encoding any reserved >> >> characters. >> >> - std::for_each(std::next(sys::path::begin(Filename)), >> >> sys::path::end(Filename), >> >> - [&Ret](StringRef Component) { >> >> - // For reasons unknown to me, we may get a backslash >> >> with >> >> - // Windows native paths for the initial backslash >> >> following >> >> - // the drive component, which we need to ignore as a >> >> URI path >> >> - // part. >> >> - if (Component == "\\") >> >> - return; >> >> + auto Iter = sys::path::begin(Filename), End = sys::path::end(Filename); >> >> + if (Iter != End) { >> >> + // Add the rest of the path components, encoding any reserved >> >> characters; >> >> + // we skip past the first path component, as it was handled it above. >> >> + std::for_each(++Iter, End, [&Ret](StringRef Component) { >> >> + // For reasons unknown to me, we may get a backslash with Windows >> >> native >> >> + // paths for the initial backslash following the drive component, >> >> which >> >> + // we need to ignore as a URI path part. >> >> + if (Component == "\\") >> >> + return; >> >> >> >> - // Add the separator between the previous path part >> >> and the >> >> - // one being currently processed. >> >> - Ret += "/"; >> >> + // Add the separator between the previous path part and the one >> >> being >> >> + // currently processed. >> >> + Ret += "/"; >> >> >> >> - // URI encode the part. >> >> - for (char C : Component) { >> >> - Ret += percentEncodeURICharacter(C); >> >> - } >> >> - }); >> >> + // URI encode the part. >> >> + for (char C : Component) { >> >> + Ret += percentEncodeURICharacter(C); >> >> + } >> >> + }); >> >> + } >> >> >> >> return Ret.str().str(); >> >> } >> >> >> >> >> >> _______________________________________________ >> >> cfe-commits mailing list >> >> cfe-commits@lists.llvm.org >> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
