[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