-
Notifications
You must be signed in to change notification settings - Fork 15k
[clang]: reflection operator parsing for global namespace and primitive types #164692
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?
Changes from all commits
cda974f
daf965a
5cfcf49
59c609c
c59b1d3
a9df861
9b353b7
88d8af8
245f466
482e25e
c2d6d7e
7c7ac79
61c7f94
78fe7c4
5ae0873
4503b11
4852569
d3f0f70
e93fa6e
e845cb8
2d8c1cb
7a331be
340d7c0
7c30c5d
08cd161
258283e
6b8aaf5
8660059
e6fec7b
95ec04c
8be44be
5af4ae2
13cbe6f
8bd9dc3
dacc226
442feeb
ad7a5e6
632c4d8
ae941f2
745cc22
f61f630
455549f
a7281de
72c191d
af47462
21b9784
0de3284
634c3dd
dc5b7cc
5a4efbd
ceafe4b
0ac0294
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5501,6 +5501,60 @@ class BuiltinBitCastExpr final | |
| } | ||
| }; | ||
|
|
||
| /// Represents a C++26 reflect expression [expr.reflect]. The operand of of the | ||
| /// expression is either a: | ||
| /// - :: (global namespace) | ||
| /// - a reflection-name | ||
| /// - a type-id | ||
| /// - id-expression. | ||
| class CXXReflectExpr : public Expr { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you document this better? This comment seems outdated? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I updated, does it look ok now? |
||
|
|
||
| // Source locations. | ||
| SourceLocation OperatorLoc; | ||
| SourceRange OperandRange; | ||
|
|
||
| CXXReflectExpr(const ASTContext &C, QualType T, QualType Ty); | ||
| CXXReflectExpr(const ASTContext &C, QualType T, Decl *Arg); | ||
| CXXReflectExpr(EmptyShell Empty); | ||
|
|
||
| public: | ||
| static CXXReflectExpr *Create(ASTContext &C, SourceLocation OperatorLoc, | ||
| SourceLocation ArgLoc, QualType Operand); | ||
|
|
||
| static CXXReflectExpr *Create(ASTContext &C, SourceLocation OperatorLoc, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the difference between these two create functions, and when would we use one vs the other? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For now, the top one I create to use for parsing builtin types, it may be extended to parse other type-id's in upcoming PRs. The other one I use for parsing global namespace for now, but I think it could be extended in upcoming PRs for other kinds, e.g, namespace name. |
||
| SourceLocation OperandLoc, Decl *Operand); | ||
|
|
||
| static CXXReflectExpr *CreateEmpty(ASTContext &C); | ||
|
|
||
| SourceLocation getBeginLoc() const LLVM_READONLY { return OperatorLoc; } | ||
| SourceLocation getEndLoc() const LLVM_READONLY { | ||
| return OperandRange.getEnd(); | ||
| } | ||
| SourceRange getSourceRange() const { | ||
| return SourceRange(getBeginLoc(), getEndLoc()); | ||
| } | ||
|
|
||
| /// Returns location of the '^^'-operator. | ||
| SourceLocation getOperatorLoc() const { return OperatorLoc; } | ||
| SourceRange getOperandRange() const { return OperandRange; } | ||
|
|
||
| /// Sets the location of the '^^'-operator. | ||
| void setOperatorLoc(SourceLocation L) { OperatorLoc = L; } | ||
| void setOperandRange(SourceRange R) { OperandRange = R; } | ||
|
|
||
| child_range children() { | ||
| return child_range(child_iterator(), child_iterator()); | ||
| } | ||
|
|
||
| const_child_range children() const { | ||
| return const_child_range(const_child_iterator(), const_child_iterator()); | ||
| } | ||
|
|
||
| static bool classof(const Stmt *T) { | ||
| return T->getStmtClass() == CXXReflectExprClass; | ||
| } | ||
| }; | ||
|
|
||
| } // namespace clang | ||
|
|
||
| #endif // LLVM_CLANG_AST_EXPRCXX_H | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2883,6 +2883,8 @@ DEF_TRAVERSE_STMT(CXXUnresolvedConstructExpr, { | |
| TRY_TO(TraverseTypeLoc(S->getTypeSourceInfo()->getTypeLoc())); | ||
| }) | ||
|
|
||
| DEF_TRAVERSE_STMT(CXXReflectExpr, {/*TODO*/}) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I might be mis-remembering, but I think this is used by template <typename T>
constexpr int r = (^^T, 1);to work properly. Might be worth implementing this in this PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From your example if looks like we need support for reflection of template right? but for now this PR only supports global namespace and primitive types. If missing this might cause some issues for the internals, let me know. I can look into it as well. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not quite - There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Whoops! Thanks for the correction. Though we probably still want to recurse over that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I haven’t thought about this too much, but the logic will probably end up being fairly similar to |
||
|
|
||
| // These expressions all might take explicit template arguments. | ||
| // We traverse those if so. FIXME: implement these. | ||
| DEF_TRAVERSE_STMT(CXXConstructExpr, {}) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -499,6 +499,7 @@ LANGOPT(BoundsSafety, 1, 0, NotCompatible, "Bounds safety extension for C") | |
| LANGOPT(EnableLifetimeSafety, 1, 0, NotCompatible, "Experimental lifetime safety analysis for C++") | ||
|
|
||
| LANGOPT(PreserveVec3Type, 1, 0, NotCompatible, "Preserve 3-component vector type") | ||
| LANGOPT(Reflection , 1, 0, NotCompatible, "C++26 Reflection") | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we want this as an option. IMO, we should just have this be turned on for C++26, and not enable-able for other language versions. |
||
|
|
||
| #undef LANGOPT | ||
| #undef ENUM_LANGOPT | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3689,6 +3689,11 @@ defm application_extension : BoolFOption<"application-extension", | |
| PosFlag<SetTrue, [], [ClangOption, CC1Option], | ||
| "Restrict code to those available for App Extensions">, | ||
| NegFlag<SetFalse>>; | ||
| defm reflection : BoolFOption<"reflection", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again, we should decide which versions we want this enabled for, but I don't think just enabling on a command line arg is going to be our direction forward. |
||
| LangOpts<"Reflection">, DefaultFalse, | ||
| PosFlag<SetTrue, [], [ClangOption, CC1Option], | ||
| "Enable C++26 reflection">, | ||
| NegFlag<SetFalse>>; | ||
| defm sized_deallocation : BoolFOption<"sized-deallocation", | ||
| LangOpts<"SizedDeallocation">, Default<cpp14.KeyPath>, | ||
| PosFlag<SetTrue, [], [], "Enable C++14 sized global deallocation functions">, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -150,6 +150,7 @@ enum class TentativeCXXTypeIdContext { | |
| AsTemplateArgument, | ||
| InTrailingReturnType, | ||
| AsGenericSelectionArgument, | ||
| AsReflectionOperand | ||
| }; | ||
|
|
||
| /// The kind of attribute specifier we have found. | ||
|
|
@@ -5167,6 +5168,10 @@ class Parser : public CodeCompletionHandler { | |
| /// Implementations are in ParseHLSL.cpp | ||
| ///@{ | ||
|
|
||
| //===--------------------------------------------------------------------===// | ||
| // Parses the operand of reflection operator | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should have a |
||
| ExprResult ParseCXXReflectExpression(SourceLocation OpLoc); | ||
|
|
||
| private: | ||
| bool MaybeParseHLSLAnnotations(Declarator &D, | ||
| SourceLocation *EndLoc = nullptr, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2566,6 +2566,11 @@ void StmtPrinter::VisitCXXUnresolvedConstructExpr( | |
| OS << ')'; | ||
| } | ||
|
|
||
| void StmtPrinter::VisitCXXReflectExpr(CXXReflectExpr *S) { | ||
| // TODO(Reflection): Implement this. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would think you could partially implement this but I guess you are avoiding it to keep things simple? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah im thinking since we don't have enough info in CXXReflectExpr yet, we can leave it for the next round. But let me know what you think we can show now. |
||
| llvm_unreachable("not implemented yet"); | ||
| } | ||
|
|
||
| void StmtPrinter::VisitCXXDependentScopeMemberExpr( | ||
| CXXDependentScopeMemberExpr *Node) { | ||
| if (!Node->isImplicitAccess()) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1208,6 +1208,13 @@ Parser::ParseCastExpression(CastParseKind ParseKind, bool isAddressOfOperand, | |
| AllowSuffix = false; | ||
| Res = ParseUnaryExprOrTypeTraitExpression(); | ||
| break; | ||
| case tok::caretcaret: { | ||
| if (getLangOpts().Reflection) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Else case here feels like a bad idea? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can add a diagnostic here in the else case about option flag for reflection not turned on, But you left a comment above about whether we need to have an option flag for reflection, so I guess lets resolve that first. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you also need the changes to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How do you distinguish a block though? Maybe we don;t want to do arbitrary lookahead to see if we find a brace everytime someone uses I think for the time being I would prefer
We can figure out recovery for objective c++ later (and maybe always propose a fixit to inject a space) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need for lookahead afaict - if you're in a binary expression, it can't be a reflect-expression. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don’t really know much Objective-C, but I would imagine that an xor of an immediately-invoked block wouldn’t be too common, and if someone wants that parse, they can still write There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just following up, are we going to need to handle the case for block literal in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I apologize but we should add tests in Objective-C to make sure we don't break things accidently. Maybe @rjmccall might be helpful here in guiding us on things to look out for here. |
||
| SourceLocation DoubleCaret = ConsumeToken(); | ||
| Res = ParseCXXReflectExpression(/*OpLoc=*/DoubleCaret); | ||
| } | ||
| break; | ||
| } | ||
| case tok::ampamp: { // unary-expression: '&&' identifier | ||
| if (NotPrimaryExpression) | ||
| *NotPrimaryExpression = true; | ||
|
|
@@ -2249,6 +2256,9 @@ ExprResult Parser::ParseUnaryExprOrTypeTraitExpression() { | |
| else if (getLangOpts().C2y && OpTok.is(tok::kw__Countof)) | ||
| Diag(OpTok, diag::warn_c2y_compat_keyword) << OpTok.getName(); | ||
|
|
||
| if (OpTok.is(tok::caretcaret)) | ||
| return ParseCXXReflectExpression(OpTok.getLocation()); | ||
|
|
||
| EnterExpressionEvaluationContext Unevaluated( | ||
| Actions, Sema::ExpressionEvaluationContext::Unevaluated, | ||
| Sema::ReuseLambdaContextDecl); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| //===--- ParseReflect.cpp - C++26 Reflection Parsing ---------------------===// | ||
| // | ||
| // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. | ||
| // See https://llvm.org/LICENSE.txt for license information. | ||
| // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
| // | ||
| //===----------------------------------------------------------------------===// | ||
| // | ||
| // This file implements parsing for reflection facilities. | ||
| // | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| #include "clang/AST/LocInfoType.h" | ||
| #include "clang/Basic/DiagnosticParse.h" | ||
| #include "clang/Parse/Parser.h" | ||
| #include "clang/Sema/EnterExpressionEvaluationContext.h" | ||
| using namespace clang; | ||
|
|
||
| ExprResult Parser::ParseCXXReflectExpression(SourceLocation OpLoc) { | ||
| // TODO(reflection) : support parsing for more reflect-expressions. | ||
| EnterExpressionEvaluationContext Unevaluated( | ||
| Actions, Sema::ExpressionEvaluationContext::Unevaluated); | ||
|
|
||
| SourceLocation OperandLoc = Tok.getLocation(); | ||
|
|
||
| { | ||
| TentativeParsingAction TPA(*this); | ||
| // global namespace :: | ||
| if (Tok.is(tok::coloncolon)) { | ||
| ConsumeToken(); | ||
| TPA.Commit(); | ||
| Decl *TUDecl = Actions.getASTContext().getTranslationUnitDecl(); | ||
| return Actions.ActOnCXXReflectExpr(OpLoc, SourceLocation(), TUDecl); | ||
| } | ||
| TPA.Revert(); | ||
| } | ||
|
|
||
| if (isCXXTypeId(TentativeCXXTypeIdContext::AsReflectionOperand)) { | ||
| TypeResult TR = ParseTypeName(/*TypeOf=*/nullptr); | ||
| if (TR.isInvalid()) | ||
| return ExprError(); | ||
|
|
||
| TypeSourceInfo *TSI = nullptr; | ||
| QualType QT = Actions.GetTypeFromParser(TR.get(), &TSI); | ||
|
|
||
| if (QT.isNull()) | ||
| return ExprError(); | ||
|
|
||
| if (!TSI) | ||
| TSI = Actions.getASTContext().getTrivialTypeSourceInfo(QT, OperandLoc); | ||
|
|
||
| QualType Canon = QT.getCanonicalType(); | ||
| if (Canon->isBuiltinType()) { | ||
| // Only supports builtin types for now | ||
| return Actions.ActOnCXXReflectExpr(OpLoc, TSI); | ||
| } | ||
| } | ||
|
|
||
| Diag(OperandLoc, diag::err_cannot_reflect_operand); | ||
| return ExprError(); | ||
| } |
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.