Skip to content
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.

Sign up for GitHub

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

add qualified_name_of #28

Closed
wants to merge 3 commits into from
Closed

add qualified_name_of #28

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9bacba3
add qualified_name_of
Apr 4, 2024
479c041
.
Apr 10, 2024
a59d242
.
Apr 10, 2024
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
65 changes: 65 additions & 0 deletions clang/lib/Sema/Metafunctions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ static bool name_of(APValue &Result, Sema &S, EvalFn Evaluator,
QualType ResultTy, SourceRange Range,
ArrayRef<Expr *> Args);

static bool qualified_name_of(APValue &Result, Sema &S, EvalFn Evaluator,
QualType ResultTy, SourceRange Range,
ArrayRef<Expr *> Args);

static bool display_name_of(APValue &Result, Sema &S, EvalFn Evaluator,
QualType ResultTy, SourceRange Range,
ArrayRef<Expr *> Args);
Expand Down Expand Up @@ -326,6 +330,7 @@ static constexpr Metafunction Metafunctions[] = {

// exposed metafunctions
{ Metafunction::MFRK_cstring, 1, 1, name_of },
{ Metafunction::MFRK_cstring, 1, 1, qualified_name_of },
{ Metafunction::MFRK_cstring, 1, 1, display_name_of },
{ Metafunction::MFRK_sourceLoc, 1, 1, source_location_of },
{ Metafunction::MFRK_metaInfo, 1, 1, type_of },
Expand Down Expand Up @@ -1252,6 +1257,66 @@ bool map_decl_to_entity(APValue &Result, Sema &S, EvalFn Evaluator,
llvm_unreachable("unknown reflection kind");
}

template <typename DT>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than template-ize this function, we may be able to make DT& declType a NamedDecl *, as NamedDecl is an ancestor of ValueDecl, TemplateDecl, NamespaceDecl, and TypeDecl. Passing by reference as you've done here (i.e., NamedDecl &) would be just as good, but let's stick with passing by pointer since that's what's done throughout the rest of this file.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could but then I have to cast everything which is a bit ugly IMHO. But then I guess it would match the rest of the clang codebase so...

bool getQualifiedName(APValue &Result, Sema &S, EvalFn Evaluator, DT& declType)
{
SmallString<128> QualifiedNameBuffer;
llvm::raw_svector_ostream OS(QualifiedNameBuffer);
declType.printQualifiedName(OS);
StringRef Name = QualifiedNameBuffer.str();
if( makeCString(Result, Name, S.Context, Evaluator) )
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nits for consistency: space after if, no padding around parens, { on the same line as if.

if (makeCString(Result, Name, S.Context, Evaluator)) {

{
llvm_unreachable("failed to create qualified name string");
return true;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to return true after llvm_unreachable - Depending on the build configuration, this will either halt execution, or probably get optimized away entirely by the compiler (that is...the compiler building the compiler 😵 ).

}
return false;
}

bool qualified_name_of(APValue &Result, Sema &S, EvalFn Evaluator, QualType ResultTy,
SourceRange Range, ArrayRef<Expr *> Args) {
assert(Args[0]->getType()->isReflectionType());
assert(ResultTy ==
S.Context.getPointerType(S.Context.getConstType(S.Context.CharTy)));

APValue R;
if (!Evaluator(R, Args[0], true))
return true;

switch (R.getReflection().getKind()) {
case ReflectionValue::RK_type: {
QualType QT = R.getReflectedType();
return SetAndSucceed(Result, getTypeName(S.Context, Evaluator, QT,
/*emptyIfUnnamed=*/true));
}
case ReflectionValue::RK_declaration: {
const ValueDecl *VD = cast<ValueDecl>(R.getReflectedDecl());
return getQualifiedName(Result, S, Evaluator, *VD);
}
case ReflectionValue::RK_template: {
const TemplateDecl *TD = cast<TemplateDecl>(R.getReflectedTemplate().getAsTemplateDecl());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this cast necessary? I think getAsTemplateDecl() already returns a TemplateDecl *.

return getQualifiedName(Result, S, Evaluator, *TD);
}
case ReflectionValue::RK_const_value: {
if (makeCString(Result, "", S.Context, Evaluator))
llvm_unreachable("failed to create empty string");
return false;
}
case ReflectionValue::RK_namespace: {
const NamespaceDecl *ND = cast<NamespaceDecl>(R.getReflectedNamespace());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This, surprisingly, actually isn't quite right - in addition to a NamespaceDecl, getReflectedNamespace() might be a NamespaceAliasDecl * or a TranslationUnitDecl * (i.e., for reflections like ^:: of the global scope). Neither of these has NamespaceDecl as an ancestor base class, so this cast<NamespaceDecl> will crash with an assert-failure.

I think what you'll want to do here is:

  1. Get R.getReflectedNamespaced() as a Decl *.
  2. Check the special case of whether it's a TranslationUnitDecl. If it is, return the empty string (the global namespace isn't a "declared entity", so P2996 says this should return empty).
  3. Otherwise cast it to a NamedDecl * (which is a base class of both NamespaceDecl and NamespaceAliasDecl), and pass that to getQualifiedName.

We should probably add test cases for ^:: and for namespace aliases as well.

return getQualifiedName(Result, S, Evaluator, *ND);
}
case ReflectionValue::RK_base_specifier: {
QualType QT = R.getReflectedBaseSpecifier()->getType();
const Decl* D = findTypeDecl(QT);
const TypeDecl *TD = cast<TypeDecl>(D);
return getQualifiedName(Result, S, Evaluator, *TD);
}
case ReflectionValue::RK_data_member_spec:
return true;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although this is what we do for name_of, it's not quite correct - P2996 specifies:

For all other reflections, an empty string_view is produced.

So that should include this case as well. Let's just stack this case on top of RK_const_value.

case ReflectionValue::RK_const_value:
case ReflectionValue::RK_data_member_spec: {
  if (makeCString(Result, "", S.Context, Evaluator))
    llvm_unreachable("failed to create empty string");
  return false;
}

(bonus points if you want to fix that in name_of, while you're in here 😄 )

}
llvm_unreachable("unknown reflection kind");
}

bool name_of(APValue &Result, Sema &S, EvalFn Evaluator, QualType ResultTy,
SourceRange Range, ArrayRef<Expr *> Args) {
assert(Args[0]->getType()->isReflectionType());
Expand Down
7 changes: 7 additions & 0 deletions libcxx/include/experimental/meta
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ using info = decltype(^int);

// name and location
consteval auto name_of(info) -> string_view;
consteval auto qualified_name_of(info) -> string_view;
consteval auto display_name_of(info) -> string_view;
consteval auto source_location_of(info r) -> source_location;

Expand Down Expand Up @@ -159,6 +160,7 @@ enum : unsigned {

// exposed metafunctions
__metafn_name_of,
__metafn_qualified_name_of,
__metafn_display_name_of,
__metafn_source_location_of,
__metafn_type_of,
Expand Down Expand Up @@ -444,6 +446,11 @@ consteval auto name_of(info r) -> string_view {
return __metafunction(detail::__metafn_name_of, r);
}

// Returns the qualified name of the reflected entity, or the empty string if it has none.
consteval auto qualified_name_of(info r) -> string_view {
return __metafunction(detail::__metafn_qualified_name_of, r);
}

// Returns a name for the reflected entity.
consteval auto display_name_of(info r) -> string_view {
return __metafunction(detail::__metafn_display_name_of, r);
Expand Down
68 changes: 68 additions & 0 deletions libcxx/test/std/experimental/reflection/p2996-ex-qualified_name_of.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
#include <experimental/meta>

#include <string>
#include <string_view>

namespace outer_namespace{
namespace a_namespace {
namespace an_inner_namespace
{
struct ClassInsideSomeNamespaces {
int k;
int j;
using type = int;
};

typedef ClassInsideSomeNamespaces ClassInsideSomeNamespacesAlias;

template <typename T>
class SomeTemplateClass
{
std::vector<T> stuff;
public:
SomeTemplateClass(const T& t)
{
stuff.push_back(t);
}
};

class DerivedClass: public ClassInsideSomeNamespaces
{
int boopTheBloop;
};

const std::string aString = "abcde";
} // an_inner_namespace
} // a_namespace
} // outer_namespace

int main()
{
using namespace outer_namespace;
using namespace a_namespace;
using namespace an_inner_namespace;

constexpr auto expectedQNofType = "outer_namespace::a_namespace::an_inner_namespace::ClassInsideSomeNamespaces";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few stylistic nits, hope you don't mind:

  • I've been trying to keep line lengths capped at 80. We can break up lines like this by writing e.g.,:
    constexpr auto expectedQNofType = "outer_namespace::a_namespace::"
                                      "an_inner_namespace::ClassInsideSomeNamespaces";
  • super nit: please include spaces between identifiers and == operators in static assertions
  • Since std::meta:: is an associated namespace of all reflection values, we can usually omit the leading std::meta:: from metafunctions. So let's rewrite the static assertions like:
static_assert(expectedQNofDeclaration == qualified_name_of(^aString));

static_assert(expectedQNofType==std::meta::qualified_name_of(^ClassInsideSomeNamespaces));

constexpr auto expectedQNofDeclaration = "outer_namespace::a_namespace::an_inner_namespace::aString";
static_assert(expectedQNofDeclaration==std::meta::qualified_name_of(^aString));

constexpr auto expectedQNofNamespace = "outer_namespace::a_namespace::an_inner_namespace";
static_assert(expectedQNofNamespace==std::meta::qualified_name_of(^an_inner_namespace));

constexpr auto expectedQNofTemplate = "outer_namespace::a_namespace::an_inner_namespace::SomeTemplateClass";
static_assert(expectedQNofTemplate==std::meta::qualified_name_of(^SomeTemplateClass));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add a case to verify that qualified_name_of(^SomeTemplateClass<int>) doesn't include the trailing <int>, since P2996 specifically calls out:

For template instances, the name does not include the template argument list.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

waaaaah! this is broken. I'll take a look...


constexpr auto expectedQNofTypeAlias = "outer_namespace::a_namespace::an_inner_namespace::ClassInsideSomeNamespacesAlias";
static_assert(expectedQNofTypeAlias==std::meta::qualified_name_of(^ClassInsideSomeNamespacesAlias));

constexpr auto expectedQNofDerivedType = "outer_namespace::a_namespace::an_inner_namespace::DerivedClass";
static_assert(expectedQNofDerivedType==std::meta::qualified_name_of(^DerivedClass));

constexpr auto expectedQNofBaseSpecifier="outer_namespace::a_namespace::an_inner_namespace::ClassInsideSomeNamespaces";
constexpr auto base = [:(std::meta::reflect_value(std::meta::bases_of(^DerivedClass)[0])):];
static_assert(expectedQNofBaseSpecifier==std::meta::qualified_name_of(base));

return 0;
}