[PATCH] Add PragmaAttr and Pragma Spelling to Tablegen

Tyler Nowicki tnowicki at apple.com
Tue Jun 10 17:01:36 PDT 2014


Hi Aaron,

Thanks for the review. I made the changes you suggested.

The previous patch was created by a git diff, perhaps that had something to do with it. Give this patch a try, its created by an svn diff.

Tyler

-------------- next part --------------
A non-text attachment was scrubbed...
Name: pragma_tablegen-svn.patch
Type: application/octet-stream
Size: 12994 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140610/f3d1bade/attachment.obj>
-------------- next part --------------

On Jun 6, 2014, at 3:56 PM, Aaron Ballman <aaron at aaronballman.com> wrote:

> On Fri, Jun 6, 2014 at 5:13 PM, Tyler Nowicki <tnowicki at apple.com> wrote:
>> Hi Aaron,
>> 
>> I noticed that 'pragma-loop-ast.cpp' wasn’t committed along with the first patch. I think this test is close to the print pretty test you are asking for. The pragma needs to be defined before a loop to pass parsing so that is the minimum amount of code to use the pragma. Also it uses -ast-print and verifies that the result matches the expected print pretty string.
>> 
>> Here is the patch rebased against master with the calf patch applied and I moved pragma-loop-ast.cpp to test/Sema/pragma-loop.cpp.
> 
> I'm not entirely certain of why, but I am still getting merge
> conflicts from TortoiseSVN when I attempt to merge this against a
> clean ToT. Specifically:
> 
> --- include/clang/Basic/Attr.td
> +++ include/clang/Basic/Attr.td
> @@ -192,0 +193,3 @@
> +class Pragma<string namespace, string name> : Spelling<name, "Pragma"> {
> +  string Namespace = namespace;
> +}
> 
> --- utils/TableGen/ClangAttrEmitter.cpp
> +++ utils/TableGen/ClangAttrEmitter.cpp
> @@ -1092,0 +1093,8 @@
> +    } else if (Variety == "Pragma") {
> +      Prefix = "#pragma ";
> +      Suffix = "\n";
> +      std::string Namespace = Spellings[I].nameSpace();
> +      if (!Namespace.empty()) {
> +        Spelling += Namespace;
> +        Spelling += " ";
> +      }
> @@ -1102,0 +1111,8 @@
> +    if (Variety == "Pragma") {
> +      OS << " \";\n";
> +      OS << "    printPrettyPragma(OS, Policy);\n";
> +      OS << "    break;\n";
> +      OS << "  }\n";
> +      continue;
> +    }
> +
> @@ -1811,0 +1829,3 @@
> +  OS << "case AttrSyntax::Pragma:\n";
> +  OS << "  return llvm::StringSwitch<bool>(Name)\n";
> +  GenerateHasAttrSpellingStringSwitch(Pragma, OS, "Pragma");
> @@ -2519,0 +2540,2 @@
> +        else if (Variety == "Pragma")
> +          Matches = &Pragma;
> @@ -2543,0 +2566,2 @@
> +  OS << "  } else if (AttributeList::AS_Pragma == Syntax) {\n";
> +  StringMatcher("Name", Pragma, OS).Emit();
> @@ -2740,0 +2768,2 @@
> +  OS << "\"\n\n";
> +  if (SupportedSpellings & Pragma) OS << "X";
> 
> What's neat is, there's no way for me to edit the conflicts. It flat
> out rejects these chunks.
> 
> That being said, the test could be a bit more specific by using
> CHECK-NEXT to ensure the pragmas are in the correct order and
> positioning relating to the loops. I gave an example of that in a
> different email thread regarding an XFAILed test, which I think should
> also be a part of this patch (since it related to the new
> functionality here). The test you include here is a good test to have
> just the same (it'll be redundant once we fix the XFAILed test, but we
> can delete one of the two of them at that point), but I think it
> should live in Misc more than Sema (since we're not checking the
> semantics engine works). So perhaps the two tests should be:
> test/Misc/ast-print-pragmas-xfail.cpp and
> test/Misc/ast-print-pragmas.cpp?
> 
> ~Aaron



More information about the cfe-commits mailing list