- User Since
- Mar 14 2013, 3:16 PM (401 w, 4 d)
This feels an awful lot like a set of attributes we already have -- can capability attributes be used for this instead? https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#basic-concepts-capabilities The documentation is largely centered around thread safety, but the gist of the idea seems similar to what you're proposing here -- certain parts of the program have different roles and you want to control how functions in one role can interact with functions in another role.
LGTM, but please wait for @njames93 in case they have additional feedback.
LGTM aside from a documentation request, thank you for this cleanup!
Fri, Nov 20
LGTM, thank you for the fixes!
Drive-by question from the peanut gallery, sorry if this is an ignorant one -- not all declarations have a trailing semicolon; is that handled properly? e.g., int x; has a trailing semicolon but int x, y; only has a trailing semicolon for one of the two declarations. Relatedly, in int f(int x);, the declaration of f has a trailing semicolon, but the declaration of x does not.
LGTM, thank you for the new check!
LGTM, thank you for the new documentation!
Thu, Nov 19
The request in D90180 was to not add a check for each kind of problematic semicolon situation but to instead make a single check that has configuration options for which semi colon situations to care about. The proposed changes here should be rolled into that unified check.
Thank you for the documentation effort!
LGTM, thank you for the new plugin example! Do you need me to commit on your behalf? If so, are you okay with Yafei Liu <email@example.com> for attribution?
Wed, Nov 18
LGTM aside from the testing request.
Thank you for the fix, I've commit on your behalf in 871fe71f2951cb19421a2b47ddb54ed6b3c8cba2
Tue, Nov 17
Thank you for working on this!
Can you mention this change in the release notes? Also, should we document it explicitly in the Language Extensions documentation, or do you think this is too tiny of a feature to warrant that?
LGTM, thank you!
LGTM, thank you!
LGTM aside from a testing request.
LGTM! Do you need me to commit on your behalf? If so, is Keishi Hattori <firstname.lastname@example.org> the correct attribution you'd like me to use?
Mon, Nov 16
Thank you for this, I think it's mostly looking good!
LGTM, thank you for the fix!
Thank you for the patch, I've commit on your behalf in 41b65f166b51760f77d0f9e465b3858f46e101f0
There are some uses of static_cast<std::underlying_type_t<ConstexprSpecKind>>() that I'm not certain really need that much complexity given that I doubt we'd ever change the underlying type of the enumeration away from being an int (or something which promotes to int). Do you think using underlying_type_t adds some extra value I'm not seeing? We seem to go both ways in this patch and should be consistent -- my weak preference is to cast to int for brevity and to reduce the template instantiation work when compiling Clang, but I don't insist.
LGTM! I'll land on your behalf momentarily.
LGTM! Would you like me to commit this one on your behalf or would you like to request your own commit access (https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access)?
Fri, Nov 13
LGTM aside from a place where I think we can remove a cast.
The attribute parts LGTM, but I did have a question about the libc++ parts.
Thu, Nov 12
I really like this attribute, thank you for working on this!