Skip to content

Commit 8009bbe

Browse files
sdkrystiansdkrystian
authored
Reapply "[Clang][Sema] Diagnose class member access expressions naming non-existent members of the current instantiation prior to instantiation in the absence of dependent base classes (#84050)" (#90152)
Reapplies #84050, addressing a bug which cases a crash when an expression with the type of the current instantiation is used as the _postfix-expression_ in a class member access expression (arrow form).
1 parent f061a39 commit 8009bbe

File tree

30 files changed

+872
-278
lines changed

30 files changed

+872
-278
lines changed

‎clang-tools-extra/clangd/unittests/FindTargetTests.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -854,7 +854,7 @@ TEST_F(TargetDeclTest, DependentExprs) {
854854
}
855855
};
856856
)cpp";
857-
EXPECT_DECLS("CXXDependentScopeMemberExpr", "void foo()");
857+
EXPECT_DECLS("MemberExpr", "void foo()");
858858

859859
// Similar to above but base expression involves a function call.
860860
Code = R"cpp(
@@ -872,7 +872,7 @@ TEST_F(TargetDeclTest, DependentExprs) {
872872
}
873873
};
874874
)cpp";
875-
EXPECT_DECLS("CXXDependentScopeMemberExpr", "void foo()");
875+
EXPECT_DECLS("MemberExpr", "void foo()");
876876

877877
// Similar to above but uses a function pointer.
878878
Code = R"cpp(
@@ -891,7 +891,7 @@ TEST_F(TargetDeclTest, DependentExprs) {
891891
}
892892
};
893893
)cpp";
894-
EXPECT_DECLS("CXXDependentScopeMemberExpr", "void foo()");
894+
EXPECT_DECLS("MemberExpr", "void foo()");
895895

896896
// Base expression involves a member access into this.
897897
Code = R"cpp(
@@ -962,7 +962,7 @@ TEST_F(TargetDeclTest, DependentExprs) {
962962
void Foo() { this->[[find]](); }
963963
};
964964
)cpp";
965-
EXPECT_DECLS("CXXDependentScopeMemberExpr", "void find()");
965+
EXPECT_DECLS("MemberExpr", "void find()");
966966
}
967967

