cjdb updated this revision to Diff 492234.
cjdb retitled this revision from "[clang] teaches Clang the special ADL rules
for functions in std::ranges" to "adds `__disable_adl` attribute".
cjdb edited the summary of this revision.
cjdb added a comment.
- updates patch so that it meets what Aaron and I want, also meets most of
Richard's feedback
- updates commit message
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D129951/new/
https://reviews.llvm.org/D129951
Files:
clang/include/clang/AST/ExprCXX.h
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/AttrDocs.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Basic/TokenKinds.def
clang/lib/Parse/ParseDecl.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/lib/Sema/SemaLookup.cpp
clang/lib/Sema/SemaOverload.cpp
clang/test/SemaCXX/disable-adl.cpp
Index: clang/test/SemaCXX/disable-adl.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/disable-adl.cpp
@@ -0,0 +1,179 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify -std=c++20
+// RUN: %clang_cc1 %s -fsyntax-only -verify -std=c++2b
+
+namespace NS1 {
+ struct S1 {};
+ S1 inhibited(S1); // expected-note 2 {{candidate function}}
+
+ namespace NNS1 {
+ struct S2 {};
+ __disable_adl void hidden(S2); // expected-note{{declared here}}
+ __disable_adl int inhibited(S1); // expected-note 4 {{candidate function}}
+ }
+}
+
+namespace NS2 {
+ __disable_adl void inhibited(NS1::S1); // expected-note 2 {{candidate function}}
+}
+
+void test_functions() {
+ hidden(NS1::NNS1::S2{}); // expected-error{{use of undeclared identifier 'hidden'; did you mean 'NS1::NNS1::hidden'?}}
+ {
+ NS1::S1 x = inhibited(NS1::S1{}); // no error
+ }
+ {
+ using namespace NS1::NNS1;
+ int x = inhibited(NS1::S1{}); // no error
+
+ using namespace NS1;
+ S1 y = inhibited(NS1::S1{}); // expected-error{{call to 'inhibited' is ambiguous}}
+ }
+ {
+ using NS1::NNS1::inhibited;
+ int x = inhibited(NS1::S1{}); // no error
+
+ using NS1::inhibited;
+ NS1::S1 y = inhibited(NS1::S1{}); // expected-error{{call to 'inhibited' is ambiguous}}
+ }
+ {
+ using namespace NS2;
+ inhibited(NS1::S1{}); // no error
+
+ using namespace NS1::NNS1;
+ inhibited(NS1::S1{}); // expected-error{{call to 'inhibited' is ambiguous}}
+ }
+ {
+ using NS2::inhibited;
+ inhibited(NS1::S1{}); // no error
+
+ using NS1::NNS1::inhibited;
+ inhibited(NS1::S1{}); // expected-error{{call to 'inhibited' is ambiguous}}
+ }
+}
+
+namespace NS1 {
+ template<typename T>
+ S1 inhibited_template(T); // expected-note 2 {{candidate function}}
+
+ namespace NNS1 {
+ template<typename T>
+ __disable_adl void hidden_template(T); // expected-note{{declared here}}
+
+ template<typename T>
+ __disable_adl int inhibited_template(T); // expected-note 4 {{candidate function}}
+ }
+}
+
+namespace NS2 {
+ template<typename T>
+ __disable_adl int inhibited_template(T); // expected-note 2 {{candidate function}}
+}
+
+void test_function_templates() {
+ hidden_template(NS1::NNS1::S2{}); // expected-error{{use of undeclared identifier 'hidden_template'; did you mean 'NS1::NNS1::hidden_template'?}}
+
+ {
+ NS1::S1 x = inhibited_template(NS1::S1{}); // no error
+ }
+ {
+ using namespace NS1::NNS1;
+ int x = inhibited_template(NS1::S1{}); // no error
+
+ using namespace NS1;
+ S1 y = inhibited_template(NS1::S1{}); // expected-error{{call to 'inhibited_template' is ambiguous}}
+ }
+ {
+ using NS1::NNS1::inhibited_template;
+ int x = inhibited_template(NS1::S1{}); // no error
+
+ using NS1::inhibited_template;
+ NS1::S1 y = inhibited_template(NS1::S1{}); // expected-error{{call to 'inhibited_template' is ambiguous}}
+ }
+ {
+ using namespace NS2;
+ inhibited_template(NS1::S1{}); // no error
+
+ using namespace NS1::NNS1;
+ inhibited_template(NS1::S1{}); // expected-error{{call to 'inhibited_template' is ambiguous}}
+ }
+ {
+ using NS2::inhibited_template;
+ inhibited_template(NS1::S1{}); // no error
+
+ using NS1::NNS1::inhibited_template;
+ inhibited_template(NS1::S1{}); // expected-error{{call to 'inhibited_template' is ambiguous}}
+ }
+}
+
+namespace NS1 {
+ S1 inhibited_mixed(S1);
+
+ namespace NNS1 {
+ template<typename T>
+ __disable_adl int inhibited_mixed(T);
+ }
+}
+
+void test_mixed() {
+ using namespace NS1::NNS1;
+ int x = inhibited_mixed(NS1::S1{}); // no error
+}
+
+// Should be covered by the hidden functions checks, but just to be sure.
+void test_NNS1_hidden() {
+ {
+ NS1::S1 a = inhibited(NS1::S1{});
+ NS1::S1 b = inhibited_template(NS1::S1{});
+ NS1::S1 c = inhibited_mixed(NS1::S1{});
+ }
+ {
+ using namespace NS1;
+ NS1::S1 a = inhibited(NS1::S1{});
+ NS1::S1 b = inhibited_template(NS1::S1{});
+ NS1::S1 c = inhibited_mixed(NS1::S1{});
+ }
+}
+
+namespace NS1 {
+ namespace NNS1 {
+ __disable_adl void operator-(S2); // expected-error{{can't apply '__disable_adl' to operators, since they're supposed to be used with ADL}}
+
+ struct hidden_friend_operator {
+ friend void operator-(hidden_friend_operator i, int) {}
+ };
+
+ struct hidden_friend_swap {
+ __disable_adl friend void swap(hidden_friend_swap, hidden_friend_swap) {}
+ };
+ }
+}
+
+void test_friends_and_operators() {
+ -NS1::NNS1::S2{}; // no error
+ NS1::NNS1::hidden_friend_operator{} - 1; // no error
+
+ swap(NS1::NNS1::hidden_friend_swap{}, NS1::NNS1::hidden_friend_swap{}); // expected-error{{use of undeclared identifier 'swap'}}
+}
+
+struct S {
+ __disable_adl void f(); // expected-error{{can't apply '__disable_adl' to member functions, since they don't interact with ADL}}
+ __disable_adl static void g(); // expected-error{{can't apply '__disable_adl' to member functions, since they don't interact with ADL}}
+};
+
+template <class> using common_comparison_category_t = int;
+template <class T> T declval;
+template <class> __disable_adl auto synth_three_way();
+template <class T, class U = T> using synth_three_way_result = decltype(synth_three_way(declval<U>));
+template <class> concept three_way_comparable_synthesisable = requires { synth_three_way; };
+template <class T2> struct pair {
+ template <three_way_comparable_synthesisable U = T2>
+ auto operator>(pair) -> common_comparison_category_t<synth_three_way_result<U>>;
+};
+struct pair_spaceship_invalid {
+ pair_spaceship_invalid() { test<pair<int>>(); } // expected-note{{}}
+ template <class> void test() noexcept;
+};
+template <class T> void pair_spaceship_invalid::test() noexcept {
+ auto p1 = T(), p2 = T();
+ requires { p1 > p2; }; // expected-warning 2 {{expression result unused}}
+}
Index: clang/lib/Sema/SemaOverload.cpp
===================================================================
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -6511,7 +6511,7 @@
NamedDecl *ND = Function;
if (auto *SpecInfo = Function->getTemplateSpecializationInfo())
ND = SpecInfo->getTemplate();
-
+
if (ND->getFormalLinkage() == Linkage::InternalLinkage) {
Candidate.Viable = false;
Candidate.FailureKind = ovl_fail_module_mismatched;
@@ -12921,12 +12921,20 @@
ArrayRef<Expr *> Args,
OverloadCandidateSet &CandidateSet,
bool PartialOverloading,
- bool KnownValid) {
+ bool KnownValid,
+ UnresolvedLookupExpr* ULE) {
NamedDecl *Callee = FoundDecl.getDecl();
if (isa<UsingShadowDecl>(Callee))
Callee = cast<UsingShadowDecl>(Callee)->getTargetDecl();
if (FunctionDecl *Func = dyn_cast<FunctionDecl>(Callee)) {
+ if (Func->hasAttr<DisableADLAttr>() && ULE) {
+ ULE->disableADL();
+ }
+ // else if (!CandidateSet.empty() && CandidateSet.begin()->FoundDecl->hasAttr<DisableADLAttr>()) {
+ // return;
+ // }
+
if (ExplicitTemplateArgs) {
assert(!KnownValid && "Explicit template arguments?");
return;
@@ -12943,6 +12951,13 @@
if (FunctionTemplateDecl *FuncTemplate
= dyn_cast<FunctionTemplateDecl>(Callee)) {
+ if (FuncTemplate->getAsFunction()->hasAttr<DisableADLAttr>() && ULE) {
+ ULE->disableADL();
+ }
+ // else if (!CandidateSet.empty() && CandidateSet.begin()->FoundDecl->hasAttr<DisableADLAttr>()) {
+ // return;
+ // }
+
S.AddTemplateOverloadCandidate(FuncTemplate, FoundDecl,
ExplicitTemplateArgs, Args, CandidateSet,
/*SuppressUserConversions=*/false,
@@ -13001,7 +13016,7 @@
E = ULE->decls_end(); I != E; ++I)
AddOverloadedCallCandidate(*this, I.getPair(), ExplicitTemplateArgs, Args,
CandidateSet, PartialOverloading,
- /*KnownValid*/ true);
+ /*KnownValid*/ true, ULE);
if (ULE->requiresADL())
AddArgumentDependentLookupCandidates(ULE->getName(), ULE->getExprLoc(),
@@ -13016,7 +13031,7 @@
ArrayRef<Expr *> Args, OverloadCandidateSet &CandidateSet) {
for (LookupResult::iterator I = R.begin(), E = R.end(); I != E; ++I)
AddOverloadedCallCandidate(*this, I.getPair(), ExplicitTemplateArgs, Args,
- CandidateSet, false, /*KnownValid*/ false);
+ CandidateSet, false, /*KnownValid*/ false, nullptr);
}
/// Determine whether a declaration with the specified name could be moved into
@@ -13962,8 +13977,8 @@
Diag(FnDecl->getLocation(),
diag::note_ovl_ambiguous_oper_binary_reversed_self);
// Mark member== const or provide matching != to disallow reversed
- // args. Eg.
- // struct S { bool operator==(const S&); };
+ // args. Eg.
+ // struct S { bool operator==(const S&); };
// S()==S();
if (auto *MD = dyn_cast<CXXMethodDecl>(FnDecl))
if (Op == OverloadedOperatorKind::OO_EqualEqual &&
Index: clang/lib/Sema/SemaLookup.cpp
===================================================================
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -3873,6 +3873,9 @@
!isa<FunctionTemplateDecl>(Underlying))
continue;
+ if (Underlying->getAsFunction()->hasAttr<DisableADLAttr>())
+ continue;
+
// The declaration is visible to argument-dependent lookup if either
// it's ordinarily visible or declared as a friend in an associated
// class.
Index: clang/lib/Sema/SemaDeclAttr.cpp
===================================================================
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -5751,6 +5751,14 @@
D->addAttr(::new (S.Context) BuiltinAliasAttr(S.Context, AL, Ident));
}
+static void handleDisableADLAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+ if (FunctionDecl *F = D->getAsFunction(); F->isOverloadedOperator() || F->isCXXClassMember()) {
+ S.Diag(AL.getLoc(), diag::err_disable_adl_no_operators) << F->isCXXClassMember();
+ return;
+ }
+ D->addAttr(::new (S.Context) DisableADLAttr(S.Context, AL));
+}
+
//===----------------------------------------------------------------------===//
// Checker-specific attribute handlers.
//===----------------------------------------------------------------------===//
@@ -9347,6 +9355,10 @@
case ParsedAttr::AT_UsingIfExists:
handleSimpleAttribute<UsingIfExistsAttr>(S, D, AL);
break;
+
+ case ParsedAttr::AT_DisableADL:
+ handleDisableADLAttr(S, D, AL);
+ break;
}
}
Index: clang/lib/Parse/ParseDecl.cpp
===================================================================
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -3782,6 +3782,14 @@
ParseAttributes(PAKM_GNU | PAKM_Declspec, DS.getAttributes(), LateAttrs);
continue;
+ case tok::kw___disable_adl: {
+ IdentifierInfo* AttrName = Tok.getIdentifierInfo();
+ SourceLocation AttrNameLoc = Tok.getLocation();
+ DS.getAttributes().addNew(AttrName, AttrNameLoc, nullptr, AttrNameLoc,
+ nullptr, 0, ParsedAttr::AS_Keyword);
+ break;
+ }
+
// Microsoft single token adornments.
case tok::kw___forceinline: {
isInvalid = DS.setFunctionSpecForceInline(Loc, PrevSpec, DiagID);
Index: clang/include/clang/Basic/TokenKinds.def
===================================================================
--- clang/include/clang/Basic/TokenKinds.def
+++ clang/include/clang/Basic/TokenKinds.def
@@ -740,6 +740,7 @@
KEYWORD(__builtin_bit_cast , KEYALL)
KEYWORD(__builtin_available , KEYALL)
KEYWORD(__builtin_sycl_unique_stable_name, KEYSYCL)
+KEYWORD(__disable_adl , KEYCXX)
// Clang-specific keywords enabled only in testing.
TESTING_KEYWORD(__unknown_anytype , KEYALL)
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3163,6 +3163,9 @@
"vector size not an integral multiple of component size">;
def err_attribute_zero_size : Error<"zero %0 size">;
def err_attribute_size_too_large : Error<"%0 size too large">;
+def err_disable_adl_no_operators : Error<
+ "can't apply '__disable_adl' to %select{operators|member functions}0, since "
+ "they%select{'re supposed to be used| don't interact}0 with ADL">;
def err_typecheck_sve_ambiguous : Error<
"cannot combine fixed-length and sizeless SVE vectors in expression, result is ambiguous (%0 and %1)">;
def err_typecheck_sve_gnu_ambiguous : Error<
Index: clang/include/clang/Basic/AttrDocs.td
===================================================================
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -539,16 +539,16 @@
let Category = DocCatStmt;
let Content = [{
If a statement is marked ``nomerge`` and contains call expressions, those call
-expressions inside the statement will not be merged during optimization. This
+expressions inside the statement will not be merged during optimization. This
attribute can be used to prevent the optimizer from obscuring the source
location of certain calls. For example, it will prevent tail merging otherwise
identical code sequences that raise an exception or terminate the program. Tail
merging normally reduces the precision of source location information, making
stack traces less useful for debugging. This attribute gives the user control
-over the tradeoff between code size and debug information precision.
+over the tradeoff between code size and debug information precision.
-``nomerge`` attribute can also be used as function attribute to prevent all
-calls to the specified function from merging. It has no effect on indirect
+``nomerge`` attribute can also be used as function attribute to prevent all
+calls to the specified function from merging. It has no effect on indirect
calls.
}];
}
@@ -1581,7 +1581,7 @@
``watchos``
Apple's watchOS operating system. The minimum deployment target is specified by
the ``-mwatchos-version-min=*version*`` command-line argument.
-
+
``driverkit``
Apple's DriverKit userspace kernel extensions. The minimum deployment target
is specified as part of the triple.
@@ -3883,7 +3883,7 @@
with pointers in the C family of languages. The various nullability attributes
indicate whether a particular pointer can be null or not, which makes APIs more
expressive and can help static analysis tools identify bugs involving null
-pointers. Clang supports several kinds of nullability attributes: the
+pointers. Clang supports several kinds of nullability attributes: the
``nonnull`` and ``returns_nonnull`` attributes indicate which function or
method parameters and result types can never be null, while nullability type
qualifiers indicate which pointer types can be null (``_Nullable``) or cannot
@@ -4059,7 +4059,7 @@
The ``returns_nonnull`` attribute implies that returning a null pointer is
undefined behavior, which the optimizer may take advantage of. The ``_Nonnull``
type qualifier indicates that a pointer cannot be null in a more general manner
-(because it is part of the type system) and does not imply undefined behavior,
+(because it is part of the type system) and does not imply undefined behavior,
making it more widely applicable
}];
}
@@ -6008,15 +6008,15 @@
let Content = [{
Code can indicate CFG checks are not wanted with the ``__declspec(guard(nocf))``
attribute. This directs the compiler to not insert any CFG checks for the entire
-function. This approach is typically used only sparingly in specific situations
-where the programmer has manually inserted "CFG-equivalent" protection. The
-programmer knows that they are calling through some read-only function table
-whose address is obtained through read-only memory references and for which the
-index is masked to the function table limit. This approach may also be applied
-to small wrapper functions that are not inlined and that do nothing more than
-make a call through a function pointer. Since incorrect usage of this directive
-can compromise the security of CFG, the programmer must be very careful using
-the directive. Typically, this usage is limited to very small functions that
+function. This approach is typically used only sparingly in specific situations
+where the programmer has manually inserted "CFG-equivalent" protection. The
+programmer knows that they are calling through some read-only function table
+whose address is obtained through read-only memory references and for which the
+index is masked to the function table limit. This approach may also be applied
+to small wrapper functions that are not inlined and that do nothing more than
+make a call through a function pointer. Since incorrect usage of this directive
+can compromise the security of CFG, the programmer must be very careful using
+the directive. Typically, this usage is limited to very small functions that
only call one function.
`Control Flow Guard documentation <https://docs.microsoft.com/en-us/windows/win32/secbp/pe-metadata>`
@@ -6847,3 +6847,6 @@
``enforce_read_only_placement`` attribute.
}];
}
+def DisableADLDocs : Documentation {
+ let Content = [{Please don't LGTM without this being fully documented.}];
+}
Index: clang/include/clang/Basic/Attr.td
===================================================================
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -4127,3 +4127,10 @@
let Subjects = SubjectList<[Record]>;
let Documentation = [ReadOnlyPlacementDocs];
}
+
+def DisableADL : InheritableAttr {
+ let Spellings = [Keyword<"__disable_adl">];
+ let Subjects = SubjectList<[Function]>;
+ let Documentation = [DisableADLDocs];
+ let LangOpts = [CPlusPlus];
+}
Index: clang/include/clang/AST/ExprCXX.h
===================================================================
--- clang/include/clang/AST/ExprCXX.h
+++ clang/include/clang/AST/ExprCXX.h
@@ -3214,6 +3214,9 @@
/// argument-dependent lookup.
bool requiresADL() const { return UnresolvedLookupExprBits.RequiresADL; }
+ /// A function marked '__disable_adl' inhibits ADL.
+ void disableADL() { UnresolvedLookupExprBits.RequiresADL = false; }
+
/// True if this lookup is overloaded.
bool isOverloaded() const { return UnresolvedLookupExprBits.Overloaded; }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
