Skip to content

Commit 5a391d3

Browse files
zygoloidzygoloid
committed
Following up on PR48517, fix handling of template arguments that refer
to dependent declarations. Treat an id-expression that names a local variable in a templated function as being instantiation-dependent. This addresses a language defect whereby a reference to a dependent declaration can be formed without any construct being value-dependent. Fixing that through value-dependence turns out to be problematic, so instead this patch takes the approach (proposed on the core reflector) of allowing the use of pointers or references to (but not values of) dependent declarations inside value-dependent expressions, and instead treating template arguments as dependent if they evaluate to a constant involving such dependent declarations. This ends up affecting a bunch of OpenMP tests, due to OpenMP imprecisely handling instantiation-dependent constructs, bailing out early instead of processing dependent constructs to the extent possible when handling the template. Previously committed as 8c1f2d1, and reverted because a dependency commit was reverted.
1 parent fbb83f1 commit 5a391d3

32 files changed

+221
-158
lines changed

‎clang/include/clang/AST/Expr.h

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -578,12 +578,12 @@ class Expr : public ValueStmt {
578578
struct EvalStatus {
579579
/// Whether the evaluated expression has side effects.
580580
/// For example, (f() && 0) can be folded, but it still has side effects.
581-
bool HasSideEffects;
581+
bool HasSideEffects = false;
582582

583583
/// Whether the evaluation hit undefined behavior.
584584
/// For example, 1.0 / 0.0 can be folded to Inf, but has undefined behavior.
585585
/// Likewise, INT_MAX + 1 can be folded to INT_MIN, but has UB.
586-
bool HasUndefinedBehavior;
586+
bool HasUndefinedBehavior = false;
587587

588588
/// Diag - If this is non-null, it will be filled in with a stack of notes
589589
/// indicating why evaluation failed (or why it failed to produce a constant
@@ -592,10 +592,7 @@ class Expr : public ValueStmt {
592592
/// foldable. If the expression is foldable, but not a constant expression,
593593
/// the notes will describes why it isn't a constant expression. If the
594594
/// expression *is* a constant expression, no notes will be produced.
595-
SmallVectorImpl<PartialDiagnosticAt> *Diag;
596-
597-
EvalStatus()
598-
: HasSideEffects(false), HasUndefinedBehavior(false), Diag(nullptr) {}
595+
SmallVectorImpl<PartialDiagnosticAt> *Diag = nullptr;
599596

600597
// hasSideEffects - Return true if the evaluated expression has
601598
// side effects.
@@ -606,8 +603,11 @@ class Expr : public ValueStmt {
606603

607604
/// EvalResult is a struct with detailed info about an evaluated expression.
608605
struct EvalResult : EvalStatus {
609-
/// Val - This is the value the expression can be folded to.
606+
/// This is the value the expression can be folded to.
610607
APValue Val;
608+
/// Indicates whether Val contains a pointer or reference or pointer to
609+
/// member naming a templated entity, and thus the value is dependent.
610+
bool Dependent = false;
611611

612612
// isGlobalLValue - Return true if the evaluated lvalue expression
613613
// is global.

‎clang/include/clang/AST/TemplateBase.h

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,12 @@ class TemplateArgument {
252252
/// Whether this template argument is dependent on a template
253253
/// parameter such that its result can change from one instantiation to
254254
/// another.
255+
///
256+
/// It's not always meaningful to ask whether a template argument is
257+
/// dependent before it's been converted to match a template parameter;
258+
/// whether a non-type template argument is dependent depends on the
259+
/// corresponding parameter. For an unconverted template argument, this
260+
/// returns true if the argument *might* be dependent.
255261
bool isDependent() const;
256262

257263
/// Whether this template argument is dependent on a template
@@ -674,13 +680,6 @@ struct alignas(void *) ASTTemplateKWAndArgsInfo {
674680
void initializeFrom(SourceLocation TemplateKWLoc,
675681
const TemplateArgumentListInfo &List,
676682
TemplateArgumentLoc *OutArgArray);
677-
// FIXME: The parameter Deps is the result populated by this method, the
678-
// caller doesn't need it since it is populated by computeDependence. remove
679-
// it.
680-
void initializeFrom(SourceLocation TemplateKWLoc,
681-
const TemplateArgumentListInfo &List,
682-
TemplateArgumentLoc *OutArgArray,
683-
TemplateArgumentDependence &Deps);
684683
void initializeFrom(SourceLocation TemplateKWLoc);
685684

686685
void copyInto(const TemplateArgumentLoc *ArgArray,

‎clang/include/clang/Sema/Sema.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3465,7 +3465,8 @@ class Sema final {
34653465
llvm::APSInt &Value, CCEKind CCE);
34663466
ExprResult CheckConvertedConstantExpression(Expr *From, QualType T,
34673467
APValue &Value, CCEKind CCE,
3468-
NamedDecl *Dest = nullptr);
3468+
NamedDecl *Dest = nullptr,
3469+
bool *ValueDependent = nullptr);
34693470

34703471
/// Abstract base class used to perform a contextual implicit
34713472
/// conversion from an expression to any type passing a filter.

‎clang/lib/AST/ComputeDependence.cpp

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ ExprDependence clang::computeDependence(UnaryOperator *E,
6464
if (VD && VD->isTemplated()) {
6565
auto *VarD = dyn_cast<VarDecl>(VD);
6666
if (!VarD || !VarD->hasLocalStorage())
67-
Dep |= ExprDependence::Value;
67+
Dep |= ExprDependence::ValueInstantiation;
6868
}
6969
}
7070
}
@@ -443,12 +443,21 @@ ExprDependence clang::computeDependence(DeclRefExpr *E, const ASTContext &Ctx) {
443443
if (auto *FirstArg = E->getTemplateArgs()) {
444444
unsigned NumArgs = E->getNumTemplateArgs();
445445
for (auto *Arg = FirstArg, *End = FirstArg + NumArgs; Arg < End; ++Arg)
446-
Deps |= toExprDependence(Arg->getArgument().getDependence());
446+
Deps |= toExprDependence(Arg->getArgument().getDependence() &
447+
~TemplateArgumentDependence::Dependent);
447448
}
448449

449450
auto *Decl = E->getDecl();
451+
auto *Found = E->getFoundDecl();
450452
auto Type = E->getType();
451453

454+
// FIXME: For a ParmVarDecl referenced in a function signature, we don't know
455+
// its dependence yet!
456+
if (!isa<ParmVarDecl>(Decl)) {
457+
if (Decl->getDeclContext()->isDependentContext() ||
458+
(Found && Found->getDeclContext()->isDependentContext()))
459+
Deps |= ExprDependence::Instantiation;
460+
}
452461
if (Decl->isParameterPack())
453462
Deps |= ExprDependence::UnexpandedPack;
454463
Deps |= toExprDependence(Type->getDependence()) & ExprDependence::Error;

‎clang/lib/AST/Expr.cpp

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -416,12 +416,9 @@ DeclRefExpr::DeclRefExpr(const ASTContext &Ctx,
416416
RefersToEnclosingVariableOrCapture;
417417
DeclRefExprBits.NonOdrUseReason = NOUR;
418418
if (TemplateArgs) {
419-
auto Deps = TemplateArgumentDependence::None;
420419
getTrailingObjects<ASTTemplateKWAndArgsInfo>()->initializeFrom(
421-
TemplateKWLoc, *TemplateArgs, getTrailingObjects<TemplateArgumentLoc>(),
422-
Deps);
423-
assert(!(Deps & TemplateArgumentDependence::Dependent) &&
424-
"built a DeclRefExpr with dependent template args");
420+
TemplateKWLoc, *TemplateArgs,
421+
getTrailingObjects<TemplateArgumentLoc>());
425422
} else if (TemplateKWLoc.isValid()) {
426423
getTrailingObjects<ASTTemplateKWAndArgsInfo>()->initializeFrom(
427424
TemplateKWLoc);
@@ -1524,16 +1521,8 @@ MemberExpr *MemberExpr::Create(
15241521
MemberExpr *E = new (Mem) MemberExpr(Base, IsArrow, OperatorLoc, MemberDecl,
15251522
NameInfo, T, VK, OK, NOUR);
15261523

1527-
// FIXME: remove remaining dependence computation to computeDependence().
1528-
auto Deps = E->getDependence();
1524+
// FIXME: Move this into the constructor.
15291525
if (HasQualOrFound) {
1530-
// FIXME: Wrong. We should be looking at the member declaration we found.
1531-
if (QualifierLoc && QualifierLoc.getNestedNameSpecifier()->isDependent())
1532-
Deps |= ExprDependence::TypeValueInstantiation;
1533-
else if (QualifierLoc &&
1534-
QualifierLoc.getNestedNameSpecifier()->isInstantiationDependent())
1535-
Deps |= ExprDependence::Instantiation;
1536-
15371526
E->MemberExprBits.HasQualifierOrFoundDecl = true;
15381527

15391528
MemberExprNameQualifier *NQ =
@@ -1546,16 +1535,26 @@ MemberExpr *MemberExpr::Create(
15461535
TemplateArgs || TemplateKWLoc.isValid();
15471536

15481537
if (TemplateArgs) {
1549-
auto TemplateArgDeps = TemplateArgumentDependence::None;
15501538
E->getTrailingObjects<ASTTemplateKWAndArgsInfo>()->initializeFrom(
15511539
TemplateKWLoc, *TemplateArgs,
1552-
E->getTrailingObjects<TemplateArgumentLoc>(), TemplateArgDeps);
1553-
if (TemplateArgDeps & TemplateArgumentDependence::Instantiation)
1554-
Deps |= ExprDependence::Instantiation;
1540+
E->getTrailingObjects<TemplateArgumentLoc>());
15551541
} else if (TemplateKWLoc.isValid()) {
15561542
E->getTrailingObjects<ASTTemplateKWAndArgsInfo>()->initializeFrom(
15571543
TemplateKWLoc);
15581544
}
1545+
1546+
// FIXME: remove remaining dependence computation to computeDependence().
1547+
auto Deps = E->getDependence();
1548+
if (NestedNameSpecifier *Qual = E->getQualifier()) {
1549+
// FIXME: Wrong. We should be looking at the member declaration we found.
1550+
if (Qual->isDependent())
1551+
Deps |= ExprDependence::TypeValueInstantiation;
1552+
else if (Qual->isInstantiationDependent())
1553+
Deps |= ExprDependence::Instantiation;
1554+
}
1555+
if (TemplateSpecializationType::anyInstantiationDependentTemplateArguments(
1556+
E->template_arguments()))
1557+
Deps |= ExprDependence::Instantiation;
15591558
E->setDependence(Deps);
15601559

15611560
return E;

‎clang/lib/AST/ExprCXX.cpp

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -433,9 +433,8 @@ OverloadExpr::OverloadExpr(StmtClass SC, const ASTContext &Context,
433433
}
434434

435435
if (TemplateArgs) {
436-
auto Deps = TemplateArgumentDependence::None;
437436
getTrailingASTTemplateKWAndArgsInfo()->initializeFrom(
438-
TemplateKWLoc, *TemplateArgs, getTrailingTemplateArgumentLoc(), Deps);
437+
TemplateKWLoc, *TemplateArgs, getTrailingTemplateArgumentLoc());
439438
} else if (TemplateKWLoc.isValid()) {
440439
getTrailingASTTemplateKWAndArgsInfo()->initializeFrom(TemplateKWLoc);
441440
}
@@ -464,9 +463,8 @@ DependentScopeDeclRefExpr::DependentScopeDeclRefExpr(
464463
DependentScopeDeclRefExprBits.HasTemplateKWAndArgsInfo =
465464
(Args != nullptr) || TemplateKWLoc.isValid();
466465
if (Args) {
467-
auto Deps = TemplateArgumentDependence::None;
468466
getTrailingObjects<ASTTemplateKWAndArgsInfo>()->initializeFrom(
469-
TemplateKWLoc, *Args, getTrailingObjects<TemplateArgumentLoc>(), Deps);
467+
TemplateKWLoc, *Args, getTrailingObjects<TemplateArgumentLoc>());
470468
} else if (TemplateKWLoc.isValid()) {
471469
getTrailingObjects<ASTTemplateKWAndArgsInfo>()->initializeFrom(
472470
TemplateKWLoc);
@@ -1376,10 +1374,9 @@ CXXDependentScopeMemberExpr::CXXDependentScopeMemberExpr(
13761374
CXXDependentScopeMemberExprBits.OperatorLoc = OperatorLoc;
13771375

13781376
if (TemplateArgs) {
1379-
auto Deps = TemplateArgumentDependence::None;
13801377
getTrailingObjects<ASTTemplateKWAndArgsInfo>()->initializeFrom(
1381-
TemplateKWLoc, *TemplateArgs, getTrailingObjects<TemplateArgumentLoc>(),
1382-
Deps);
1378+
TemplateKWLoc, *TemplateArgs,
1379+
getTrailingObjects<TemplateArgumentLoc>());
13831380
} else if (TemplateKWLoc.isValid()) {
13841381
getTrailingObjects<ASTTemplateKWAndArgsInfo>()->initializeFrom(
13851382
TemplateKWLoc);

0 commit comments

Comments
 (0)