-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[sil] Add an implicit operator bool conversion to SILDebugLocation. #33161
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
Changes from all commits
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 |
---|---|---|
|
@@ -755,17 +755,18 @@ class SILDebugScope; | |
|
||
/// A SILLocation paired with a SILDebugScope. | ||
class SILDebugLocation { | ||
const SILDebugScope *Scope = nullptr; | ||
SILLocation Location; | ||
const SILDebugScope *debugScope; | ||
SILLocation location; | ||
|
||
public: | ||
SILDebugLocation() | ||
: Scope(nullptr), | ||
Location(RegularLocation::getAutoGeneratedLocation()) {} | ||
SILDebugLocation(SILLocation Loc, const SILDebugScope *DS) | ||
: Scope(DS), Location(Loc) {} | ||
SILLocation getLocation() const { return Location; } | ||
const SILDebugScope *getScope() const { return Scope; } | ||
: debugScope(nullptr), | ||
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 prefer the = nullptr spelling, since it makes it harder to forget to initialize the member when adding a new constructor. 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. Ok. To me it just looked redundant. I am fine fixing it in a follow on commit. |
||
location(RegularLocation::getAutoGeneratedLocation()) {} | ||
SILDebugLocation(SILLocation location, const SILDebugScope *debugScope) | ||
: debugScope(debugScope), location(location) {} | ||
SILLocation getLocation() const { return location; } | ||
const SILDebugScope *getScope() const { return debugScope; } | ||
operator bool() const { return bool(location) && debugScope; } | ||
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. This part LGTM. |
||
}; | ||
|
||
} // end swift namespace | ||
|
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.
The original code used the LLVM naming convention. Do we now have a Swift naming convention that's different from the LLVM one?
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.
Yes. Swift does camelCase for variables. We are slowly changing over time.
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.
And LLVM is changing more to that style as well