[PATCH] D52721: [Preprocessor] Fix a crash when handling non-alpha include header.

[Kristina Brooks via Phabricator via cfe-commits]

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