[PATCH] D20020: Provide support for preserving assembly comments

[Nirav Dave via llvm-commits]

niravd added inline comments.

================
Comment at: include/llvm/MC/MCStreamer.h:281
@@ +280,3 @@
+  /// \brief emit explicit comments
+  virtual void outputExplicitComments();
+
----------------
rnk wrote:
> This is only called from MCAsmStreamer. Should we sink it down there and devirtualize it, or do we think there will be clients other than EmitEOL?
I'm inclined to keep it here with the other outputting functions. That said, by that argument it probably should be renamed emitExplicitComments.

================
Comment at: include/llvm/Target/TargetOptions.h:191
@@ +190,3 @@
+    /// Disable the integrated assembler.
+    unsigned PreserveAsmComments : 1;
+
----------------
Make sense. Changed.

================
Comment at: include/llvm/Target/TargetOptions.h:191
@@ +190,3 @@
+    /// Disable the integrated assembler.
+    unsigned PreserveAsmComments : 1;
+
----------------
niravd wrote:
> Make sense. Changed.
Agreed

================
Comment at: lib/MC/MCAsmInfo.cpp:110
@@ -109,2 +109,3 @@
   UseIntegratedAssembler = false;
+  PreserveAsmComments = true;
 
----------------
rnk wrote:
> Why default to true?
Since the integrated assembler goes from preprocessor assembly to preprocessed assembly with some nominal cleanup, it seems like if we're assuming comments may have semantic value the default should be preservation for people who haven't reasoned about it explicitly. 



http://reviews.llvm.org/D20020