[cfe-commits] [LLVMdev] [cfe-dev] OpenMP support in CLANG: A proposal

[Dmitri Gribenko]

Hi Mahesha,

On Wed, Oct 24, 2012 at 3:05 PM, Mahesha HS <mahesha.llvm at gmail.com> wrote:
> In revised patch 3, I removed all global variables as you suggested,
> and implemented simple *const* string tables using simple binary
> search algorithm.

Is it possible to use std::lower_bound()?

+//===----------------------------------------------------------------------===//
+// OpenMP - An instance of this interface is defined to initiate
OpenMP parsing,
+// and to help Parser during subsequent parsing of OpenMP constrcuts.
+//===----------------------------------------------------------------------===//
+class OpenMP {

Please use standard Doxygen.

+  // brief The member object reference to the Preprocessor.
+  Preprocessor &PP;
+
+  // -brief Handler objects which handle different OpenMP pragma directive
+  // statements.

'\brief' and three slashes, please (here and below).

+  bool PragmaOmpUnknownWarning;

This is possibly bikeshedding, but I would call that PragmaOmpUnknownWarned.

+ANNOTATION(pragma_omp_parallel)
+ANNOTATION(pragma_omp_for)
+ANNOTATION(pragma_omp_sections)
...

Bikeshedding: is there some ordering criteria in this list (and other
code sequences using these constants)?  Seems like it has the same
order as in the OpenMP spec.  Please add comment about that so that
future maintainers will keep this order when adding support for new
directives.

+/// \brief getScheduleKind - Helper routine to get an enum kind associated

Please do not duplicate routine name in documentation comment.  Yes,
old code is written that way, but current coding guidelines recommend
against it.

Please do not duplicate the same comment in .h and .cpp.

Dmitri

-- 
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/