[PATCH] D31588: Fix PR25627: Certain constant local variables must be usable as template arguments (without being odr-used)

[Richard Smith via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: lib/Parse/ParseTemplate.cpp:1203-1204
+  {
+    EnterExpressionEvaluationContext EnterConstantEvaluated(
+        Actions, Sema::ExpressionEvaluationContext::ConstantEvaluated);
+    if (isCXXTypeId(TypeIdAsTemplateArgument)) {
----------------
Please add a comment here, something like:

isCXXTypeId might look up and annotate an identifier as an id-expression during disambiguation, so enter the appropriate context for a constant expression template argument before trying to disambiguate.


================
Comment at: lib/Parse/ParseTemplate.cpp:1214-1243
+    } else {
+      // If we disambiguated as an expression that we identified as potentially
+      // not being odr-used (consistent with a template argument context), and
+      // annotated our token as that expression, then remove it from the
+      // MaybeODRUsedExprs so that it doesn't trigger a false error, since it
+      // would otherwise have been removed when completing processing of a
+      // constant expression.
----------------
... we shouldn't need to do any of this: instead, keep your `ExprEvaluationContext` alive through the call to `ParseConstantExpression`, and tell it to not create its own context in this case.


================
Comment at: lib/Parse/ParseTemplate.cpp:1234-1237
+        // it for later since we can be certain that this expression would
+        // eventually have been removed during ActOnConstantExpression called
+        // from ParseConstantExpression when parsing the non-type template
+        // argument below.  Eitherway, the appropriate checking for an
----------------
This doesn't seem right. If the template parameter is of reference type, the named entity should be considered to be odr-used. And likewise just because we stopped disambiguation after finding an id-expression that names a non-type, that does not imply that the overall template argument is the entity named by that id-expression. (Eg, consider `X<F()>`, where `F` is a functor whose `operator()` returns `this` -- that template argument should be considered to odr-use `F`.) But...


Repository:
  rL LLVM

https://reviews.llvm.org/D31588