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

nonblocking/nonallocating attributes: 2nd pass caller/callee analysis #99656

Merged
merged 71 commits into from
Oct 3, 2024
Merged

nonblocking/nonallocating attributes: 2nd pass caller/callee analysis #99656

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
71 commits
Select commit Hold shift + click to select a range
8c5f854
nonblocking/nonallocating attributes: 2nd pass caller/callee analysis…
May 5, 2024
95b7a00
- Sema.h: Move function decls to be in the correct per-source-file se…
Jul 20, 2024
21c780a
clang-format
Jul 20, 2024
ff10413
- Detect ObjC @throw and @catch, diagnose identically to their C++ co…
Jul 26, 2024
d472964
FunctionEffect constructors: cast Kind to uint8_t
May 5, 2024
f1142db
New file: EFfectAnalysis.cpp.
Jul 26, 2024
efe1b93
CallableInfo doesn't need to cache the name.
Jul 26, 2024
d4f8dd5
Merge branch 'main' into nonblocking-part2
Jul 26, 2024
7ffdbef
Simpler bitmap implementation of FunctionEffectKindSet
Aug 2, 2024
c39e28e
- function does not permit inference of '%0': include the name of the…
Aug 2, 2024
dec1a61
Merge remote-tracking branch 'llvm-origin/main' into nonblocking-part2
Aug 5, 2024
06ca4c5
Add tests around lambda traversal and contexts like decltype, sizeof,…
Aug 7, 2024
9e45e6f
Inline checks preceding maybeAddDeclWithEffects
Aug 7, 2024
b99f784
Remove dead code in EffectAnalysis.cpp. Add comment about AST travers…
Aug 7, 2024
dbdd8f8
Merge remote-tracking branch 'llvm-origin/main' into nonblocking-part2
Aug 7, 2024
1b9874f
patch what the bot's clang-format wishes clang-format on my system wo…
Aug 7, 2024
b8818a3
Merge branch 'main' into nonblocking-part2
Aug 8, 2024
8390e69
"function cannot be inferred" -> "declaration cannot be inferred"
Aug 8, 2024
7acda8c
No need to override VisitCXXDefaultInitExpr.
Aug 8, 2024
bffacc5
Begin a list of unsafe builtin functions, starting with malloc and fr…
Aug 13, 2024
c718b5a
Apply suggestions from code review
dougsonos Aug 14, 2024
8b225f4
- FunctionEffect and FunctionEffectKindSet are tiny, pass by value wh…
Aug 14, 2024
2cb4539
- Comments begin with capital letters and end with full stops.
Aug 14, 2024
0e07315
Implement FunctionEffectKindSet with std::bitset.
Aug 14, 2024
47dfce8
Merge remote-tracking branch 'llvm-origin/main' into nonblocking-part2
Aug 14, 2024
fddd9d2
- Diagnose __builtin_operator_new and delete
Aug 14, 2024
eb536ab
EffectAnalysis.cpp => SemaFunctionEffects.cpp (minimal first step; mo…
Aug 15, 2024
15b399f
Move most Sema methods involving effects into SemaFunctionEffects.cpp.
Aug 15, 2024
dfebc1a
Move FunctionEffectDiff/Differences into SemaFunctionEffects.cpp.
Aug 16, 2024
82cb07d
Combine the diagnostics for 5 violations into one using %select{}
Aug 16, 2024
a7060ad
Merge branch 'main' into nonblocking-part2
Aug 16, 2024
f93ee01
Violation constructor: use optional instead of pointer.
Aug 16, 2024
6cc0a62
clang-format
Aug 16, 2024
69e1ae6
emitDiagnostics doesn't need to receive another S
Aug 16, 2024
076302e
VisitVarDecl can reuse followTypeDtor().
Aug 16, 2024
7b891c6
Add a test for a delegating initializer.
Aug 16, 2024
d1fcceb
Fix typo in comment for TraverseLambdaExpr.
Aug 16, 2024
abcf022
Fix test broken by rewording of diagnostics.
Aug 17, 2024
47ebf27
Diagnostics are now more specific about where a construct was found, …
Aug 17, 2024
6650c1f
Two fixes for default arguments:
Aug 20, 2024
250b80b
Type.cpp: remove asserts preceding llvm_unreachable().
Aug 20, 2024
e8bcd9f
Fix pre-C++20 compile error.
Aug 22, 2024
ea7f3fc
Review feedback: ViolationSite can use a PointerIntPair. Expand list …
Aug 29, 2024
d1a39e2
Fix: was traversing virtual base class destructors twice, exposed by …
Aug 29, 2024
75365ef
More builtin functions.
Aug 29, 2024
6aadec0
Apply suggestions from code review
dougsonos Sep 4, 2024
9b123a6
Review feedback:
Sep 4, 2024
ba57bfb
Fix: Don't try to follow a deleted destructor. (Happens with a std::o…
Sep 4, 2024
54feb05
Fix test broken by rewording of warnings.
Sep 5, 2024
93cb74e
Fix: need to ignore concept requirements.
Sep 5, 2024
d583c85
clang-format
Sep 5, 2024
cda1a9c
Make the FunctionEffects and PerfConstraintImpliesNoexcept diagnostic…
Sep 7, 2024
9c971c6
Add a release note.
Sep 7, 2024
b23e942
Merge branch 'main' into nonblocking-part2
Sep 8, 2024
9bfbe12
Fix botched format string in warn_func_effect_calls_expr_without_effect
Sep 9, 2024
f06909b
Fix test failures introduced by last commit.
Sep 17, 2024
adcfd14
If a function is noexcept and noreturn, exempt it from effect analysis.
Sep 18, 2024
17320b8
Remove the new diagnostics from `-Wall`. Fix a couple of sentences in…
Sep 23, 2024
909d7ff
Apply suggestions from code review
dougsonos Sep 23, 2024
9367103
Review feedback:
Sep 23, 2024
eb782e0
- Add constraint tests for plain C.
Sep 24, 2024
eccc7cf
-Wperf-constraint-implies-noexcept is part of -Wall again
Sep 24, 2024
220a1bf
Remove FunctionEffect::Kind::None as a sentinel; use optionals instead.
Sep 25, 2024
a8fef93
Ensure that setjmp() is not included in the special treatment of `nor…
Sep 26, 2024
092bc16
Update clang/lib/Sema/SemaFunctionEffects.cpp
dougsonos Sep 26, 2024
ca3f44a
Merge branch 'main' into nonblocking-part2
Sep 26, 2024
424a74b
Builtins with effects don't get the noexcept/noreturn special treatment.
Sep 26, 2024
273871a
warning-wall test was broken a few commits ago; fix it now.
Sep 26, 2024
a77d4e3
- FunctionEffect::Kind: no need to be specific with enum values. Same…
Sep 26, 2024
f8d9189
Tweak release note, fix typo in comment.
Sep 26, 2024
da725c1
Tweak: "function with 'nonblocking' attribute should be declared noex…
Oct 2, 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
23 changes: 9 additions & 14 deletions clang/include/clang/AST/Type.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4715,13 +4715,13 @@ class FunctionEffect {
public:
/// Identifies the particular effect.
enum class Kind : uint8_t {
None = 0,
NonBlocking = 1,
NonAllocating = 2,
Blocking = 3,
Allocating = 4,
NonBlocking = 0,
NonAllocating = 1,
Blocking = 2,
Allocating = 3,

Copy link
Collaborator

Choose a reason for hiding this comment

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

Would there be a value to an EndList sort of element, so you could use that to auto-generate KindCount?

Also, is there some meaning to the values of the enums besides just wanting the default behavior of enums?

Copy link
Contributor Author

@dougsonos dougsonos Sep 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Sure, I'll add and use EndList. Being explicit with enum values is an ancient habit from the days when debugging sometimes involved looking at memory in hex. I'd agree they're superfluous here.

Copy link
Collaborator

@erichkeane erichkeane Sep 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, we get around that in a few places I think... perhaps you can find what we do in other enums? Not at a computer at the moment, else I'd help search.

EDIT TO ADD: It might be "EndList = LastEntry" ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Last seems to be the most common name for that last value...

enum class ComparisonCategoryType : unsigned char {
  PartialOrdering,
  WeakOrdering,
  StrongOrdering,
  First = PartialOrdering,
  Last = StrongOrdering
};

enum class Lang { C = 0, CXX, LastValue = CXX };

enum class SyncScope {
  SystemScope,
  DeviceScope,
  WorkgroupScope,
  WavefrontScope,
  SingleScope,
  HIPSingleThread,
  HIPWavefront,
  HIPWorkgroup,
  HIPAgent,
  HIPSystem,
  OpenCLWorkGroup,
  OpenCLDevice,
  OpenCLAllSVMDevices,
  OpenCLSubGroup,
  Last = OpenCLSubGroup

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, yes, we do use it quite a bit, you can also see the = , so just do that and use it kinda thing IMO.

};
constexpr static size_t KindMaximum = 4;
constexpr static size_t KindCount = 4;

/// Flags describing some behaviors of the effect.
using Flags = unsigned;
Expand All @@ -4747,8 +4747,6 @@ class FunctionEffect {
// be considered for uniqueness.

public:
FunctionEffect() : FKind(Kind::None) {}

explicit FunctionEffect(Kind K) : FKind(K) {}

/// The kind of the effect.
Expand Down Expand Up @@ -4777,8 +4775,6 @@ class FunctionEffect {
case Kind::Blocking:
case Kind::Allocating:
return 0;
case Kind::None:
break;
}
llvm_unreachable("unknown effect kind");
}
Expand Down Expand Up @@ -4843,7 +4839,6 @@ struct FunctionEffectWithCondition {
FunctionEffect Effect;
EffectConditionExpr Cond;

FunctionEffectWithCondition() = default;
FunctionEffectWithCondition(FunctionEffect E, const EffectConditionExpr &C)
: Effect(E), Cond(C) {}

Expand Down Expand Up @@ -4959,7 +4954,7 @@ class FunctionEffectsRef {
/// A mutable set of FunctionEffect::Kind.
class FunctionEffectKindSet {
Sirraide marked this conversation as resolved.
Show resolved Hide resolved
// For now this only needs to be a bitmap.
constexpr static size_t EndBitPos = FunctionEffect::KindMaximum;
constexpr static size_t EndBitPos = FunctionEffect::KindCount;
using KindBitsT = std::bitset<EndBitPos>;

KindBitsT KindBits{};
Expand All @@ -4970,11 +4965,11 @@ class FunctionEffectKindSet {
// position in the bitset.

constexpr static size_t kindToPos(FunctionEffect::Kind K) {
return static_cast<size_t>(K) - 1;
return static_cast<size_t>(K);
}

constexpr static FunctionEffect::Kind posToKind(size_t Pos) {
return static_cast<FunctionEffect::Kind>(Pos + 1);
return static_cast<FunctionEffect::Kind>(Pos);
}

// Iterates through the bits which are set.
Expand Down
12 changes: 7 additions & 5 deletions clang/include/clang/Sema/Sema.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15028,13 +15028,15 @@ class Sema final : public SemaBase {

FunctionEffect::Kind EffectKind;
Kind DiffKind;
FunctionEffectWithCondition Old; // Invalid when 'Kind' is 'Added'.
FunctionEffectWithCondition New; // Invalid when 'Kind' is 'Removed'.
std::optional<FunctionEffectWithCondition>
Old; // Invalid when 'Kind' is 'Added'.
std::optional<FunctionEffectWithCondition>
New; // Invalid when 'Kind' is 'Removed'.

StringRef effectName() const {
if (Old.Effect.kind() != FunctionEffect::Kind::None)
return Old.Effect.name();
return New.Effect.name();
if (Old)
return Old.value().Effect.name();
return New.value().Effect.name();
}

/// Describes the result of effects differing between a base class's virtual
Expand Down
13 changes: 2 additions & 11 deletions clang/lib/AST/Type.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5117,8 +5117,6 @@ FunctionEffect::Kind FunctionEffect::oppositeKind() const {
return Kind::Allocating;
case Kind::Allocating:
return Kind::NonAllocating;
case Kind::None:
return Kind::None;
}
llvm_unreachable("unknown effect kind");
}
Expand All @@ -5133,8 +5131,6 @@ StringRef FunctionEffect::name() const {
return "blocking";
case Kind::Allocating:
return "allocating";
case Kind::None:
return "(none)";
}
llvm_unreachable("unknown effect kind");
}
Expand All @@ -5159,11 +5155,8 @@ std::optional<FunctionEffect> FunctionEffect::effectProhibitingInference(
case Kind::Blocking:
assert(0 && "effectProhibitingInference with non-inferable effect kind");
Sirraide marked this conversation as resolved.
Show resolved Hide resolved
break;

case Kind::None:
break;
}
llvm_unreachable("unknown effect kind or None");
llvm_unreachable("unknown effect kind");
}

bool FunctionEffect::shouldDiagnoseFunctionCall(
Expand All @@ -5185,10 +5178,8 @@ bool FunctionEffect::shouldDiagnoseFunctionCall(
case Kind::Allocating:
case Kind::Blocking:
return false;
case Kind::None:
break;
}
llvm_unreachable("unknown effect kind or None");
llvm_unreachable("unknown effect kind");
}

// =====
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Sema/SemaDeclCXX.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18192,7 +18192,7 @@ bool Sema::CheckOverridingFunctionAttributes(CXXMethodDecl *New,
<< Old->getReturnTypeSourceRange();
break;
case FunctionEffectDiff::OverrideResult::Merge: {
NewFX.insert(Diff.Old, Errs);
NewFX.insert(Diff.Old.value(), Errs);
const auto *NewFT = New->getType()->castAs<FunctionProtoType>();
FunctionProtoType::ExtProtoInfo EPI = NewFT->getExtProtoInfo();
EPI.FunctionEffects = FunctionEffectsRef(NewFX);
Expand Down
28 changes: 10 additions & 18 deletions clang/lib/Sema/SemaFunctionEffects.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,22 +78,19 @@ class ViolationSite {
// be inferred as holding that effect.
struct Violation {
FunctionEffect Effect;
FunctionEffect
std::optional<FunctionEffect>
CalleeEffectPreventingInference; // Only for certain IDs; can be None.
dougsonos marked this conversation as resolved.
Show resolved Hide resolved
ViolationID ID = ViolationID::None;
ViolationSite Site;
SourceLocation Loc;
const Decl *Callee =
nullptr; // Only valid for ViolationIDs Calls{Decl,Expr}WithoutEffect.

Violation() = default;

Violation(FunctionEffect Effect, ViolationID ID, ViolationSite VS,
SourceLocation Loc, const Decl *Callee = nullptr,
std::optional<FunctionEffect> CalleeEffect = std::nullopt)
: Effect(Effect), CalleeEffectPreventingInference(
CalleeEffect.value_or(FunctionEffect())),
ID(ID), Site(VS), Loc(Loc), Callee(Callee) {}
: Effect(Effect), CalleeEffectPreventingInference(CalleeEffect), ID(ID),
Site(VS), Loc(Loc), Callee(Callee) {}

unsigned diagnosticSelectIndex() const {
return unsigned(ID) - unsigned(ViolationID::BaseDiagnosticIndex);
Expand Down Expand Up @@ -915,7 +912,7 @@ class Analyzer {
case ViolationID::DeclDisallowsInference:
S.Diag(Viol2.Loc, diag::note_func_effect_call_disallows_inference)
<< SiteDescIndex(CalleeInfo.CDecl, nullptr) << effectName
<< Viol2.CalleeEffectPreventingInference.name();
<< Viol2.CalleeEffectPreventingInference->name();
break;
case ViolationID::CallsExprWithoutEffect:
S.Diag(Viol2.Loc, diag::note_func_effect_call_indirect)
Expand Down Expand Up @@ -1458,14 +1455,16 @@ Sema::FunctionEffectDiffVector::FunctionEffectDiffVector(
if (cmp < 0) {
// removal
FunctionEffectWithCondition Old = *POld;
push_back(FunctionEffectDiff{
Old.Effect.kind(), FunctionEffectDiff::Kind::Removed, Old, {}});
push_back(FunctionEffectDiff{Old.Effect.kind(),
FunctionEffectDiff::Kind::Removed, Old,
std::nullopt});
++POld;
} else if (cmp > 0) {
// addition
FunctionEffectWithCondition New = *PNew;
push_back(FunctionEffectDiff{
New.Effect.kind(), FunctionEffectDiff::Kind::Added, {}, New});
push_back(FunctionEffectDiff{New.Effect.kind(),
FunctionEffectDiff::Kind::Added,
std::nullopt, New});
++PNew;
} else {
++POld;
Expand Down Expand Up @@ -1506,8 +1505,6 @@ bool Sema::FunctionEffectDiff::shouldDiagnoseConversion(
case FunctionEffect::Kind::Blocking:
case FunctionEffect::Kind::Allocating:
return false;
case FunctionEffect::Kind::None:
break;
}
llvm_unreachable("unknown effect kind");
}
Expand All @@ -1531,8 +1528,6 @@ bool Sema::FunctionEffectDiff::shouldDiagnoseRedeclaration(
case FunctionEffect::Kind::Blocking:
case FunctionEffect::Kind::Allocating:
return false;
case FunctionEffect::Kind::None:
break;
}
llvm_unreachable("unknown effect kind");
}
Expand Down Expand Up @@ -1563,9 +1558,6 @@ Sema::FunctionEffectDiff::shouldDiagnoseMethodOverride(
case FunctionEffect::Kind::Blocking:
case FunctionEffect::Kind::Allocating:
return OverrideResult::NoAction;

case FunctionEffect::Kind::None:
break;
}
llvm_unreachable("unknown effect kind");
}
Expand Down