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

This revision was automatically updated to reflect the committed changes. Closed by commit rC343481: [Preprocessor] Fix a crash when handling non-alpha include header. (authored by hokein, committed by ). Changed prior to commit: https://reviews.llvm.org/D52721?vs=167737&id=167738#toc Reposito

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

hokein added a comment. I verified this patch is passed all clang tests (`ninja check-clang`). Repository: rC Clang https://reviews.llvm.org/D52721 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/list

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

hokein updated this revision to Diff 167737. hokein marked 3 inline comments as done. hokein added a comment. Address review comments. Repository: rC Clang https://reviews.llvm.org/D52721 Files: lib/Lex/PPDirectives.cpp test/Preprocessor/include-nonalpha-no-crash.c Index: test/Preproce

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

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 +

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

kristina accepted this revision. kristina added a comment. Yes I think the clangd crash will have to go in another diff, but this fixes the crash in clang, which is pretty good in itself. I don't build clangd at the moment and have no buildbot available so I can try to look into it later if you

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

sammccall added inline comments. Comment at: lib/Lex/PPDirectives.cpp:1898 + } + if (Filename.empty()) +return Filename; kristina wrote: > sammccall wrote: > > simplify the logic by merging with the while loop? (and drop the assert)

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

kristina added inline comments. Comment at: lib/Lex/PPDirectives.cpp:1898 + } + if (Filename.empty()) +return Filename; sammccall wrote: > simplify the logic by merging with the while loop? (and drop the assert) I thought the assert

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

kristina added a comment. Have you tested that build is successful and obviously the FileCheck test should pass now? What sort of behavior does this produce when you attempt to use an invalid path like that now when compiling with clang? Just a predecessor diagnostic? I'm updating software on

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

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: lib/Lex/PPDirectives.cpp:1895 +auto CorrectTypoFilename = [](llvm::StringRef Filename) { + while (!Filename.empty() && !isAlphanumeric(

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

hokein added inline comments. Comment at: lib/Lex/PPDirectives.cpp:1891 StringRef OriginalFilename = Filename; if (!File) { +while (!Filename.empty() && !isAlphanumeric(Filename.front())) { sammccall wrote: > everything in this block is guar

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

hokein updated this revision to Diff 167724. hokein marked an inline comment as done. hokein added a comment. Update the code. Repository: rC Clang https://reviews.llvm.org/D52721 Files: lib/Lex/PPDirectives.cpp test/Preprocessor/include-nonalpha-no-crash.c Index: test/Preprocessor/inc

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

kristina added inline comments. Comment at: lib/Lex/PPDirectives.cpp:1892 if (!File) { -while (!isAlphanumeric(Filename.front())) { +while (!Filename.empty() && !isAlphanumeric(Filename.front())) { Filename = Filename.drop_front(); --

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

kristina added inline comments. Comment at: lib/Lex/PPDirectives.cpp:1892 if (!File) { -while (!isAlphanumeric(Filename.front())) { +while (!Filename.empty() && !isAlphanumeric(Filename.front())) { Filename = Filename.drop_front(); --

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

kristina added a reviewer: kristina. kristina added a comment. Able to reproduce easily: clang-8: /SourceCache/llvm-trunk-8.0/include/llvm/ADT/StringRef.h:143: char llvm::StringRef::front() const: Assertion `!empty()' failed. Will try it with patches applied in a moment. Repository: rC Cl

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

sammccall added inline comments. Comment at: lib/Lex/PPDirectives.cpp:1891 StringRef OriginalFilename = Filename; if (!File) { +while (!Filename.empty() && !isAlphanumeric(Filename.front())) { everything in this block is guarded by !Filename

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

hokein created this revision. hokein added a reviewer: sammccall. Herald added a subscriber: jsji. This also fixes a crash of code completion on `#include "./^"`. Repository: rC Clang https://reviews.llvm.org/D52721 Files: lib/Lex/PPDirectives.cpp test/Preprocessor/include-nonalpha-no-cr