Page MenuHomePhabricator
Differential D75560 Diff 247997 clang/include/clang/AST/DeclBase.h

Changeset View

Standalone View

View Options

clang/include/clang/AST/DeclBase.h

Show First 20 Lines • Show All 91 Lines • ▼ Show 20 Lines
void setModulePrivate() { void setModulePrivate() {
// The module-private specifier has no effect on unowned declarations. // The module-private specifier has no effect on unowned declarations.
// FIXME: We should track this in some way for source fidelity. // FIXME: We should track this in some way for source fidelity.
if (getModuleOwnershipKind() == ModuleOwnershipKind::Unowned) if (getModuleOwnershipKind() == ModuleOwnershipKind::Unowned)
return; return;
setModuleOwnershipKind(ModuleOwnershipKind::ModulePrivate); setModuleOwnershipKind(ModuleOwnershipKind::ModulePrivate);
} }
public:
void setFromASTFile() { FromASTFile = true; }
rsmithUnsubmitted
Not Done
ReplyInline Actions

Setting this after creating a Decl is not safe in general; declarations with this flag set have 8 bytes of additional storage allocated before their start.

An assert(FromASTFile || hasLocalOwningModuleStorage()); would at least catch the unsafe cases. This also needs a comment saying that it's not safe to use in general, and perhaps a scarier name to discourage people from calling it. (Unless you're creating declarations with ::CreateDeserialized -- in which case you'd need all the Decl subclasses to befriend the creator, making this function unnecessary -- this is currently only going to be safe if ModulesLocalVisibility is enabled.)

rsmith: Setting this after creating a `Decl` is not safe in general; declarations with this flag set…
aprantlAuthorUnsubmitted
Done
ReplyInline Actions

I think the warning is unnecessary, since the storage is always allocated. Please let me know if I misunderstood something.

aprantl: I think the warning is unnecessary, since the storage is always allocated. Please let me know…
rsmithUnsubmitted
Not Done
ReplyInline Actions

The storage isn't necessarily allocated if the decl is created with ::Create rather than ::CreateDeserialized. Just changing the comment to say the declaration must have been created with CreateDeserialized would be fine.

rsmith: The storage isn't necessarily allocated if the decl is created with `::Create` rather than `…
/// Set the owning module ID. /// Set the owning module ID.
void setOwningModuleID(unsigned ID) { void setOwningModuleID(unsigned ID) {
assert(isFromASTFile() && "Only works on a deserialized declaration"); assert(isFromASTFile() && "Only works on a deserialized declaration");
*((unsigned*)this - 2) = ID; *((unsigned*)this - 2) = ID;
} }
public:
/// Determine the availability of the given declaration. /// Determine the availability of the given declaration.
/// ///
/// This routine will determine the most restrictive availability of /// This routine will determine the most restrictive availability of
/// the given declaration (e.g., preferring 'unavailable' to /// the given declaration (e.g., preferring 'unavailable' to
/// 'deprecated'). /// 'deprecated').
/// ///
/// \param Message If non-NULL and the result is not \c /// \param Message If non-NULL and the result is not \c
/// AR_Available, will be set to a (possibly empty) message /// AR_Available, will be set to a (possibly empty) message
▲ Show 20 Lines • Show All 91 Lines • Show Last 20 Lines