On Sat, Oct 20, 2012 at 4:38 PM, Mahesha HS <[email protected]> wrote: > On Fri, Oct 19, 2012 at 4:03 AM, Eli Friedman <[email protected]> wrote: >> On Thu, Oct 18, 2012 at 6:34 AM, Mahesha HS <[email protected]> wrote: >>> Sorry, in my previous mail, I had missed to attach changes to >>> "clang/include/clang/Basic/TokenKinds.def" in the patch 2. Please >>> refer to the patch (2) attached in *this* mail, instead of the one >>> sent in the previous mail. Patch 1 is fine. >> >> +//-------------------------------- >> +// OpenMP directives >> +//-------------------------------- >> +// \#pragam omp parallel ... >> +TOK(pragma_omp_parallel) >> +// \#pragam omp for ... >> +TOK(pragma_omp_for) >> >> Please use ANNOTATION(pragma_omp_parallel) etc. instead. > > I have taken care of it. Also, for patch 1 related test case, I > replaced -v by -###. > >> >> +/// PragmaOmpHandler - "\#pragma omp ...". >> +template<class T, tok::TokenKind TKind> >> +struct PragmaOmpHandler : public PragmaHandler { >> + PragmaOmpHandler() : PragmaHandler(T::Name) {} >> + virtual void HandlePragma(Preprocessor &PP, PragmaIntroducerKind >> Introducer, >> + Token &OmpTok) { >> + PP.HandlePragmaOmp(OmpTok, TKind); >> + } >> +}; >> >> Something like following would make your patch substantially shorter: >> >> struct PragmaOmpHandler : public PragmaHandler { >> tok::TokenKind TKind; >> PragmaOmpHandler(tok::TokenKind Kind, StringRef Name) >> : PragmaHandler(Name), TKind(Kind) {} >> virtual void HandlePragma(Preprocessor &PP, PragmaIntroducerKind >> Introducer, >> Token &OmpTok) { >> PP.HandlePragmaOmp(OmpTok, TKind); >> } >> }; > > My mis-understanding of PragmaNamespace::AddPragma() function, in > particular, mis-understanding of *assertion* statement within this > function mislead me. I was in an impression that, adding same class > more than once to same pragma namespace will result in assertion > failure. But, I had not carefully looked into it that it should be > fine as long as the associated names are different. Anyway, I have > taken care of this too. > >> >> >> Please add a test that the pragmas round-trip correctly through clang -E. > > This test case is getting an assertion failure. Looks like, all the > #pragmas which requires parsing, has to be registered (to > preprocessor) from libParse.a just like how #pragma unused, etc are > handled? When -E option is added, these pragmas are handled by > Preprocessor as unknown pragmas. > > Waiting for more clarification from you.
I have proceeded further, and followed exactly the similar approach as implemented in case of other pragmas like #pragma unused, etc. Attached is the revised patch (omp_pragma_registration_revised.patch). In case of any further comments, please let me know. Otherwise, we need to check-in both patch 1 (fopenmp_option_support.patch) and patch 2 (this one). -- mahesha > > -- > mahesha > >> >> -Eli > > > > -- > mahesha -- mahesha
omp_pragma_registration_revised.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
