[PATCH] D10599: [OPENMP 4.0] Initial support for '#pragma omp declare simd' directive.

[Richard Smith]

rsmith added inline comments.

================
Comment at: lib/Parse/ParseOpenMP.cpp:171-177
@@ +170,9 @@
+
+    // Append the current token at the end of the new token stream so that it
+    // doesn't get lost.
+    CachedPragmas.push_back(Tok);
+    // Push back tokens for pragma.
+    PP.EnterTokenStream(CachedPragmas.data(), CachedPragmas.size(),
+                        /*DisableMacroExpansion=*/true,
+                        /*OwnsTokens=*/false);
+    // Parse pragma itself.
----------------
ABataev wrote:
> rsmith wrote:
> > Why are you doing this delayed parsing? You still seem to have no tests that require it.
> We need it for future parsing of clauses associated with the 'declare simd' construct. Some of the clauses may have references to function arguments. That's why I'm using delayed parsing: at first we need to parse a function along with arguments and only then we'll parse all the pragmas along with their clauses and references to args.
> Of course currently it is not used, because this patch does not introduce parsing of clauses. It will be added later.
This code won't work for that; you've left the scope of the function parameters, so lookup for them will fail here. Generally-speaking, we don't like speculative / untested code to be committed, and would prefer to hold back on those changes until we can actually test them in some way.

Here's what I suggest: remove the delayed parsing code from here for this commit (parse the pragma first, then parse the nested declaration, then act on the result), and bring that code back once you introduce parsing for a clause that needs it, if indeed that's the right approach for those clauses. (As I mentioned before, if all they do is to provide a list of identifiers naming parameters, you don't need delay parsing and can instead store them as identifiers and look them up in Sema after the fact. On the other hand, if this pragma allows an arbitrary expression referencing a parameter to appear before the declaration of that parameter, then we'll need something like delayed parsing.)


http://reviews.llvm.org/D10599