968968
TEST_F(TargetDeclTest, DependentTypes) {

‎clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -621,7 +621,7 @@ sizeof...($TemplateParameter[[Elements]]);
621621
struct $Class_def[[Foo]] {
622622
int $Field_decl[[Waldo]];
623623
void $Method_def[[bar]]() {
624-
$Class[[Foo]]().$Field_dependentName[[Waldo]];
624+
$Class[[Foo]]().$Field[[Waldo]];
625625
}
626626
template $Bracket[[<]]typename $TemplateParameter_def[[U]]$Bracket[[>]]
627627
void $Method_def[[bar1]]() {

‎clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/owning-memory.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,8 @@ struct HeapArray { // Ok, since destruc
309309

310310
HeapArray(HeapArray &&other) : _data(other._data), size(other.size) { // Ok
311311
other._data = nullptr; // Ok
312+
// CHECK-NOTES: [[@LINE-1]]:5: warning: expected assignment source to be of type 'gsl::owner<>'; got 'std::nullptr_t'
313+
// FIXME: This warning is emitted because an ImplicitCastExpr for the NullToPointer conversion isn't created for dependent types.
312314
other.size = 0;
313315
}
314316

‎clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-copy.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,8 @@ template <class T>
260260
struct Template {
261261
Template() = default;
262262
Template(const Template &Other) : Field(Other.Field) {}
263+
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
264+
// CHECK-FIXES: Template(const Template &Other) = default;
263265
Template &operator=(const Template &Other);
264266
void foo(const T &t);
265267
int Field;
@@ -269,8 +271,12 @@ Template<T> &Template<T>::operator=(const Template<T> &Other) {
269271
Field = Other.Field;
270272
return *this;
271273
}
274+
// CHECK-MESSAGES: :[[@LINE-4]]:27: warning: use '= default'
275+
// CHECK-FIXES: Template<T> &Template<T>::operator=(const Template<T> &Other) = default;
276+
272277
Template<int> T1;
273278

279+
274280
// Dependent types.
275281
template <class T>
276282
struct DT1 {
@@ -284,6 +290,9 @@ DT1<T> &DT1<T>::operator=(const DT1<T> &Other) {
284290
Field = Other.Field;
285291
return *this;
286292
}
293+
// CHECK-MESSAGES: :[[@LINE-4]]:17: warning: use '= default'
294+
// CHECK-FIXES: DT1<T> &DT1<T>::operator=(const DT1<T> &Other) = default;
295+
287296
DT1<int> Dt1;
288297

289298
template <class T>
@@ -303,6 +312,9 @@ DT2<T> &DT2<T>::operator=(const DT2<T> &Other) {
303312
struct T {
304313
typedef int TT;
305314
};
315+
// CHECK-MESSAGES: :[[@LINE-8]]:17: warning: use '= default'
316+
// CHECK-FIXES: DT2<T> &DT2<T>::operator=(const DT2<T> &Other) = default;
317+
306318
DT2<T> Dt2;
307319

308320
// Default arguments.

‎clang/docs/ReleaseNotes.rst

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -414,6 +414,18 @@ Improvements to Clang's diagnostics
414414

415415
- Clang now diagnoses requires expressions with explicit object parameters.
416416

417+
- Clang now looks up members of the current instantiation in the template definition context
418+
if the current instantiation has no dependent base classes.
419+
420+
.. code-block:: c++
421+
422+
template<typename T>
423+
struct A {
424+
int f() {
425+
return this->x; // error: no member named 'x' in 'A<T>'
426+
}
427+
};
428+
417429
Improvements to Clang's time-trace
418430
----------------------------------
419431

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -499,7 +499,9 @@ class LookupResult {
499499
/// Note that while no result was found in the current instantiation,
500500
/// there were dependent base classes that could not be searched.
501501
void setNotFoundInCurrentInstantiation() {
502-
assert(ResultKind == NotFound && Decls.empty());
502+
assert((ResultKind == NotFound ||
503+
ResultKind == NotFoundInCurrentInstantiation) &&
504+
Decls.empty());
503505
ResultKind = NotFoundInCurrentInstantiation;
504506
}
505507

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

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6978,12 +6978,6 @@ class Sema final : public SemaBase {
69786978
SourceLocation TemplateKWLoc,
69796979
UnqualifiedId &Member, Decl *ObjCImpDecl);
69806980

6981-
MemberExpr *BuildMemberExpr(
6982-
Expr *Base, bool IsArrow, SourceLocation OpLoc, const CXXScopeSpec *SS,
6983-
SourceLocation TemplateKWLoc, ValueDecl *Member, DeclAccessPair FoundDecl,
6984-
bool HadMultipleCandidates, const DeclarationNameInfo &MemberNameInfo,
6985-
QualType Ty, ExprValueKind VK, ExprObjectKind OK,
6986-
const TemplateArgumentListInfo *TemplateArgs = nullptr);
69876981
MemberExpr *
69886982
BuildMemberExpr(Expr *Base, bool IsArrow, SourceLocation OpLoc,
69896983
NestedNameSpecifierLoc NNS, SourceLocation TemplateKWLoc,
@@ -7472,7 +7466,7 @@ class Sema final : public SemaBase {
74727466
bool LookupQualifiedName(LookupResult &R, DeclContext *LookupCtx,
74737467
CXXScopeSpec &SS);
74747468
bool LookupParsedName(LookupResult &R, Scope *S, CXXScopeSpec *SS,
7475-
bool AllowBuiltinCreation = false,
7469+
QualType ObjectType, bool AllowBuiltinCreation = false,
74767470
bool EnteringContext = false);
74777471
ObjCProtocolDecl *LookupProtocol(
74787472
IdentifierInfo *II, SourceLocation IdLoc,
@@ -8881,11 +8875,13 @@ class Sema final : public SemaBase {
88818875
/// functions (but no function templates).
88828876
FoundFunctions,
88838877
};
8884-
bool LookupTemplateName(
8885-
LookupResult &R, Scope *S, CXXScopeSpec &SS, QualType ObjectType,
8886-
bool EnteringContext, bool &MemberOfUnknownSpecialization,
8887-
RequiredTemplateKind RequiredTemplate = SourceLocation(),
8888-
AssumedTemplateKind *ATK = nullptr, bool AllowTypoCorrection = true);
8878+
8879+
bool
8880+
LookupTemplateName(LookupResult &R, Scope *S, CXXScopeSpec &SS,
8881+
QualType ObjectType, bool EnteringContext,
8882+
RequiredTemplateKind RequiredTemplate = SourceLocation(),
8883+
AssumedTemplateKind *ATK = nullptr,
8884+
bool AllowTypoCorrection = true);
88898885

88908886
TemplateNameKind isTemplateName(Scope *S, CXXScopeSpec &SS,
88918887
bool hasTemplateKeyword,

‎clang/lib/AST/Expr.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ const Expr *Expr::skipRValueSubobjectAdjustments(
103103
}
104104
} else if (const auto *ME = dyn_cast<MemberExpr>(E)) {
105105
if (!ME->isArrow()) {
106-
assert(ME->getBase()->getType()->isRecordType());
106+
assert(ME->getBase()->getType()->getAsRecordDecl());
107107
if (const auto *Field = dyn_cast<FieldDecl>(ME->getMemberDecl())) {
108108
if (!Field->isBitField() && !Field->getType()->isReferenceType()) {
109109
E = ME->getBase();

‎clang/lib/Parse/ParseDecl.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3026,7 +3026,7 @@ bool Parser::ParseImplicitInt(DeclSpec &DS, CXXScopeSpec *SS,
30263026
<< TokenName << TagName << getLangOpts().CPlusPlus
30273027
<< FixItHint::CreateInsertion(Tok.getLocation(), FixitTagName);
30283028

3029-
if (Actions.LookupParsedName(R, getCurScope(), SS)) {
3029+
if (Actions.LookupName(R, getCurScope())) {
30303030
for (LookupResult::iterator I = R.begin(), IEnd = R.end();
30313031
I != IEnd; ++I)
30323032
Diag((*I)->getLocation(), diag::note_decl_hiding_tag_type)

‎clang/lib/Sema/HLSLExternalSemaSource.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,12 +126,15 @@ struct BuiltinTypeDeclBuilder {
126126

127127
static DeclRefExpr *lookupBuiltinFunction(ASTContext &AST, Sema &S,
128128
StringRef Name) {
129-
CXXScopeSpec SS;
130129
IdentifierInfo &II = AST.Idents.get(Name, tok::TokenKind::identifier);
131130
DeclarationNameInfo NameInfo =
132131
DeclarationNameInfo(DeclarationName(&II), SourceLocation());
133132
LookupResult R(S, NameInfo, Sema::LookupOrdinaryName);
134-
S.LookupParsedName(R, S.getCurScope(), &SS, false);
133+
// AllowBuiltinCreation is false but LookupDirect will create
134+
// the builtin when searching the global scope anyways...
135+
S.LookupName(R, S.getCurScope());
136+
// FIXME: If the builtin function was user-declared in global scope,
137+
// this assert *will* fail. Should this call LookupBuiltin instead?
135138
assert(R.isSingleResult() &&
136139
"Since this is a builtin it should always resolve!");
137140
auto *VD = cast<ValueDecl>(R.getFoundDecl());

‎clang/lib/Sema/SemaAttr.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -837,7 +837,7 @@ void Sema::ActOnPragmaUnused(const Token &IdTok, Scope *curScope,
837837

838838
IdentifierInfo *Name = IdTok.getIdentifierInfo();
839839
LookupResult Lookup(*this, Name, IdTok.getLocation(), LookupOrdinaryName);
840-
LookupParsedName(Lookup, curScope, nullptr, true);
840+
LookupName(Lookup, curScope, /*AllowBuiltinCreation=*/true);
841841

842842
if (Lookup.empty()) {
843843
Diag(PragmaLoc, diag::warn_pragma_unused_undeclared_var)

‎clang/lib/Sema/SemaDecl.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -832,7 +832,7 @@ static bool isTagTypeWithMissingTag(Sema &SemaRef, LookupResult &Result,
832832
IdentifierInfo *&Name,
833833
SourceLocation NameLoc) {
834834
LookupResult R(SemaRef, Name, NameLoc, Sema::LookupTagName);
835-
SemaRef.LookupParsedName(R, S, &SS);
835+
SemaRef.LookupParsedName(R, S, &SS, /*ObjectType=*/QualType());
836836
if (TagDecl *Tag = R.getAsSingle<TagDecl>()) {
837837
StringRef FixItTagName;
838838
switch (Tag->getTagKind()) {
@@ -869,7 +869,7 @@ static bool isTagTypeWithMissingTag(Sema &SemaRef, LookupResult &Result,
869869

870870
// Replace lookup results with just the tag decl.
871871
Result.clear(Sema::LookupTagName);
872-
SemaRef.LookupParsedName(Result, S, &SS);
872+
SemaRef.LookupParsedName(Result, S, &SS, /*ObjectType=*/QualType());
873873
return true;
874874
}
875875

@@ -896,7 +896,8 @@ Sema::NameClassification Sema::ClassifyName(Scope *S, CXXScopeSpec &SS,
896896
}
897897

898898
LookupResult Result(*this, Name, NameLoc, LookupOrdinaryName);
899-
LookupParsedName(Result, S, &SS, !CurMethod);
899+
LookupParsedName(Result, S, &SS, /*ObjectType=*/QualType(),
900+
/*AllowBuiltinCreation=*/!CurMethod);
900901

901902
if (SS.isInvalid())
902903
return NameClassification::Error();

‎clang/lib/Sema/SemaDeclCXX.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4519,7 +4519,7 @@ Sema::BuildMemInitializer(Decl *ConstructorD,
45194519
DS.getBeginLoc(), DS.getEllipsisLoc());
45204520
} else {
45214521
LookupResult R(*this, MemberOrBase, IdLoc, LookupOrdinaryName);
4522-
LookupParsedName(R, S, &SS);
4522+
LookupParsedName(R, S, &SS, /*ObjectType=*/QualType());
45234523

45244524
TypeDecl *TyD = R.getAsSingle<TypeDecl>();
45254525
if (!TyD) {
@@ -12270,7 +12270,7 @@ Decl *Sema::ActOnUsingDirective(Scope *S, SourceLocation UsingLoc,
1227012270

1227112271
// Lookup namespace name.
1227212272
LookupResult R(*this, NamespcName, IdentLoc, LookupNamespaceName);
12273-
LookupParsedName(R, S, &SS);
12273+
LookupParsedName(R, S, &SS, /*ObjectType=*/QualType());
1227412274
if (R.isAmbiguous())
1227512275
return nullptr;
1227612276

@@ -13729,7 +13729,7 @@ Decl *Sema::ActOnNamespaceAliasDef(Scope *S, SourceLocation NamespaceLoc,
1372913729

1373013730
// Lookup the namespace name.
1373113731
LookupResult R(*this, Ident, IdentLoc, LookupNamespaceName);
13732-
LookupParsedName(R, S, &SS);
13732+
LookupParsedName(R, S, &SS, /*ObjectType=*/QualType());
1373313733

1373413734
if (R.isAmbiguous())
1373513735
return nullptr;

‎clang/lib/Sema/SemaExpr.cpp

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -673,8 +673,9 @@ ExprResult Sema::DefaultLvalueConversion(Expr *E) {
673673
// expressions of certain types in C++.
674674
if (getLangOpts().CPlusPlus &&
675675
(E->getType() == Context.OverloadTy ||
676-
T->isDependentType() ||
677-
T->isRecordType()))
676+
// FIXME: This is a hack! We want the lvalue-to-rvalue conversion applied
677+
// to pointer types even if the pointee type is dependent.
678+
(T->isDependentType() && !T->isPointerType()) || T->isRecordType()))
678679
return E;
679680

680681
// The C standard is actually really unclear on this point, and
@@ -2751,8 +2752,8 @@ Sema::ActOnIdExpression(Scope *S, CXXScopeSpec &SS,
27512752
if (isBoundsAttrContext() && !getLangOpts().CPlusPlus && S->isClassScope()) {
27522753
// See if this is reference to a field of struct.
27532754
LookupResult R(*this, NameInfo, LookupMemberName);
2754-
// LookupParsedName handles a name lookup from within anonymous struct.
2755-
if (LookupParsedName(R, S, &SS)) {
2755+
// LookupName handles a name lookup from within anonymous struct.
2756+
if (LookupName(R, S)) {
27562757
if (auto *VD = dyn_cast<ValueDecl>(R.getFoundDecl())) {
27572758
QualType type = VD->getType().getNonReferenceType();
27582759
// This will eventually be translated into MemberExpr upon
@@ -2773,20 +2774,19 @@ Sema::ActOnIdExpression(Scope *S, CXXScopeSpec &SS,
27732774
// lookup to determine that it was a template name in the first place. If
27742775
// this becomes a performance hit, we can work harder to preserve those
27752776
// results until we get here but it's likely not worth it.
2776-
bool MemberOfUnknownSpecialization;
27772777
AssumedTemplateKind AssumedTemplate;
2778-
if (LookupTemplateName(R, S, SS, QualType(), /*EnteringContext=*/false,
2779-
MemberOfUnknownSpecialization, TemplateKWLoc,
2778+
if (LookupTemplateName(R, S, SS, /*ObjectType=*/QualType(),
2779+
/*EnteringContext=*/false, TemplateKWLoc,
27802780
&AssumedTemplate))
27812781
return ExprError();
27822782

2783-
if (MemberOfUnknownSpecialization ||
2784-
(R.getResultKind() == LookupResult::NotFoundInCurrentInstantiation))
2783+
if (R.wasNotFoundInCurrentInstantiation())
27852784
return ActOnDependentIdExpression(SS, TemplateKWLoc, NameInfo,
27862785
IsAddressOfOperand, TemplateArgs);
27872786
} else {
27882787
bool IvarLookupFollowUp = II && !SS.isSet() && getCurMethodDecl();
2789-
LookupParsedName(R, S, &SS, !IvarLookupFollowUp);
2788+
LookupParsedName(R, S, &SS, /*ObjectType=*/QualType(),
2789+
/*AllowBuiltinCreation=*/!IvarLookupFollowUp);
27902790

27912791
// If the result might be in a dependent base class, this is a dependent
27922792
// id-expression.

‎clang/lib/Sema/SemaExprCXX.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9157,7 +9157,7 @@ Sema::CheckMicrosoftIfExistsSymbol(Scope *S,
91579157
// Do the redeclaration lookup in the current scope.
91589158
LookupResult R(*this, TargetNameInfo, Sema::LookupAnyName,
91599159
RedeclarationKind::NotForRedeclaration);
9160-
LookupParsedName(R, S, &SS);
9160+
LookupParsedName(R, S, &SS, /*ObjectType=*/QualType());
91619161
R.suppressDiagnostics();
91629162

91639163
switch (R.getResultKind()) {

0 commit comments

Comments
 (0)