Il 02/07/2012 18:16, Jordan Rose ha scritto:
>
> On Jul 2, 2012, at 8:22 AM, Dmitri Gribenko wrote:
>
>> On Sat, Jun 30, 2012 at 11:08 AM, Jordan Rose <[email protected]> wrote:
>>> On Jun 30, 2012, at 2:31 AM, Dmitri Gribenko wrote:
>>>> On Sat, Jun 30, 2012 at 12:55 AM, Enea Zaffanella
>>>> <[email protected]> wrote:
>>>>> In include/clang/AST/RawCommentList.h we have
>>>>>
>>>>> class RawComment {
>>>>> public:
>>>>> enum CommentKind {
>>>>> CK_Invalid, ///< Invalid comment
>>>>> [...]
>>>>>
>>>>> In include/clang/AST/OperationKinds.h the following macro is defined
>>>>>
>>>>> #define CK_Invalid ((CastKind) -1)
>>>>
>>>> Hi Enea,
>>>>
>>>> Thank you for noticing!
>>>>
>>>> Here is a patch that should fix this.
>>>>
>>>> Please review.
>>>>
>>>> Dmitri
>>>
>>> It's possible the reason this isn't used is because CastKind would
>>> otherwise be only positive numbers and thus suitable for storing in an
>>> unsigned bitfield without complaint. Maybe ~0U would be a better choice of
>>> constant?
>>
>> Is there any reason why CK_Invalid has an all-ones value? (We could
>> add it to enum without an initializer)
>
> I think it's just so it will never intersect with a valid value. If these
> constants are exposed in libclang, they're expected to remain stable. Then
> again, CK_Invalid probably may not be exposed in libclang even if the others
> are.
>
> I agree with Enea that it's probably not in the enum so that exhaustive
> switch statements can ignore it -- you should never actually be consuming a
> cast with CK_Invalid type. And in general I think I agree that we should try
> to have a different prefix for comment kinds: RC_ or RCK_, since both comment
> kinds and cast kinds /could/ show up in the same section of code.
I've attached a patch to do that. If you don't have any objection I'll
apply it.
Index: include/clang/AST/OperationKinds.h
===================================================================
--- include/clang/AST/OperationKinds.h (revision 159647)
+++ include/clang/AST/OperationKinds.h (working copy)
@@ -291,7 +291,7 @@
CK_CopyAndAutoreleaseBlockObject
};
-#define CK_Invalid ((CastKind) -1)
+static const CastKind CK_Invalid = static_cast<CastKind>(-1);
enum BinaryOperatorKind {
// Operators listed in order of precedence.
Index: include/clang/AST/RawCommentList.h
===================================================================
--- include/clang/AST/RawCommentList.h (revision 159647)
+++ include/clang/AST/RawCommentList.h (working copy)
@@ -21,17 +21,17 @@
class RawComment {
public:
enum CommentKind {
- CK_Invalid, ///< Invalid comment
- CK_OrdinaryBCPL, ///< Any normal BCPL comments
- CK_OrdinaryC, ///< Any normal C comment
- CK_BCPLSlash, ///< \code /// stuff \endcode
- CK_BCPLExcl, ///< \code //! stuff \endcode
- CK_JavaDoc, ///< \code /** stuff */ \endcode
- CK_Qt, ///< \code /*! stuff */ \endcode, also used by HeaderDoc
- CK_Merged ///< Two or more documentation comments merged together
+ RCK_Invalid, ///< Invalid comment
+ RCK_OrdinaryBCPL, ///< Any normal BCPL comments
+ RCK_OrdinaryC, ///< Any normal C comment
+ RCK_BCPLSlash, ///< \code /// stuff \endcode
+ RCK_BCPLExcl, ///< \code //! stuff \endcode
+ RCK_JavaDoc, ///< \code /** stuff */ \endcode
+ RCK_Qt, ///< \code /*! stuff */ \endcode, also used by HeaderDoc
+ RCK_Merged ///< Two or more documentation comments merged together
};
- RawComment() : Kind(CK_Invalid), IsAlmostTrailingComment(false) { }
+ RawComment() : Kind(RCK_Invalid), IsAlmostTrailingComment(false) { }
RawComment(const SourceManager &SourceMgr, SourceRange SR,
bool Merged = false);
@@ -41,11 +41,11 @@
}
bool isInvalid() const LLVM_READONLY {
- return Kind == CK_Invalid;
+ return Kind == RCK_Invalid;
}
bool isMerged() const LLVM_READONLY {
- return Kind == CK_Merged;
+ return Kind == RCK_Merged;
}
/// Returns true if it is a comment that should be put after a member:
@@ -67,7 +67,7 @@
/// Returns true if this comment is not a documentation comment.
bool isOrdinary() const LLVM_READONLY {
- return (Kind == CK_OrdinaryBCPL) || (Kind == CK_OrdinaryC);
+ return (Kind == RCK_OrdinaryBCPL) || (Kind == RCK_OrdinaryC);
}
/// Returns true if this comment any kind of a documentation comment.
Index: lib/Sema/Sema.cpp
===================================================================
--- lib/Sema/Sema.cpp (revision 159647)
+++ lib/Sema/Sema.cpp (working copy)
@@ -1029,10 +1029,10 @@
Comment.getBegin().getLocWithOffset(3));
StringRef MagicMarkerText;
switch (RC.getKind()) {
- case RawComment::CK_OrdinaryBCPL:
+ case RawComment::RCK_OrdinaryBCPL:
MagicMarkerText = "///<";
break;
- case RawComment::CK_OrdinaryC:
+ case RawComment::RCK_OrdinaryC:
MagicMarkerText = "/**<";
break;
default:
Index: lib/AST/RawCommentList.cpp
===================================================================
--- lib/AST/RawCommentList.cpp (revision 159647)
+++ lib/AST/RawCommentList.cpp (working copy)
@@ -19,19 +19,19 @@
/// Get comment kind and bool describing if it is a trailing comment.
std::pair<RawComment::CommentKind, bool> getCommentKind(StringRef Comment) {
if (Comment.size() < 3 || Comment[0] != '/')
- return std::make_pair(RawComment::CK_Invalid, false);
+ return std::make_pair(RawComment::RCK_Invalid, false);
RawComment::CommentKind K;
if (Comment[1] == '/') {
if (Comment.size() < 3)
- return std::make_pair(RawComment::CK_OrdinaryBCPL, false);
+ return std::make_pair(RawComment::RCK_OrdinaryBCPL, false);
if (Comment[2] == '/')
- K = RawComment::CK_BCPLSlash;
+ K = RawComment::RCK_BCPLSlash;
else if (Comment[2] == '!')
- K = RawComment::CK_BCPLExcl;
+ K = RawComment::RCK_BCPLExcl;
else
- return std::make_pair(RawComment::CK_OrdinaryBCPL, false);
+ return std::make_pair(RawComment::RCK_OrdinaryBCPL, false);
} else {
assert(Comment.size() >= 4);
@@ -40,14 +40,14 @@
if (Comment[1] != '*' ||
Comment[Comment.size() - 2] != '*' ||
Comment[Comment.size() - 1] != '/')
- return std::make_pair(RawComment::CK_Invalid, false);
+ return std::make_pair(RawComment::RCK_Invalid, false);
if (Comment[2] == '*')
- K = RawComment::CK_JavaDoc;
+ K = RawComment::RCK_JavaDoc;
else if (Comment[2] == '!')
- K = RawComment::CK_Qt;
+ K = RawComment::RCK_Qt;
else
- return std::make_pair(RawComment::CK_OrdinaryC, false);
+ return std::make_pair(RawComment::RCK_OrdinaryC, false);
}
const bool TrailingComment = (Comment.size() > 3) && (Comment[3] == '<');
return std::make_pair(K, TrailingComment);
@@ -65,7 +65,7 @@
BeginLineValid(false), EndLineValid(false) {
// Extract raw comment text, if possible.
if (SR.getBegin() == SR.getEnd() || getRawText(SourceMgr).empty()) {
- Kind = CK_Invalid;
+ Kind = RCK_Invalid;
return;
}
@@ -78,7 +78,7 @@
IsAlmostTrailingComment = RawText.startswith("//<") ||
RawText.startswith("/*<");
} else {
- Kind = CK_Merged;
+ Kind = RCK_Merged;
IsTrailingComment = mergedCommentIsTrailingComment(RawText);
}
}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
