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
pragma_tablegen-svn.patch
Description: Binary data
On Jun 6, 2014, at 3:56 PM, Aaron Ballman <[email protected]> wrote: > On Fri, Jun 6, 2014 at 5:13 PM, Tyler Nowicki <[email protected]> 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
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
