-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[clang][Parser] Allow private type aliases in out-of-line member function return types #169272
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
base: main
Are you sure you want to change the base?
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
58432c2 to
7563ce7
Compare
|
@vgvassilev Would you mind please reviewing this for viability? It's a very small change, but I can't be sure it's the correct way to go about this. We're aiming to solve this problem #164885 which currently prevents |
|
@llvm/pr-subscribers-clang Author: Paulo Rafael Feodrippe (pfeodrippe) ChangesFixes #164885. Used Copilot. Run all clang tests locally with From #164885, we can see the issue below in clang-repl that works just fine in normal clang. The issue happens when we are parsing input like "A::B *A::foo()" where the type "A::B" might be a private member type, but if this turns out to be an out-of-line member function definition. The fix, only applied in incremental mode (clang-repl / Interpreter) seems to be to suppress access checks for qualified type lookups when no delayed diagnostic pool is active. /***** somecode.c *****/
struct scheduler
{ };
class io_context
{
using impl_type = scheduler;
public:
impl_type *foo();
};
/* The error doesn't occur if `impl_type` is a parameter type. Only for the return type. */
io_context::impl_type *io_context::foo()
{ return nullptr; }/***** in clang-repl *****/
❯ clang-repl
clang-repl> #include "asio-repro.hpp"
In file included from <<< inputs >>>:1:
In file included from input_line_1:1:
./asio-repro.hpp:13:13: error: 'impl_type' is a private member of 'io_context'
13 | io_context::impl_type *io_context::add_impl()
| ^
./asio-repro.hpp:7:9: note: implicitly declared private here
7 | using impl_type = scheduler;
| ^
error: Parsing failed.Full diff: https://github.com/llvm/llvm-project/pull/169272.diff 2 Files Affected:
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index a6fc676f23a51..ac5c90bf7c7ac 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -2020,6 +2020,27 @@ bool Parser::TryAnnotateTypeOrScopeTokenAfterScopeSpec(
CXXScopeSpec &SS, bool IsNewScope,
ImplicitTypenameContext AllowImplicitTypename) {
if (Tok.is(tok::identifier)) {
+ // In incremental/clang-repl mode, suppress access checks for qualified
+ // type lookups when no delayed diagnostic pool is active. This handles
+ // the case where we're parsing input like "A::B *A::foo()" where the type
+ // "A::B" might be a private member type, but if this turns out to be an
+ // out-of-line member function definition, access should be allowed.
+ //
+ // When the declaration is actually parsed (via ParseDeclarationOrFunctionDefinition),
+ // the ParsingDeclSpec will set up proper delayed diagnostics to handle
+ // access checking in the correct context.
+ //
+ // We only do this in incremental mode because this is where the issue
+ // manifests - disambiguation happens before ParsingDeclSpec is created.
+ // In normal compilation, these access checks would be re-triggered during
+ // actual parsing with delayed diagnostics active.
+ bool SuppressAccess = getLangOpts().IncrementalExtensions &&
+ SS.isNotEmpty() &&
+ !Actions.DelayedDiagnostics.shouldDelayDiagnostics();
+ std::optional<SuppressAccessChecks> SAC;
+ if (SuppressAccess)
+ SAC.emplace(*this, true);
+
// Determine whether the identifier is a type name.
if (ParsedType Ty = Actions.getTypeName(
*Tok.getIdentifierInfo(), Tok.getLocation(), getCurScope(), &SS,
diff --git a/clang/test/Interpreter/private-member-access.cpp b/clang/test/Interpreter/private-member-access.cpp
new file mode 100644
index 0000000000000..a18860e8ab152
--- /dev/null
+++ b/clang/test/Interpreter/private-member-access.cpp
@@ -0,0 +1,60 @@
+// RUN: cat %s | clang-repl | FileCheck %s
+
+extern "C" int printf(const char*, ...);
+
+// Test 1: Private nested struct in return type
+class Container { struct Node { int data; }; public: Node* create(); };
+Container::Node* Container::create() { return new Node{456}; }
+printf("Private nested struct: %d\n", Container().create()->data);
+// CHECK: Private nested struct: 456
+
+// Test 2: Private enum in return type
+class Status { enum Code { OK = 0, ERROR = 1 }; public: Code get(); };
+Status::Code Status::get() { return OK; }
+printf("Private enum: %d\n", Status().get());
+// CHECK: Private enum: 0
+
+// Test 3: Template with private type alias
+template<typename T> class Handler { using ptr = T*; public: ptr get(); };
+template<typename T> typename Handler<T>::ptr Handler<T>::get() { return nullptr; }
+printf("Template with private type: passed\n");
+// CHECK: Template with private type: passed
+
+// Test 4: Protected type alias (not just private)
+class ProtectedBase { protected: using value_type = int; public: value_type get(); };
+ProtectedBase::value_type ProtectedBase::get() { return 42; }
+printf("Protected type alias: %d\n", ProtectedBase().get());
+// CHECK: Protected type alias: 42
+
+// Test 5: Deeply nested private type (A::B::C)
+class Outer { public: class Middle { struct Inner { int x; }; public: Inner* create(); }; };
+Outer::Middle::Inner* Outer::Middle::create() { return new Inner{789}; }
+printf("Deeply nested: %d\n", Outer::Middle().create()->x);
+// CHECK: Deeply nested: 789
+
+// Test 6: Private typedef (not using declaration)
+class WithTypedef { typedef double real_t; public: real_t compute(); };
+WithTypedef::real_t WithTypedef::compute() { return 2.718; }
+printf("Private typedef: %.3f\n", WithTypedef().compute());
+// CHECK: Private typedef: 2.718
+
+// Test 7: Const-qualified return type with private type
+class ConstReturn { using data_t = int; public: const data_t& get(); private: data_t val = 100; };
+const ConstReturn::data_t& ConstReturn::get() { return val; }
+printf("Const return: %d\n", ConstReturn().get());
+// CHECK: Const return: 100
+
+// Test 8: Reference return type with private type
+class RefReturn { using ref_t = int; ref_t value = 55; public: ref_t& getRef(); };
+RefReturn::ref_t& RefReturn::getRef() { return value; }
+RefReturn rr; rr.getRef() = 66;
+printf("Reference return: %d\n", rr.getRef());
+// CHECK: Reference return: 66
+
+// Test 9: Pointer-to-pointer with private type
+class PtrPtr { using inner_t = int; public: inner_t** get(); };
+PtrPtr::inner_t** PtrPtr::get() { static int* p = nullptr; return &p; }
+printf("Pointer to pointer: passed\n");
+// CHECK: Pointer to pointer: passed
+
+%quit
|
This is not the way to go. We should fix this issue in the tentative parsing logic around: llvm-project/clang/lib/Parse/ParseTentative.cpp Lines 56 to 63 in e04c01b
|
…tion return types Bug: In clang-repl (incremental parsing mode), out-of-line member function definitions that use private/protected type aliases in their return type incorrectly fail with access errors (e.g., io_context::impl_type *io_context::foo() errors with "'impl_type' is a private member of 'io_context'"). This happens because the disambiguation logic in isCXXDeclarationStatement calls isCXXSimpleDeclaration, which triggers access checking before the parser establishes the correct declaration context. Fix: Extend the heuristic in ParseTentative.cpp that skips isCXXSimpleDeclaration to also recognize pointer/reference patterns (A::B *id, A::B &id, A::B &&id, A::B **id) as likely declarations, avoiding the premature access check.
7563ce7 to
e1ca2f5
Compare
| // member functions and static variable definitions. Check if the next | ||
| // token is also an identifier and assume a declaration. | ||
| // We cannot check if the scopes match because the declarations could | ||
| // involve namespaces and friend declarations. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me know if this comment above should be modified \o
| RevertingTentativeParsingAction PA(*this); | ||
| ConsumeToken(); // consume the identifier (type name) | ||
| // Skip all consecutive *, &, && tokens (for cases like A::B **) | ||
| while (Tok.isOneOf(tok::star, tok::amp, tok::ampamp)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I presume the c++ scope specifier has been parsed already. In this case we should call a parser function rather than implementing some parsing by ourselves. You can trace what clang does for out-of-line definitions after calling ParseOptionalCXXScopeSpecifier. We should do the same and if that fails we know that this is not a declaration, more parsing will be required otherwise.
Fixes #164885.
Used Copilot. Run all clang tests locally with
ninja check-clang(everything that was gree in main is also green here.)From #164885,
clang-replfails to handle private member aliases in member function return type, we can see the issue below in clang-repl that works just fine in normal clang.Fix: Extend the heuristic in ParseTentative.cpp that skips isCXXSimpleDeclaration to also recognize pointer/reference patterns (A::B *id, A::B &id, A::B &&id, A::B **id) as likely declarations, avoiding the premature access check.