[PATCH] Pragma Loop constant expression support

[Richard Smith]

+    // Tokens following an error in an ill-formed constant expression will
+    // remain in the token stream and must be removed.
+    if (Tok.isNot(tok::eof)) {
+      Diag(Tok.getLocation(), diag::warn_pragma_extra_tokens_at_eol)
+          << PragmaLoopHintString(Info->PragmaName, Info->Option);
+      while (Tok.isNot(tok::eof))
+        ConsumeToken();

You need to use ConsumeAnyToken() here, or this will fail if it hits
certain kinds of special tokens.

+    if (Actions.CheckLoopHintExpr(R.get(), Toks[0].getLocation(),
+                                  /*AllowValueDependent=*/true))
+      return false;

If R.isInvalid(), you should bail out here rather than calling Sema with a
null pointer.

+  // Read constant expression.
+  // FIXME: This loop stops on first ')'. Should use
BalancedDelimiterTracker
+  // to track parens and parse expressions like ((i+1)*2).

BalancedDelimiterTracker doesn't help here; this is not what it's for. You
can just keep a simple count of the number of unclosed parens you've
encountered.

+  Token EoF;

The 'o' in EOF is traditionally capitalized.

+  if (!E) {
+    Diag(Loc, diag::err_pragma_loop_invalid_expression);
+    return true;
+  }

This seems wrong: if we couldn't parse an expression, we should have
already produced a diagnostic. But I think it's redundant given the change
I suggested above.

+  if (E->isValueDependent()) {
+    if (AllowValueDependent)
+      return false;
+    Diag(E->getExprLoc(), diag::err_pragma_loop_invalid_expression);
+    return true;
+  }

The AllowValueDependent flag here is unnecessary; instead, always allow
value-dependent things. You won't see value-dependent expressions here
unless you're still in a template or some other error has already occurred
(and been diagnosed).

+  if (!R.isUsable())
+    return true;

Please use isInvalid() here. isUsable suggests that you're anticipating
valid-but-null results.

+    std::string Value = "\'" + ValueAPS.toString(10) + "\'";

We usually put the ''s in the diagnostic, not in the argument.


On Mon, Aug 25, 2014 at 9:10 AM, Tyler Nowicki <tyler.nowicki at gmail.com>
wrote:

> Ping...
>
>
> On Thu, Aug 21, 2014 at 3:32 PM, Tyler Nowicki <tyler.nowicki at gmail.com>
> wrote:
>
>> Hi,
>>
>> I am back from my post-internship vacation. Aaron has previously given
>> this patch a LGTM, but he suggested Richard take a look as well.
>>
>> @Richard, could you please review this patch.
>>
>> In follow-up patches I will move CodeGen/pragma-loop.cpp to CodeGenCXX
>> and use BalancedDelimiterTracker to ensure parens are balanced when parsing
>> the constant expression. Please email me at tyler.nowicki at gmail.com (not
>> tnowicki at apple.com).
>>
>> Thanks,
>>
>> Tyler Nowicki
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140825/b0eea4de/attachment.html>