[PATCH] D80381: Fix debug line info when line markers are present inside macros.

Wed Jun 3 09:53:24 PDT 2020

compnerd added inline comments.

================
Comment at: llvm/lib/MC/MCParser/AsmParser.cpp:1705
+      return parseCppHashLineFilenameComment(IDLoc);
+    // Eat the line marker.
+    Lex();
----------------
leandrov wrote:
> thopre wrote:
> > Is there code inside parseCppHashLineFilenameComment that could be hoisted out in a separate function that could be used here? If not, maybe you could create a function anyway which would make the code here smaller and more obvious.
> > 
> > Might be nice having in the heading comment for that function the format of a line marker: # linenum filename flags. I don't know if it's customary to refer to documentation for a format when it's an online page, but if it is you could mention https://gcc.gnu.org/onlinedocs/cpp/Preprocessor-Output.html in that heading comment.
> `parseCppHashLineFilenameComment()` also calls `Lex()` 3 times but saves the content in the process. In here, we just want to skip it. I felt it wasn't worth to hoist out in a separate function (that in this case would simply discard token contents) and it's unlikely for this to repeat elsewhere... I would say to keep it like this (maybe) but let us wait for other reviewers.
I think that having a parameter to `parseCppHashLineFilenameComment` that indicates if the location should be preserved would be simple to add and avoid the duplication.  We could default to to `true` and avoid altering the existing callers.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80381/new/

https://reviews.llvm.org/D80381