[PATCH] Implementation of #pragma (data|code|const|bss)_seg

Reid Kleckner rnk at google.com
Tue Apr 8 13:00:51 PDT 2014


  Looks good, feel free to commit with some tweaks.


================
Comment at: include/clang/Basic/DiagnosticParseKinds.td:775
@@ +774,3 @@
+def warn_pragma_expected_section_name : Warning<
+  "expected a string literal for the section name '#pragma %0' - ignored">,
+  InGroup<IgnoredPragmas>;
----------------
I think you omitted "in" before '#pragma'

================
Comment at: include/clang/Sema/Sema.h:7073
@@ +7072,3 @@
+  enum PragmaMSKind {
+    PSK_DataSeg,
+    PSK_BSSSeg,
----------------
I'm not sure what the S in PSK stands for.  Maybe PMK for PragmaMsKind?

================
Comment at: lib/Parse/ParsePragma.cpp:563
@@ +562,3 @@
+  Actions.ActOnPragmaMSSeg(PragmaLocation, Action, SlotLabel,
+    SegmentName, PragmaName);
+  return 0;
----------------
indentation

================
Comment at: include/clang/Sema/Sema.h:277
@@ -276,1 +276,3 @@
 
+  enum PragmaMsStackAction {
+    PSK_Reset,    // #pragma ()
----------------
Looks like these are used as flags.  Can you give the enumerators explicit hex values to make that clear?  I thought "Val & PSK_Push" was a bug.

================
Comment at: lib/Sema/SemaAttr.cpp:328-342
@@ -327,1 +327,17 @@
 
+template<typename Type> Type
+PopToName(std::vector<llvm::StringRef, Type>& Stack, llvm::StringRef Key) {
+  if (!ActionTarget.empty()) {
+    auto I = std::find_if(DataSegStack.rbegin(), DataSegStack.rend(),
+      [&](const std::pair<llvm::StringRef, StringLiteral *> &x) {
+      return x.first == ActionTarget;
+    });
+    CurrentDataSegment = I->second;
+    if (I != DataSegStack.rend())
+      DataSegStack.erase(std::prev(I.base()), DataSegStack.end());
+  } else if (!DataSegStack.empty()) {
+    CurrentDataSegment = DataSegStack.back().second;
+    DataSegStack.pop_back();
+  }
+}
+
----------------
This function looks like it has bugs, but it's dead and should be deleted.

================
Comment at: include/clang/Sema/Sema.h:7092
@@ +7091,3 @@
+    int SectionFlags;
+    SectionInfo() {}
+    SectionInfo(DeclaratorDecl *Decl,
----------------
This can be SectionInfo() = default so the default ctor is still trivial.

================
Comment at: lib/Sema/SemaDecl.cpp:7065
@@ +7064,3 @@
+    NewFD->addAttr(SectionAttr::CreateImplicit(Context,
+      SectionAttr::Declspec_allocate,
+      CodeSegStack.CurrentValue->getString(),
----------------
indentation

================
Comment at: lib/Sema/SemaDecl.cpp:8958
@@ +8957,3 @@
+      var->addAttr(SectionAttr::CreateImplicit(
+                   Context, SectionAttr::Declspec_allocate,
+                   Stack->CurrentValue->getString(),
----------------
indentation


http://reviews.llvm.org/D3065






More information about the cfe-commits mailing list