faisalv created this revision.
This patch ensures that clang processes the expression-nodes that are generated
when disambiguating between types and expressions within template arguments, as
if they were truly constant-expressions. Currently, trunk correctly
disambiguates, and identifies the expression as an expression - and while it
annotates the token with the expression - it fails to complete the odr-use
processing (specifically, failing to trigger
Sema::UpdateMarkingForLValueToRValue as is done so for all Constant
Expressions, which removes it from being considered odr-used).
For e.g:
template<int> struct X { };
void f() {
const int N = 10;
X<N> x; // should be OK.
[] { return X<N>{}; }; // also OK - no capture.
}
See a related bug: https://bugs.llvm.org//show_bug.cgi?id=25627
The fix is as follows:
- Remove the EnteredConstantEvaluatedContext action from
ParseTemplateArgumentList (relying that ParseTemplateArgument will get it right)
- Add the EnteredConstantEvaluatedContext action just prior to undergoing the
disambiguating parse, and if the parse succeeds for an expression, make sure it
doesn't linger within MaybeODRUsedExprs by clearing it (while asserting that it
only contains the offending expression)
I need to add some tests... and fix one regression.
Does the approach look sound?
Thanks!
Repository:
rL LLVM
https://reviews.llvm.org/D31588
Files:
lib/Parse/ParseTemplate.cpp
Index: lib/Parse/ParseTemplate.cpp
===================================================================
--- lib/Parse/ParseTemplate.cpp
+++ lib/Parse/ParseTemplate.cpp
@@ -1198,42 +1198,73 @@
// expression is resolved to a type-id, regardless of the form of
// the corresponding template-parameter.
//
- // Therefore, we initially try to parse a type-id.
- if (isCXXTypeId(TypeIdAsTemplateArgument)) {
- SourceLocation Loc = Tok.getLocation();
- TypeResult TypeArg = ParseTypeName(/*Range=*/nullptr,
- Declarator::TemplateTypeArgContext);
- if (TypeArg.isInvalid())
- return ParsedTemplateArgument();
-
- return ParsedTemplateArgument(ParsedTemplateArgument::Type,
- TypeArg.get().getAsOpaquePtr(),
- Loc);
- }
-
+ // Therefore, we initially try to parse a type-id.
+ {
+ EnterExpressionEvaluationContext EnterConstantEvaluated(
+ Actions, Sema::ExpressionEvaluationContext::ConstantEvaluated);
+ if (isCXXTypeId(TypeIdAsTemplateArgument)) {
+ SourceLocation Loc = Tok.getLocation();
+ TypeResult TypeArg =
+ ParseTypeName(/*Range=*/nullptr, Declarator::TemplateTypeArgContext);
+ if (TypeArg.isInvalid())
+ return ParsedTemplateArgument();
+
+ return ParsedTemplateArgument(ParsedTemplateArgument::Type,
+ TypeArg.get().getAsOpaquePtr(), Loc);
+ } 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.
+ if (Actions.MaybeODRUseExprs.size()) {
+ assert(Actions.MaybeODRUseExprs.size() == 1 &&
+ "we should only need to parse one expression to determine an "
+ "error or typeid");
+ assert(Tok.getKind() == tok::annot_primary_expr &&
+ getExprAnnotation(Tok).get() ==
+ *Actions.MaybeODRUseExprs.begin() &&
+ "The expression (DeclRefExpr or MemExpr) stored within "
+ "MaybeODRUseExprs for later checking whether we perform an "
+ "lvalue-to-rvalue conversion before determining odr-use must be "
+ "the same as the annotated token during ambiguity resolution");
+ // We clear this state, since this lingering partially processed
+ // expression should not trigger an error now and we don't need to save
+ // 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
+ // appropriate constant-expression that matches the
+ // non-type-template-parameter occurs later in CheckTemplateArgument.
+ Actions.MaybeODRUseExprs.clear();
+ }
+ }
+ } // End ExpressionEvaluationContext
+
// Try to parse a template template argument.
{
TentativeParsingAction TPA(*this);
- ParsedTemplateArgument TemplateTemplateArgument
- = ParseTemplateTemplateArgument();
+ ParsedTemplateArgument TemplateTemplateArgument =
+ ParseTemplateTemplateArgument();
if (!TemplateTemplateArgument.isInvalid()) {
TPA.Commit();
return TemplateTemplateArgument;
}
-
+
// Revert this tentative parse to parse a non-type template argument.
TPA.Revert();
}
-
- // Parse a non-type template argument.
+
+ // Parse a non-type template argument.
SourceLocation Loc = Tok.getLocation();
ExprResult ExprArg = ParseConstantExpression(MaybeTypeCast);
if (ExprArg.isInvalid() || !ExprArg.get())
return ParsedTemplateArgument();
- return ParsedTemplateArgument(ParsedTemplateArgument::NonType,
- ExprArg.get(), Loc);
+ return ParsedTemplateArgument(ParsedTemplateArgument::NonType, ExprArg.get(),
+ Loc);
}
/// \brief Determine whether the current tokens can only be parsed as a
@@ -1274,16 +1305,16 @@
/// template-argument-list ',' template-argument
bool
Parser::ParseTemplateArgumentList(TemplateArgList &TemplateArgs) {
- // Template argument lists are constant-evaluation contexts.
- EnterExpressionEvaluationContext EvalContext(
- Actions, Sema::ExpressionEvaluationContext::ConstantEvaluated);
+
ColonProtectionRAIIObject ColonProtection(*this, false);
do {
ParsedTemplateArgument Arg = ParseTemplateArgument();
SourceLocation EllipsisLoc;
- if (TryConsumeToken(tok::ellipsis, EllipsisLoc))
+ if (TryConsumeToken(tok::ellipsis, EllipsisLoc)) {
+ // FIXME: Do we need need to enter a constant evaluation context here?
Arg = Actions.ActOnPackExpansion(Arg, EllipsisLoc);
+ }
if (Arg.isInvalid()) {
SkipUntil(tok::comma, tok::greater, StopAtSemi | StopBeforeMatch);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
