Skip to content

WIP: Extend SourceLocation to 64 bits. #146314

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

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -127,12 +127,12 @@ struct CognitiveComplexity final {
// https://sonarcloud.io/projects?languages=c%2Ccpp&size=5 we can estimate:
// value ~20 would result in no allocs for 98% of functions, ~12 for 96%, ~10
// for 91%, ~8 for 88%, ~6 for 84%, ~4 for 77%, ~2 for 64%, and ~1 for 37%.
static_assert(sizeof(Detail) <= 8,
static_assert(sizeof(Detail) <= 16,
"Since we use SmallVector to minimize the amount of "
"allocations, we also need to consider the price we pay for "
"that in terms of stack usage. "
"Thus, it is good to minimize the size of the Detail struct.");
SmallVector<Detail, DefaultLimit> Details; // 25 elements is 200 bytes.
SmallVector<Detail, DefaultLimit> Details; // 25 elements is 400 bytes.
// Yes, 25 is a magic number. This is the seemingly-sane default for the
// upper limit for function cognitive complexity. Thus it would make sense
// to avoid allocations for any function that does not violate the limit.
Expand Down
4 changes: 3 additions & 1 deletion clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -829,7 +829,9 @@ TEST(SourceCodeTests, isSpelledInSource) {
// FIXME: Should it return false on SourceLocation()? Does it matter?
EXPECT_TRUE(isSpelledInSource(SourceLocation(), SM));
EXPECT_FALSE(isSpelledInSource(
SourceLocation::getFromRawEncoding(SourceLocation::UIntTy(1 << 31)), SM));
SourceLocation::getFromRawEncoding(
SourceLocation::UIntTy(1ULL << (SourceLocation::Bits - 1))),
SM));
}

