kristina added a comment.
Please make sure it builds after you update the revision, I usually do it
myself but it'll take too long on my desktop and I accidentally broke the
hypervisor on my buildbot so can't do it this time.
================
Comment at: lib/Lex/PPDirectives.cpp:1898
+ }
+ if (Filename.empty())
+ return Filename;
----------------
sammccall wrote:
> kristina wrote:
> > sammccall wrote:
> > > simplify the logic by merging with the while loop? (and drop the assert)
> > I thought the assert was a good idea in case a similar issue popped up
> > again (somehow triggering this but in reverse, not sure if that makes
> > sense), it's a no-op in upstream release builds anyway.
> `while (!empty && !isAlpha(back)) { drop_back }`
> is just as safe, and easier to understand than
> ```
> // ... subtly establish that there is an alphanum in the string ...
> while (!isAlpha(back)) { drop_back; assert(!empty) }
> ```
Fair enough, that would be better. Don't think I have anything else to add, I
think this revision just requires addressing that and then it's good to go, but
see my comment on `clangd`.
Repository:
rC Clang
https://reviews.llvm.org/D52721
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
