Skip to content

Commit c9a70b4

Browse files
committed
[clang] Fix conflicting declaration error with using_if_exists
This fixes an issue with using_if_exists where we would hit `conflicts with target of using declaration already in scope` with a using_if_exists attribute referring to a declaration which did not exist. That is, if we have `using ::bar __attribute__((using_if_exists))` but `bar` is not in the global namespace, then nothing should actually be declared here. This PR contains the following changes: 1. Ensure we only diagnose this error if the target decl and [Non]Tag decl can be substitutes for each other. 2. Prevent LookupResult from considering UnresolvedUsingIfExistsDecls in the event of ambiguous results. 3. Update tests. This includes the minimal repo for a regression test, and changes to existing tests which also seem to exhibit this bug. Fixes #85335
1 parent ae2b303 commit c9a70b4

File tree

3 files changed

+78
-32
lines changed

3 files changed

+78
-32
lines changed

clang/lib/Sema/SemaDeclCXX.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include "clang/AST/EvaluatedExprVisitor.h"
2424
#include "clang/AST/Expr.h"
2525
#include "clang/AST/ExprCXX.h"
26+
#include "clang/AST/GlobalDecl.h"
2627
#include "clang/AST/RecordLayout.h"
2728
#include "clang/AST/StmtVisitor.h"
2829
#include "clang/AST/TypeLoc.h"
@@ -12832,6 +12833,15 @@ bool Sema::CheckUsingShadowDecl(BaseUsingDecl *BUD, NamedDecl *Orig,
1283212833
(isa_and_nonnull<UnresolvedUsingIfExistsDecl>(NonTag))) {
1283312834
if (!NonTag && !Tag)
1283412835
return false;
12836+
12837+
// Only check report the error if this using_if_exists decl can be a
12838+
// substitute for the original decl. LookupResult will find things with
12839+
// the same name but we also want to take into account namespaces and
12840+
// other scopes. GlobalDecl helps take care of that.
12841+
NamedDecl *UsedTag = NonTag ? NonTag : Tag;
12842+
if (GlobalDecl(Target) != GlobalDecl(UsedTag))
12843+
return false;
12844+
1283512845
Diag(BUD->getLocation(), diag::err_using_decl_conflict);
1283612846
Diag(Target->getLocation(), diag::note_using_decl_target);
1283712847
Diag((NonTag ? NonTag : Tag)->getLocation(),

clang/lib/Sema/SemaLookup.cpp

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -521,11 +521,15 @@ void LookupResult::resolveKind() {
521521

522522
llvm::SmallVector<const NamedDecl *, 4> EquivalentNonFunctions;
523523
llvm::BitVector RemovedDecls(N);
524+
llvm::BitVector UnresolvedUsingDecls(N);
524525

525526
for (unsigned I = 0; I < N; I++) {
526527
const NamedDecl *D = Decls[I]->getUnderlyingDecl();
527528
D = cast<NamedDecl>(D->getCanonicalDecl());
528529

530+
if (isa<UnresolvedUsingIfExistsDecl>(D))
531+
UnresolvedUsingDecls.set(I);
532+
529533
// Ignore an invalid declaration unless it's the only one left.
530534
// Also ignore HLSLBufferDecl which not have name conflict with other Decls.
531535
if ((D->isInvalidDecl() || isa<HLSLBufferDecl>(D)) &&
@@ -633,17 +637,24 @@ void LookupResult::resolveKind() {
633637
getSema().diagnoseEquivalentInternalLinkageDeclarations(
634638
getNameLoc(), HasNonFunction, EquivalentNonFunctions);
635639

640+
if ((HasNonFunction && (HasFunction || HasUnresolved)) ||
641+
(HideTags && HasTag && (HasFunction || HasNonFunction || HasUnresolved)))
642+
Ambiguous = true;
643+
644+
if (Ambiguous && UnresolvedUsingDecls.count()) {
645+
// If we would have an ambiguous reference but any of them are
646+
// using_if_exist decls, ignore them since they are unresolved.
647+
RemovedDecls |= UnresolvedUsingDecls;
648+
Ambiguous = false;
649+
}
650+
636651
// Remove decls by replacing them with decls from the end (which
637652
// means that we need to iterate from the end) and then truncating
638653
// to the new size.
639654
for (int I = RemovedDecls.find_last(); I >= 0; I = RemovedDecls.find_prev(I))
640655
Decls[I] = Decls[--N];
641656
Decls.truncate(N);
642657

643-
if ((HasNonFunction && (HasFunction || HasUnresolved)) ||
644-
(HideTags && HasTag && (HasFunction || HasNonFunction || HasUnresolved)))
645-
Ambiguous = true;
646-
647658
if (Ambiguous && ReferenceToPlaceHolderVariable)
648659
setAmbiguous(LookupAmbiguityKind::AmbiguousReferenceToPlaceholderVariable);
649660
else if (Ambiguous)

clang/test/SemaCXX/using-if-exists.cpp

Lines changed: 53 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -22,28 +22,28 @@ using NS::x UIE;
2222
namespace NS1 {}
2323
namespace NS2 {}
2424
namespace NS3 {
25-
int A(); // expected-note{{target of using declaration}}
26-
struct B {}; // expected-note{{target of using declaration}}
27-
int C(); // expected-note{{conflicting declaration}}
28-
struct D {}; // expected-note{{conflicting declaration}}
25+
int A();
26+
struct B {};
27+
int C();
28+
struct D {};
2929
} // namespace NS3
3030

31-
using NS1::A UIE;
32-
using NS2::A UIE; // expected-note{{using declaration annotated with 'using_if_exists' here}} expected-note{{conflicting declaration}}
33-
using NS3::A UIE; // expected-error{{target of using declaration conflicts with declaration already in scope}}
34-
int i = A(); // expected-error{{reference to unresolved using declaration}}
31+
using NS1::A UIE; // OK since this declaration shouldn't exist since `A` is not in `NS1`
32+
using NS2::A UIE; // OK since this declaration shouldn't exist since `A` is not in `NS2`
33+
using NS3::A UIE; // OK since prior UIEs of `A` shouldn't have declare anything since they don't exist
34+
int i = A(); // OK since `A` resolved to the single UIE in the previous line
3535

36-
using NS1::B UIE;
37-
using NS2::B UIE; // expected-note{{conflicting declaration}} expected-note{{using declaration annotated with 'using_if_exists' here}}
38-
using NS3::B UIE; // expected-error{{target of using declaration conflicts with declaration already in scope}}
39-
B myB; // expected-error{{reference to unresolved using declaration}}
36+
using NS1::B UIE; // OK since this declaration shouldn't exist since `B` is not in `NS1`
37+
using NS2::B UIE; // OK since this declaration shouldn't exist since `B` is not in `NS2
38+
using NS3::B UIE; // OK since prior UIEs of `B` shouldn't have declare anything since they don't exist
39+
B myB; // OK since `B` resolved to the single UIE in the previous lin
4040

4141
using NS3::C UIE;
42-
using NS2::C UIE; // expected-error{{target of using declaration conflicts with declaration already in scope}} expected-note{{target of using declaration}}
42+
using NS2::C UIE; // OK since NS2::C doesn't exist
4343
int j = C();
4444

4545
using NS3::D UIE;
46-
using NS2::D UIE; // expected-error{{target of using declaration conflicts with declaration already in scope}} expected-note{{target of using declaration}}
46+
using NS2::D UIE; // OK since NS2::D doesn't exist
4747
D myD;
4848
} // namespace test_redecl
4949

@@ -113,29 +113,36 @@ struct NonDep : BaseEmpty {
113113
namespace test_using_pack {
114114
template <class... Ts>
115115
struct S : Ts... {
116-
using typename Ts::x... UIE; // expected-error 2 {{target of using declaration conflicts with declaration already in scope}} expected-note{{conflicting declaration}} expected-note{{target of using declaration}}
116+
// We don't expect any errors with conflicting targets for variables `a`, `b`, `c`,
117+
// and `d` below this. For `a`, `x` will not be declared because neither E1 nor E2
118+
// defines it. For `b`, `x` is the same type so there won't be any conflicts. For
119+
// `c` and `d`, only one of the template parameters has a class that defines it,
120+
// so there's no conflict.
121+
using typename Ts::x... UIE;
117122
};
118123

119124
struct E1 {};
120125
struct E2 {};
121126
S<E1, E2> a;
122127

123128
struct F1 {
124-
typedef int x; // expected-note 2 {{conflicting declaration}}
129+
typedef int x;
125130
};
126131
struct F2 {
127-
typedef int x; // expected-note 2 {{target of using declaration}}
132+
typedef int x;
128133
};
129134
S<F1, F2> b;
130135

131-
S<E1, F2> c; // expected-note{{in instantiation of template class}}
132-
S<F1, E2> d; // expected-note{{in instantiation of template class}}
136+
S<E1, F2> c;
137+
S<F1, E2> d;
133138

134139
template <class... Ts>
135140
struct S2 : Ts... {
136-
using typename Ts::x... UIE; // expected-error 2 {{target of using declaration conflicts with declaration already in scope}} expected-note 3 {{using declaration annotated with 'using_if_exists' here}} expected-note{{conflicting declaration}} expected-note{{target of using declaration}}
141+
// OK for the same reasons listed in `struct S` above. We don't expect any conflicts w.r.t
142+
// redefinitions of `x` but we still expect errors when using `x` for cases it's not available.
143+
using typename Ts::x... UIE; // expected-note 4 {{using declaration annotated with 'using_if_exists' here}}
137144

138-
x mem(); // expected-error 3 {{reference to unresolved using declaration}}
145+
x mem(); // expected-error 4 {{reference to unresolved using declaration}}
139146
};
140147

141148
S2<E1, E2> e; // expected-note{{in instantiation of template class}}
@@ -145,14 +152,15 @@ S2<F1, E2> h; // expected-note{{in instantiation of template class}}
145152

146153
template <class... Ts>
147154
struct S3 : protected Ts... {
148-
using Ts::m... UIE; // expected-error{{target of using declaration conflicts with declaration already in scope}} expected-note{{target of using declaration}}
155+
// No errors for conflicting declarations because only one of the parent classes declares `m`.
156+
using Ts::m... UIE;
149157
};
150158
struct B1 {
151-
enum { m }; // expected-note{{conflicting declaration}}
159+
enum { m };
152160
};
153161
struct B2 {};
154162

155-
S3<B1, B2> i; // expected-note{{in instantiation of template}}
163+
S3<B1, B2> i;
156164
S<B2, B1> j;
157165

158166
} // namespace test_using_pack
@@ -170,23 +178,23 @@ NS2::x y; // expected-error {{reference to unresolved using declaration}}
170178
} // namespace test_nested
171179

172180
namespace test_scope {
173-
int x; // expected-note{{conflicting declaration}}
181+
int x;
174182
void f() {
175-
int x; // expected-note{{conflicting declaration}}
183+
int x;
176184
{
177185
using ::x UIE; // expected-note {{using declaration annotated with 'using_if_exists' here}}
178186
(void)x; // expected-error {{reference to unresolved using declaration}}
179187
}
180188

181189
{
182190
using test_scope::x;
183-
using ::x UIE; // expected-error{{target of using declaration conflicts with declaration already in scope}} expected-note{{target of using declaration}}
191+
using ::x UIE; // OK since there's no `x` in the global namespace, so this wouldn't be any declaration
184192
(void)x;
185193
}
186194

187195
(void)x;
188196

189-
using ::x UIE; // expected-error{{target of using declaration conflicts with declaration already in scope}} expected-note{{target of using declaration}}
197+
using ::x UIE; // OK since there's no `x` in the global namespace, so this wouldn't be any declaration
190198
(void)x;
191199
}
192200
} // namespace test_scope
@@ -224,3 +232,20 @@ int main() {
224232
size = fake_printf();
225233
size = std::fake_printf();
226234
}
235+
236+
// Regression test for https://github.com/llvm/llvm-project/issues/85335.
237+
// No errors should be reported here.
238+
namespace PR85335 {
239+
void foo();
240+
241+
namespace N {
242+
void bar();
243+
244+
using ::foo __attribute__((__using_if_exists__));
245+
using ::bar __attribute__((__using_if_exists__));
246+
}
247+
248+
void baz() {
249+
N::bar();
250+
}
251+
} // namespace PR85335

0 commit comments

Comments
 (0)