[PATCH] D46511: [llvm-rc] Don't strictly require quotes around external file names

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added inline comments.


================
Comment at: tools/llvm-rc/ResourceScriptToken.cpp:290
   const char CurChar = Data[Pos];
-  return std::isalnum(CurChar) || CurChar == '_';
+  return std::isalnum(CurChar) || CurChar == '_' || CurChar == '.' || CurChar == '/';
 }
----------------
amccarth wrote:
> zturner wrote:
> > amccarth wrote:
> > > mstorsjo wrote:
> > > > amccarth wrote:
> > > > > What about backslashes?
> > > > > 
> > > > > ```
> > > > > 101 BITMAP "assets\foo.bmp"
> > > > > ```
> > > > > 
> > > > > 
> > > > I'm not entirely sure how that should be handled, since backslash also is an escape char (and maybe the escape behaviour is different outisde of quotes). I think I'd just ignore backslashes for now...
> > > My example was bad because I shouldn't have had the quotation marks.
> > > 
> > > ```
> > > 101 BITMAP assets\foo.bmp
> > > ```
> > > 
> > > Is backslash an escape character outside of strings?  The above line works in Microsoft's rc.exe, and I'm sure I've worked with code that specifies paths like that in resource scripts.
> > Might make sense to handle this in a separate patch that is specifically for correct handling of backslashes in resource scripts, since it sounds like there are potentially some complicated edge cases here.
> I'm OK with deferring it, as long as we see a path forward.  I hope this doesn't break the approach of looking to see if the next token is a string or identifier.
It turned out that backslashes work just fine as such with no extra effort by adding it to the list of allowed chars in that token.

However, a path like `dir\bitmap.bmp` probably doesn't work when testing on a unix host, so I'm omitting a test for that part.


Repository:
  rL LLVM

https://reviews.llvm.org/D46511