-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[NFC][Clang] Use StringSwitch instead of array for parsing attribute scope #115414
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
Conversation
…scope Change-Id: Iacfabcfd3a1cedeb7c864b3b59bad88b2177e7a8
|
@llvm/pr-subscribers-clang Author: Chinmay Deshpande (chinmaydd) ChangesFull diff: https://github.com/llvm/llvm-project/pull/115414.diff 2 Files Affected:
diff --git a/clang/include/clang/Basic/AttributeCommonInfo.h b/clang/include/clang/Basic/AttributeCommonInfo.h
index 11c64547721739..350154859ccdec 100644
--- a/clang/include/clang/Basic/AttributeCommonInfo.h
+++ b/clang/include/clang/Basic/AttributeCommonInfo.h
@@ -67,7 +67,7 @@ class AttributeCommonInfo {
IgnoredAttribute,
UnknownAttribute,
};
- enum class Scope { NONE, CLANG, GNU, MSVC, OMP, HLSL, GSL, RISCV };
+ enum class Scope { NONE, CLANG, GNU, MSVC, OMP, HLSL, GSL, RISCV, INVALID };
private:
const IdentifierInfo *AttrName = nullptr;
diff --git a/clang/lib/Basic/Attributes.cpp b/clang/lib/Basic/Attributes.cpp
index 2d18fb3f9d5bb2..a2ffaa147f3ef7 100644
--- a/clang/lib/Basic/Attributes.cpp
+++ b/clang/lib/Basic/Attributes.cpp
@@ -18,6 +18,7 @@
#include "clang/Basic/TargetInfo.h"
#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringSwitch.h"
using namespace clang;
@@ -155,26 +156,23 @@ std::string AttributeCommonInfo::getNormalizedFullName() const {
normalizeName(getAttrName(), getScopeName(), getSyntax()));
}
-// Sorted list of attribute scope names
-static constexpr std::pair<StringRef, AttributeCommonInfo::Scope> ScopeList[] =
- {{"", AttributeCommonInfo::Scope::NONE},
- {"clang", AttributeCommonInfo::Scope::CLANG},
- {"gnu", AttributeCommonInfo::Scope::GNU},
- {"gsl", AttributeCommonInfo::Scope::GSL},
- {"hlsl", AttributeCommonInfo::Scope::HLSL},
- {"msvc", AttributeCommonInfo::Scope::MSVC},
- {"omp", AttributeCommonInfo::Scope::OMP},
- {"riscv", AttributeCommonInfo::Scope::RISCV}};
-
AttributeCommonInfo::Scope
getScopeFromNormalizedScopeName(StringRef ScopeName) {
- auto It = std::lower_bound(
- std::begin(ScopeList), std::end(ScopeList), ScopeName,
- [](const std::pair<StringRef, AttributeCommonInfo::Scope> &Element,
- StringRef Value) { return Element.first < Value; });
- assert(It != std::end(ScopeList) && It->first == ScopeName);
-
- return It->second;
+ AttributeCommonInfo::Scope ParsedScope =
+ llvm::StringSwitch<AttributeCommonInfo::Scope>(ScopeName)
+ .Case("", AttributeCommonInfo::Scope::NONE)
+ .Case("clang", AttributeCommonInfo::Scope::CLANG)
+ .Case("gnu", AttributeCommonInfo::Scope::GNU)
+ .Case("gsl", AttributeCommonInfo::Scope::GSL)
+ .Case("hlsl", AttributeCommonInfo::Scope::HLSL)
+ .Case("msvc", AttributeCommonInfo::Scope::MSVC)
+ .Case("omp", AttributeCommonInfo::Scope::OMP)
+ .Case("riscv", AttributeCommonInfo::Scope::RISCV)
+ .Default(AttributeCommonInfo::Scope::INVALID);
+
+ assert(ParsedScope != AttributeCommonInfo::Scope::INVALID);
+
+ return ParsedScope;
}
unsigned AttributeCommonInfo::calculateAttributeSpellingListIndex() const {
|
nikic
left a comment
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.
LGTM
erichkeane
left a comment
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.
1 nit, else LGTM.
clang/lib/Basic/Attributes.cpp
Outdated
| .Case("riscv", AttributeCommonInfo::Scope::RISCV) | ||
| .Default(AttributeCommonInfo::Scope::INVALID); | ||
|
|
||
| assert(ParsedScope != AttributeCommonInfo::Scope::INVALID); |
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.
Remove .Default and the assert. The operator R has an assert in it to do this for you.
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.
Thanks @erichkeane and @nikic. This looks quite clean now :)
f90efc1 to
c70b565
Compare
Change-Id: I99a618d8127680cb91146e1a82e0e0b6c0717888
c70b565 to
a64632f
Compare
No description provided.