Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions clang-tools-extra/clangd/unittests/FindTargetTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -854,7 +854,7 @@ TEST_F(TargetDeclTest, DependentExprs) {
}
};
)cpp";
EXPECT_DECLS("CXXDependentScopeMemberExpr", "void foo()");
EXPECT_DECLS("MemberExpr", "void foo()");

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

// Similar to above but uses a function pointer.
Code = R"cpp(
Expand All @@ -891,7 +891,7 @@ TEST_F(TargetDeclTest, DependentExprs) {
}
};
)cpp";
EXPECT_DECLS("CXXDependentScopeMemberExpr", "void foo()");
EXPECT_DECLS("MemberExpr", "void foo()");

// Base expression involves a member access into this.
Code = R"cpp(
Expand Down Expand Up @@ -962,7 +962,7 @@ TEST_F(TargetDeclTest, DependentExprs) {
void Foo() { this->[[find]](); }
};
)cpp";
EXPECT_DECLS("CXXDependentScopeMemberExpr", "void find()");
EXPECT_DECLS("MemberExpr", "void find()");
}

TEST_F(TargetDeclTest, DependentTypes) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,7 @@ sizeof...($TemplateParameter[[Elements]]);
struct $Class_def[[Foo]] {
int $Field_decl[[Waldo]];
void $Method_def[[bar]]() {
$Class[[Foo]]().$Field_dependentName[[Waldo]];
$Class[[Foo]]().$Field[[Waldo]];
}
template $Bracket[[<]]typename $TemplateParameter_def[[U]]$Bracket[[>]]
void $Method_def[[bar1]]() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,8 @@ struct HeapArray { // Ok, since destruc

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,8 @@ template <class T>
struct Template {
Template() = default;
Template(const Template &Other) : Field(Other.Field) {}
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
// CHECK-FIXES: Template(const Template &Other) = default;
Template &operator=(const Template &Other);
void foo(const T &t);
int Field;
Expand All @@ -269,8 +271,12 @@ Template<T> &Template<T>::operator=(const Template<T> &Other) {
Field = Other.Field;
return *this;
}
// CHECK-MESSAGES: :[[@LINE-4]]:27: warning: use '= default'
// CHECK-FIXES: Template<T> &Template<T>::operator=(const Template<T> &Other) = default;

Template<int> T1;


// Dependent types.
template <class T>
struct DT1 {
Expand All @@ -284,6 +290,9 @@ DT1<T> &DT1<T>::operator=(const DT1<T> &Other) {
Field = Other.Field;
return *this;
}
// CHECK-MESSAGES: :[[@LINE-4]]:17: warning: use '= default'
// CHECK-FIXES: DT1<T> &DT1<T>::operator=(const DT1<T> &Other) = default;

DT1<int> Dt1;

template <class T>
Expand All @@ -303,6 +312,9 @@ DT2<T> &DT2<T>::operator=(const DT2<T> &Other) {
struct T {
typedef int TT;
};
// CHECK-MESSAGES: :[[@LINE-8]]:17: warning: use '= default'
// CHECK-FIXES: DT2<T> &DT2<T>::operator=(const DT2<T> &Other) = default;

DT2<T> Dt2;

// Default arguments.
Expand Down
12 changes: 12 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,18 @@ Improvements to Clang's diagnostics

- Clang now diagnoses requires expressions with explicit object parameters.

- Clang now looks up members of the current instantiation in the template definition context
if the current instantiation has no dependent base classes.

.. code-block:: c++

template<typename T>
struct A {
int f() {
return this->x; // error: no member named 'x' in 'A<T>'
}
};

Improvements to Clang's time-trace
----------------------------------

Expand Down
4 changes: 3 additions & 1 deletion clang/include/clang/Sema/Lookup.h
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,9 @@ class LookupResult {
/// Note that while no result was found in the current instantiation,
/// there were dependent base classes that could not be searched.
void setNotFoundInCurrentInstantiation() {
assert(ResultKind == NotFound && Decls.empty());
assert((ResultKind == NotFound ||
ResultKind == NotFoundInCurrentInstantiation) &&
Decls.empty());
ResultKind = NotFoundInCurrentInstantiation;
}

Expand Down
20 changes: 8 additions & 12 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -6978,12 +6978,6 @@ class Sema final : public SemaBase {
SourceLocation TemplateKWLoc,
UnqualifiedId &Member, Decl *ObjCImpDecl);

MemberExpr *BuildMemberExpr(
Expr *Base, bool IsArrow, SourceLocation OpLoc, const CXXScopeSpec *SS,
SourceLocation TemplateKWLoc, ValueDecl *Member, DeclAccessPair FoundDecl,
bool HadMultipleCandidates, const DeclarationNameInfo &MemberNameInfo,
QualType Ty, ExprValueKind VK, ExprObjectKind OK,
const TemplateArgumentListInfo *TemplateArgs = nullptr);
MemberExpr *
BuildMemberExpr(Expr *Base, bool IsArrow, SourceLocation OpLoc,
NestedNameSpecifierLoc NNS, SourceLocation TemplateKWLoc,
Expand Down Expand Up @@ -7472,7 +7466,7 @@ class Sema final : public SemaBase {
bool LookupQualifiedName(LookupResult &R, DeclContext *LookupCtx,
CXXScopeSpec &SS);
bool LookupParsedName(LookupResult &R, Scope *S, CXXScopeSpec *SS,
bool AllowBuiltinCreation = false,
QualType ObjectType, bool AllowBuiltinCreation = false,
bool EnteringContext = false);
ObjCProtocolDecl *LookupProtocol(
IdentifierInfo *II, SourceLocation IdLoc,
Expand Down Expand Up @@ -8881,11 +8875,13 @@ class Sema final : public SemaBase {
/// functions (but no function templates).
FoundFunctions,
};
bool LookupTemplateName(
LookupResult &R, Scope *S, CXXScopeSpec &SS, QualType ObjectType,
bool EnteringContext, bool &MemberOfUnknownSpecialization,
RequiredTemplateKind RequiredTemplate = SourceLocation(),
AssumedTemplateKind *ATK = nullptr, bool AllowTypoCorrection = true);

bool
LookupTemplateName(LookupResult &R, Scope *S, CXXScopeSpec &SS,
QualType ObjectType, bool EnteringContext,
RequiredTemplateKind RequiredTemplate = SourceLocation(),
AssumedTemplateKind *ATK = nullptr,
bool AllowTypoCorrection = true);

TemplateNameKind isTemplateName(Scope *S, CXXScopeSpec &SS,
bool hasTemplateKeyword,
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/AST/Expr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ const Expr *Expr::skipRValueSubobjectAdjustments(
}
} else if (const auto *ME = dyn_cast<MemberExpr>(E)) {
if (!ME->isArrow()) {
assert(ME->getBase()->getType()->isRecordType());
assert(ME->getBase()->getType()->getAsRecordDecl());
if (const auto *Field = dyn_cast<FieldDecl>(ME->getMemberDecl())) {
if (!Field->isBitField() && !Field->getType()->isReferenceType()) {
E = ME->getBase();
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Parse/ParseDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3026,7 +3026,7 @@ bool Parser::ParseImplicitInt(DeclSpec &DS, CXXScopeSpec *SS,
<< TokenName << TagName << getLangOpts().CPlusPlus
<< FixItHint::CreateInsertion(Tok.getLocation(), FixitTagName);

if (Actions.LookupParsedName(R, getCurScope(), SS)) {
if (Actions.LookupName(R, getCurScope())) {
for (LookupResult::iterator I = R.begin(), IEnd = R.end();
I != IEnd; ++I)
Diag((*I)->getLocation(), diag::note_decl_hiding_tag_type)
Expand Down
7 changes: 5 additions & 2 deletions clang/lib/Sema/HLSLExternalSemaSource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -126,12 +126,15 @@ struct BuiltinTypeDeclBuilder {

static DeclRefExpr *lookupBuiltinFunction(ASTContext &AST, Sema &S,
StringRef Name) {
CXXScopeSpec SS;
IdentifierInfo &II = AST.Idents.get(Name, tok::TokenKind::identifier);
DeclarationNameInfo NameInfo =
DeclarationNameInfo(DeclarationName(&II), SourceLocation());
LookupResult R(S, NameInfo, Sema::LookupOrdinaryName);
S.LookupParsedName(R, S.getCurScope(), &SS, false);
// AllowBuiltinCreation is false but LookupDirect will create
// the builtin when searching the global scope anyways...
S.LookupName(R, S.getCurScope());
// FIXME: If the builtin function was user-declared in global scope,
// this assert *will* fail. Should this call LookupBuiltin instead?
assert(R.isSingleResult() &&
"Since this is a builtin it should always resolve!");
auto *VD = cast<ValueDecl>(R.getFoundDecl());
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Sema/SemaAttr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -837,7 +837,7 @@ void Sema::ActOnPragmaUnused(const Token &IdTok, Scope *curScope,

IdentifierInfo *Name = IdTok.getIdentifierInfo();
LookupResult Lookup(*this, Name, IdTok.getLocation(), LookupOrdinaryName);
LookupParsedName(Lookup, curScope, nullptr, true);
LookupName(Lookup, curScope, /*AllowBuiltinCreation=*/true);

if (Lookup.empty()) {
Diag(PragmaLoc, diag::warn_pragma_unused_undeclared_var)
Expand Down
7 changes: 4 additions & 3 deletions clang/lib/Sema/SemaDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -832,7 +832,7 @@ static bool isTagTypeWithMissingTag(Sema &SemaRef, LookupResult &Result,
IdentifierInfo *&Name,
SourceLocation NameLoc) {
LookupResult R(SemaRef, Name, NameLoc, Sema::LookupTagName);
SemaRef.LookupParsedName(R, S, &SS);
SemaRef.LookupParsedName(R, S, &SS, /*ObjectType=*/QualType());
if (TagDecl *Tag = R.getAsSingle<TagDecl>()) {
StringRef FixItTagName;
switch (Tag->getTagKind()) {
Expand Down Expand Up @@ -869,7 +869,7 @@ static bool isTagTypeWithMissingTag(Sema &SemaRef, LookupResult &Result,

// Replace lookup results with just the tag decl.
Result.clear(Sema::LookupTagName);
SemaRef.LookupParsedName(Result, S, &SS);
SemaRef.LookupParsedName(Result, S, &SS, /*ObjectType=*/QualType());
return true;
}

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

LookupResult Result(*this, Name, NameLoc, LookupOrdinaryName);
LookupParsedName(Result, S, &SS, !CurMethod);
LookupParsedName(Result, S, &SS, /*ObjectType=*/QualType(),
/*AllowBuiltinCreation=*/!CurMethod);

if (SS.isInvalid())
return NameClassification::Error();
Expand Down
6 changes: 3 additions & 3 deletions clang/lib/Sema/SemaDeclCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4519,7 +4519,7 @@ Sema::BuildMemInitializer(Decl *ConstructorD,
DS.getBeginLoc(), DS.getEllipsisLoc());
} else {
LookupResult R(*this, MemberOrBase, IdLoc, LookupOrdinaryName);
LookupParsedName(R, S, &SS);
LookupParsedName(R, S, &SS, /*ObjectType=*/QualType());

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

// Lookup namespace name.
LookupResult R(*this, NamespcName, IdentLoc, LookupNamespaceName);
LookupParsedName(R, S, &SS);
LookupParsedName(R, S, &SS, /*ObjectType=*/QualType());
if (R.isAmbiguous())
return nullptr;

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

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

if (R.isAmbiguous())
return nullptr;
Expand Down
20 changes: 10 additions & 10 deletions clang/lib/Sema/SemaExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -673,8 +673,9 @@ ExprResult Sema::DefaultLvalueConversion(Expr *E) {
// expressions of certain types in C++.
if (getLangOpts().CPlusPlus &&
(E->getType() == Context.OverloadTy ||
T->isDependentType() ||
T->isRecordType()))
// FIXME: This is a hack! We want the lvalue-to-rvalue conversion applied
// to pointer types even if the pointee type is dependent.
(T->isDependentType() && !T->isPointerType()) || T->isRecordType()))
return E;

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

if (MemberOfUnknownSpecialization ||
(R.getResultKind() == LookupResult::NotFoundInCurrentInstantiation))
if (R.wasNotFoundInCurrentInstantiation())
return ActOnDependentIdExpression(SS, TemplateKWLoc, NameInfo,
IsAddressOfOperand, TemplateArgs);
} else {
bool IvarLookupFollowUp = II && !SS.isSet() && getCurMethodDecl();
LookupParsedName(R, S, &SS, !IvarLookupFollowUp);
LookupParsedName(R, S, &SS, /*ObjectType=*/QualType(),
/*AllowBuiltinCreation=*/!IvarLookupFollowUp);

// If the result might be in a dependent base class, this is a dependent
// id-expression.
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Sema/SemaExprCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9157,7 +9157,7 @@ Sema::CheckMicrosoftIfExistsSymbol(Scope *S,
// Do the redeclaration lookup in the current scope.
LookupResult R(*this, TargetNameInfo, Sema::LookupAnyName,
RedeclarationKind::NotForRedeclaration);
LookupParsedName(R, S, &SS);
LookupParsedName(R, S, &SS, /*ObjectType=*/QualType());
R.suppressDiagnostics();

switch (R.getResultKind()) {
Expand Down
Loading
Loading