On Fri, Jul 31, 20= 15 at 2:07 PM, Richard Smith <richard@metafoo.co.uk> wro= te:

rsmith added inline comments.

=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
Comment at: lib/Parse/ParseOpenMP.cpp:171-177
@@ +170,9 @@
+
+=C2=A0 =C2=A0 // Append the current token at the end of the new token stre= am so that it
+=C2=A0 =C2=A0 // doesn't get lost.
+=C2=A0 =C2=A0 CachedPragmas.push_back(Tok);
+=C2=A0 =C2=A0 // Push back tokens for pragma.
+=C2=A0 =C2=A0 PP.EnterTokenStream(CachedPragmas.data(), CachedPragmas.size= (),
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 /*DisableMacroExpansion=3D*/true,
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 /*OwnsTokens=3D*/false);
+=C2=A0 =C2=A0 // 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 'decl= are simd' construct. Some of the clauses may have references to functio= n 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 introd= uce 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 p= refer 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 th= is 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 ide= ntifiers naming parameters, you don't need delay parsing and can instea= d 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 pa= rameter to appear before the declaration of that parameter, then we'll = need something like delayed parsing.)

http://reviews.llvm.org/D10599

_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-c= ommits