[PATCH] Emit Clang version information into .comment section (MC's part of implementation) [PART 2B]

Rafael Ávila de Espíndola rafael.espindola at gmail.com
Wed Oct 2 11:04:50 PDT 2013


  This is missing a testcase.


================
Comment at: include/llvm/MC/MCStreamer.h:537
@@ -536,1 +536,3 @@
 
+  /// EmitIdent - Emit the "identifiers" directive.  This implements the
+  /// '.ident "version foo"' assembler directive.
----------------
Please don't duplicate the function name.


================
Comment at: include/llvm/MC/MCAsmInfo.h:276
@@ +275,3 @@
+    /// true for ELF targets.
+    bool HasIdentDirective;                  // Defaults to true.
+
----------------
Why not default to false, that way you only need to set it to true in one place (ELF).

================
Comment at: lib/MC/MCAsmStreamer.cpp:826
@@ -825,3 +826,2 @@
 
-
 void MCAsmStreamer::EmitFileDirective(StringRef Filename) {
----------------
unrelated change.


================
Comment at: lib/MC/MCMachOStreamer.cpp:89
@@ +88,3 @@
+  virtual void EmitIdent(StringRef IdentString) {
+    llvm_unreachable("macho doesn't support this directive");
+  }
----------------
do we get here if a user tries to use .ident in a .s in OS X? If so, a report_fatal_error is better.


http://llvm-reviews.chandlerc.com/D1812



More information about the llvm-commits mailing list