struct IncrementalTestStep {
Expand Down
2 changes: 1 addition & 1 deletion clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ class Reporter {
// Points within the main file that reference a Symbol.
// Implicit refs will be marked with a symbol just before the token.
struct Ref {
unsigned Offset;
uint64_t Offset;
RefType Type;
Symbol Sym;
SmallVector<SymbolLocation> Locations = {};
Expand Down
2 changes: 2 additions & 0 deletions clang/include/clang/AST/ASTContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -3356,6 +3356,8 @@ class ASTContext : public RefCountedBase<ASTContext> {
getTrivialTypeSourceInfo(QualType T,
SourceLocation Loc = SourceLocation()) const;

CXXOperatorSourceInfo *getCXXOperatorSourceInfo(SourceRange R) const;

/// Add a deallocation callback that will be invoked when the
/// ASTContext is destroyed.
///
Expand Down
8 changes: 1 addition & 7 deletions clang/include/clang/AST/DeclBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -1953,16 +1953,10 @@ class DeclContext {
/// For the bits in DeclContextBitfields
LLVM_PREFERRED_TYPE(DeclContextBitfields)
uint32_t : NumDeclContextBits;

// Not a bitfield but this saves space.
// Note that ObjCContainerDeclBitfields is full.
SourceLocation AtStart;
};

/// Number of inherited and non-inherited bits in ObjCContainerDeclBitfields.
/// Note that here we rely on the fact that SourceLocation is 32 bits
/// wide. We check this with the static_assert in the ctor of DeclContext.
enum { NumObjCContainerDeclBits = 64 };
enum { NumObjCContainerDeclBits = NumDeclContextBits };

/// Stores the bits used by LinkageSpecDecl.
/// If modified NumLinkageSpecDeclBits and the accessor
Expand Down
5 changes: 3 additions & 2 deletions clang/include/clang/AST/DeclObjC.h
Original file line number Diff line number Diff line change
Expand Up @@ -946,6 +946,7 @@ class ObjCContainerDecl : public NamedDecl, public DeclContext {
// This class stores some data in DeclContext::ObjCContainerDeclBits
// to save some space. Use the provided accessors to access it.

SourceLocation AtStart;
// These two locations in the range mark the end of the method container.
// The first points to the '@' token, and the second to the 'end' token.
SourceRange AtEnd;
Expand Down Expand Up @@ -1090,10 +1091,10 @@ class ObjCContainerDecl : public NamedDecl, public DeclContext {
/// Note, the superclass's properties are not included in the list.
virtual void collectPropertiesToImplement(PropertyMap &PM) const {}

SourceLocation getAtStartLoc() const { return ObjCContainerDeclBits.AtStart; }
SourceLocation getAtStartLoc() const { return AtStart; }

void setAtStartLoc(SourceLocation Loc) {
ObjCContainerDeclBits.AtStart = Loc;
AtStart = Loc;
}

// Marks the end of the container.
Expand Down
51 changes: 25 additions & 26 deletions clang/include/clang/AST/DeclarationName.h
Original file line number Diff line number Diff line change
Expand Up @@ -682,6 +682,11 @@ class DeclarationNameTable {
DeclarationName getCXXLiteralOperatorName(const IdentifierInfo *II);
};

struct CXXOperatorSourceInfo {
SourceLocation BeginOpNameLoc;
SourceLocation EndOpNameLoc;
};

/// DeclarationNameLoc - Additional source/type location info
/// for a declaration name. Needs a DeclarationName in order
/// to be interpreted correctly.
Expand All @@ -698,8 +703,7 @@ class DeclarationNameLoc {

// The location (if any) of the operator keyword is stored elsewhere.
struct CXXOpName {
SourceLocation BeginOpNameLoc;
SourceLocation EndOpNameLoc;
CXXOperatorSourceInfo* OInfo;
Copy link
Collaborator

Choose a reason for hiding this comment

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

How much does this indirection save perf wise?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't see the performance impact on this indirection -- this is for the C++ operation call, which is rare in practice (the perf here https://llvm-compile-time-tracker.com/compare.php?from=ff5a67315305f59f91041bad8b905e161b872442&to=38c519de69b127faa823078d3faff5670bc60209&stat=instructions:u)

};

// The location (if any) of the operator keyword is stored elsewhere.
Expand All @@ -719,11 +723,6 @@ class DeclarationNameLoc {

void setNamedTypeLoc(TypeSourceInfo *TInfo) { NamedType.TInfo = TInfo; }

void setCXXOperatorNameRange(SourceRange Range) {
CXXOperatorName.BeginOpNameLoc = Range.getBegin();
CXXOperatorName.EndOpNameLoc = Range.getEnd();
}

void setCXXLiteralOperatorNameLoc(SourceLocation Loc) {
CXXLiteralOperatorName.OpNameLoc = Loc;
}
Expand All @@ -739,12 +738,17 @@ class DeclarationNameLoc {

/// Return the beginning location of the getCXXOperatorNameRange() range.
SourceLocation getCXXOperatorNameBeginLoc() const {
return CXXOperatorName.BeginOpNameLoc;
if (!CXXOperatorName.OInfo)
return {};
return
CXXOperatorName.OInfo->BeginOpNameLoc;
}

/// Return the end location of the getCXXOperatorNameRange() range.
SourceLocation getCXXOperatorNameEndLoc() const {
return CXXOperatorName.EndOpNameLoc;
if (!CXXOperatorName.OInfo)
return {};
return CXXOperatorName.OInfo->EndOpNameLoc;
}

/// Return the range of the operator name (without the operator keyword).
Expand All @@ -769,17 +773,12 @@ class DeclarationNameLoc {
DNL.setNamedTypeLoc(TInfo);
return DNL;
}

/// Construct location information for a non-literal C++ operator.
static DeclarationNameLoc makeCXXOperatorNameLoc(SourceLocation BeginLoc,
SourceLocation EndLoc) {
return makeCXXOperatorNameLoc(SourceRange(BeginLoc, EndLoc));
}


/// Construct location information for a non-literal C++ operator.
static DeclarationNameLoc makeCXXOperatorNameLoc(SourceRange Range) {
static DeclarationNameLoc
makeCXXOperatorNameLoc(CXXOperatorSourceInfo *OInfo) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a weird function to exist... am I correct in that it is only called 1x? I find myself thinkin perhaps we should just inline it manually.

DeclarationNameLoc DNL;
DNL.setCXXOperatorNameRange(Range);
DNL.CXXOperatorName.OInfo = OInfo;
return DNL;
}

Expand Down Expand Up @@ -839,7 +838,7 @@ struct DeclarationNameInfo {
return nullptr;
return LocInfo.getNamedTypeInfo();
}

/// setNamedTypeInfo - Sets the source type info associated to
/// the name. Assumes it is a constructor, destructor or conversion.
void setNamedTypeInfo(TypeSourceInfo *TInfo) {
Expand All @@ -849,6 +848,13 @@ struct DeclarationNameInfo {
LocInfo = DeclarationNameLoc::makeNamedTypeLoc(TInfo);
}

/// Sets the range of the operator name (without the operator keyword).
/// Assumes it is a C++ operator.
void setCXXOperatorNameInfo(CXXOperatorSourceInfo *OInfo) {
assert(Name.getNameKind() == DeclarationName::CXXOperatorName);
LocInfo = DeclarationNameLoc::makeCXXOperatorNameLoc(OInfo);
}

/// getCXXOperatorNameRange - Gets the range of the operator name
/// (without the operator keyword). Assumes it is a (non-literal) operator.
SourceRange getCXXOperatorNameRange() const {
Expand All @@ -857,13 +863,6 @@ struct DeclarationNameInfo {
return LocInfo.getCXXOperatorNameRange();
}

/// setCXXOperatorNameRange - Sets the range of the operator name
/// (without the operator keyword). Assumes it is a C++ operator.
void setCXXOperatorNameRange(SourceRange R) {
assert(Name.getNameKind() == DeclarationName::CXXOperatorName);
LocInfo = DeclarationNameLoc::makeCXXOperatorNameLoc(R);
}

/// getCXXLiteralOperatorNameLoc - Returns the location of the literal
/// operator name (not the operator keyword).
/// Assumes it is a literal operator.
Expand Down
Loading
Loading