[PATCH] D32792: [LLVM][inline-asm] Altmacro string escape character '!'

[michael zuckerman via Phabricator via llvm-commits]

m_zuckerman marked an inline comment as done.
m_zuckerman added inline comments.


================
Comment at: lib/MC/MCParser/AsmParser.cpp:1207
   while ((*CharPtr != '>') && (*CharPtr != '\'') && (*CharPtr != '\n') &&
 	     (*CharPtr != '\r') && (*CharPtr != '\0')){
+	  if(*CharPtr == '!')
----------------
rengolin wrote:
> This could be:
> 
>     while ( ((...) && (*CharPtr != '\0')) || (*CharPtr == '!')) {
>       CharPtr++;
It will not work since you need double promotion. 
Consider the following code <5!>4> you want to jump into the '4' char. For that, you will need double promotion. 


================
Comment at: lib/MC/MCParser/AsmParser.cpp:1219
+/// \brief creating a string without the escape characters '!'. 
+void AsmParser::altMacroString(StringRef AltMacroStr,std::string &Res) {
+  for (int Pos = 0; Pos < AltMacroStr.size(); Pos++) {
----------------
rengolin wrote:
> This should really use `isAltmacroString` to make sure it really is a string, or it could have the same problems you detect on the other cases. But doing that, the same logic would need to be implemented twice.
> 
> A better approach would be if `isAltmacroString` was actually just `altMacroString` and, if the result is not empty, it "is an alt-macro string".
> 
> 
This function called after "isAltMacroString" validates the string D32701.
How the flow works: 
  #  One of the argument contains the '<' sign (parseMacroArguments). 
  #  This argument is then validated by the second "else if" in the "parseMacroArguments" function(D32701) 
  #  If the string begins with '<' and ends with '>' the function(parseMacroArguments) marks him as a string.
  #  Only token that begins with '<' and marked as a string can enter to this function. 
 
The problem with your approach is that we need to create a new string that not contain the '!'. This we can't do unless we will create a new global string in the begin of the main function "handleMacroEntry". Since the Stringref only contains the reference. We can't modify him and only mark him.

What you think, Do you prefer to create a new global string in the begin of the function and then use it? 


https://reviews.llvm.org/D32792