[PATCH] D85287: Extend -Wtautological-bitwise-compare "bitwise or with non-zero value" warnings

jordan_rose added a comment. Nothing seems too mysterious here, but why limit to `constexpr` instead of anything where Clang can see a constant value, which would make it work in C as well? (If you're allowing `constexpr`, you're already allowing arbitrary work.) CHANGES SINCE LAST ACTION

[PATCH] D68846: Do the bare minimum to get ClangdXPC.framework building with CMake's Xcode generator

jordan_rose closed this revision. jordan_rose added a comment. Committed in rCTE374494 . Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68846/new/ https://reviews.llvm.org/D68846

[PATCH] D68846: Do the bare minimum to get ClangdXPC.framework building with CMake's Xcode generator

jordan_rose created this revision. jordan_rose added reviewers: beanz, jkorous. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, dexonsmith, MaskRay, ilya-biryukov, mgorny. Herald added a project: clang. The output directories for CMake's Xcode project generator are specific

[PATCH] D68430: Don't use object libraries with Xcode

jordan_rose closed this revision. jordan_rose added a comment. Committed as rC373769 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68430/new/ https://reviews.llvm.org/D68430

[PATCH] D68430: Don't use object libraries with Xcode

jordan_rose updated this revision to Diff 223250. jordan_rose added a comment. Okay, having Xcode force-load the static libraries doesn't seem bad at all. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68430/new/ https://reviews.llvm.org/D68430

[PATCH] D68430: Don't use object libraries with Xcode

jordan_rose added a comment. In D68430#1693965 , @beanz wrote: > clang_cpp can't link the libraries "normally" because it has no unresolved > symbols to force the contents of the libraries to link. I don't like it, but > I think the best option is to

[PATCH] D68430: Don't use object libraries with Xcode

jordan_rose added a comment. I'm not quite sure //what// it's doing. The executable targets end up trying to link against the static libraries anyway, which of course haven't been built. It's possible that this is because the LIBTYPE is both STATIC and OBJECT and if it were just OBJECT we

[PATCH] D68430: Don't use object libraries with Xcode

jordan_rose created this revision. jordan_rose added a reviewer: beanz. Herald added subscribers: cfe-commits, mgorny. Herald added a project: clang. Undoes some of the effects of D61909 when using the Xcode CMake generator—it doesn't handle object libraries

[PATCH] D65256: [Sema][ObjC] Mark C union fields that have non-trivial ObjC ownership qualifications as unavailable if the union is declared in a system header

jordan_rose added a comment. In D65256#1601509 , @rjmccall wrote: > Sorry, am I missing something? Such a union would've been either ill-formed > or unavailable in ARC (depending on where it was declared) before this recent > work. Apparently that

[PATCH] D65256: [Sema][ObjC] Mark C union fields that have non-trivial ObjC ownership qualifications as unavailable if the union is declared in a system header

jordan_rose added a comment. I'm personally still of the opinion that allowing non-trivial fields in unions was a mistake, but it's too late to change that as well. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65256/new/ https://reviews.llvm.org/D65256

[PATCH] D65256: [Sema][ObjC] Mark C union fields that have non-trivial ObjC ownership qualifications as unavailable if the union is declared in a system header

jordan_rose added a comment. In D65256#1601410 , @rjmccall wrote: > These were unavailable in system headers before because otherwise we would've > had to make them invalid. Since these unions are no longer otherwise > invalid, there shouldn't be a

[PATCH] D65256: [Sema][ObjC] Mark C union fields that have non-trivial ObjC ownership qualifications as unavailable if the union is declared in a system header

jordan_rose added inline comments. Comment at: lib/Sema/SemaDecl.cpp:16379 + !getLangOpts().CPlusPlus && !FD->hasAttr() && + !FD->getType()->getAs()) { + // For backward compatibility, fields of C unions declared in system This

[PATCH] D65212: [analyzer] Fix exporting SARIF files from scan-build on Windows

jordan_rose added a comment. Oops. It looks like there's another place where this pattern shows up (see rC139382 ). The other one should probably be changed as well. (Thanks for diverging from test(1), perl…) Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D63943: ObjC: Squeeze in one more bit for the count of protocols in 'id' types

jordan_rose added a comment. Yeah, I'll write one. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63943/new/ https://reviews.llvm.org/D63943 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D63943: ObjC: Squeeze in one more bit for the count of protocols in 'id' types

jordan_rose created this revision. jordan_rose added reviewers: jfb, doug.gregor, dexonsmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. Someone actually had an Objective-C object pointer type with more than 64 protocols, probably collected through typedefs. Raise the

[PATCH] D60543: [clang] Add matcher for subclasses of Objective-C interfaces 

jordan_rose added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:1479 +/// \endcode +AST_MATCHER_P(ObjCInterfaceDecl, isSubclassOfInterface, + internal::Matcher, aaron.ballman wrote: > jordan_rose wrote: > > aaron.ballman

[PATCH] D60543: [clang] Add matcher for subclasses of Objective-C interfaces 

jordan_rose added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:1479 +/// \endcode +AST_MATCHER_P(ObjCInterfaceDecl, isSubclassOfInterface, + internal::Matcher, aaron.ballman wrote: > jordan_rose wrote: > > aaron.ballman

[PATCH] D60543: [clang] Add matcher for subclasses of Objective-C interfaces 

jordan_rose added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:1479 +/// \endcode +AST_MATCHER_P(ObjCInterfaceDecl, isSubclassOfInterface, + internal::Matcher, aaron.ballman wrote: > stephanemoore wrote: > >

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

jordan_rose added a comment. Ooh, I should have remembered "designated initializer". I guess it doesn't matter so much either way. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59806/new/ https://reviews.llvm.org/D59806

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 

jordan_rose added a comment. I don't think there's ever a reason to call `[super self]`, and doing so through a macro could easily indicate a bug. Diagnostic nitpick: the Objective-C term is "init method", not "initializer". Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

jordan_rose added inline comments. Comment at: lib/CodeGen/CGObjCMac.cpp:7285 -Entry = new llvm::GlobalVariable(CGM.getModule(), ObjCTypes.ClassnfABIPtrTy, +Entry = new llvm::GlobalVariable(CGM.getModule(), ClassGV->getType(),

[PATCH] D59176: Modules: Add LangOptions::CacheGeneratedPCH

jordan_rose added a comment. This commit by itself doesn't change any behavior, right? Comment at: clang/lib/Frontend/FrontendActions.cpp:115 CI.getPreprocessorOpts().AllowPCHWithCompilerErrors, - FrontendOpts.IncludeTimestamps)); +

[PATCH] D59197: [NFC][clang][PCH][ObjC] Add some missing `VisitStmt(S); `

jordan_rose accepted this revision. jordan_rose added a comment. This revision is now accepted and ready to land. I'm no Clang serialization expert but this makes sense to me. It's consistent with all the other statement visitor methods. Repository: rC Clang CHANGES SINCE LAST ACTION

[PATCH] D57076: [ObjC generics] Fix applying `__kindof` to the type parameter.

jordan_rose added a comment. I //think// this is reasonable but Doug was the one who worked on this. I wonder if it also helps with the test cases in rdar://problem/24619481. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57076/new/ https://reviews.llvm.org/D57076

[PATCH] D55865: [ObjC] Add a new attribute to opt-out of implicit callee retain/release in ARC

jordan_rose added a comment. I think Aaron and John have covered any comments I would make! I'm glad this came out so simply, though. I was afraid it'd be a much larger change than just expanding what was already there. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55865/new/

[PATCH] D22391: [Sema] Add warning for implicitly casting a null constant to a non null pointer type

jordan_rose added inline comments. Comment at: test/Sema/conditional-expr.c:20 vp = 0 ? (double *)0 : (void *)0; - ip = 0 ? (double *)0 : (void *)0; // expected-warning {{incompatible pointer types assigning to 'int *' from 'double *'}} + ip = 0 ? (double *)0 : (void *)0;

[PATCH] D22391: [Sema] Add warning for implicitly casting a null constant to a non null pointer type

jordan_rose added a subscriber: dergachev.a. jordan_rose added a comment. Sorry for the delay. I think this is mostly good, but I do still have a concern about the diagnostics change. Comment at: lib/Sema/SemaExpr.cpp:7117 +if (E && S.checkNonNullExpr(E)) + return

[PATCH] D44498: Sink PrettyDeclStackTrace down to the AST library

jordan_rose closed this revision. jordan_rose added a comment. Committed in https://reviews.llvm.org/rL328276. Repository: rC Clang https://reviews.llvm.org/D44498 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D44498: Sink PrettyDeclStackTrace down to the AST library

jordan_rose created this revision. jordan_rose added reviewers: bruno, rsmith. ...and add some very basic stack trace entries for module building. This would have helped track down rdar://problem/38434694 sooner (which I can't share, sorry). Repository: rC Clang

[PATCH] D43162: [Parser] (C++) Make -Wextra-semi slightly more useful

jordan_rose added a comment. It's been a long time since that commit, but I think the difference is that `-pedantic` / `Extension`-type diagnostics are magic and `-Wc++98-compat-pedantic` is not. I like Richard's way better and if it could be applied to -Wnewline-eof later as well then that

[PATCH] D39035: Unnamed bitfields don't block constant evaluation of constexpr constructors

jordan_rose closed this revision. jordan_rose added a comment. Landed in https://reviews.llvm.org/rL316408. Repository: rL LLVM https://reviews.llvm.org/D39035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D39035: Unnamed bitfields don't block constant evaluation of constexpr constructors

jordan_rose added a comment. Thanks, Aaron! Comment at: test/SemaCXX/warn-global-constructors.cpp:130 + +namespace HasUnnamedBitfield { + struct HasUnnamedBitfield { aaron.ballman wrote: > Does this namespace require a name? If so, can it be named different

[PATCH] D39035: Unnamed bitfields don't block constant evaluation of constexpr constructors

jordan_rose created this revision. jordan_rose added a project: clang. C++14 [dcl.constexpr]p4 states that in the body of a constexpr constructor, > every non-variant non-static data member and base class sub-object shall be > initialized However, [class.bit]p2 notes that > Unnamed bit-fields

[PATCH] D38327: [Sema] Put nullability fix-it after the end of the pointer.

jordan_rose accepted this revision. jordan_rose added a comment. This revision is now accepted and ready to land. Nice catch, Volodymyr! Looks good to me. https://reviews.llvm.org/D38327 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D35533: [Basic] Update CMakeLists.txt to handle Repo

jordan_rose added a subscriber: modocache. jordan_rose added a comment. > The addition of "${path}/.git/HEAD" at line 37 is used only once when repo is > initially synced "for the first time". This is because ${git_path}/logs/HEAD > file at line 34 for Git or Git submodule does not exist with

[PATCH] D35533: [Basic] Update CMakeLists.txt to handle Repo

jordan_rose added a comment. As commented at https://reviews.llvm.org/D34955#798369, this isn't sufficient because it doesn't force a rebuild when new changes are merged in from upstream. It's not //harmful// to stick with the old version information, but if you can find something better that

[PATCH] D34955: [Basic] Detect Git submodule version in CMake

jordan_rose accepted this revision. jordan_rose added a comment. This revision is now accepted and ready to land. Whoops, yes, this new version looks good! https://reviews.llvm.org/D34955 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D34955: [Basic] Detect Git submodule version in CMake

jordan_rose added inline comments. Comment at: lib/Basic/CMakeLists.txt:27 +STRING(REGEX REPLACE "\n" "" file_contents "${file_contents}") +if ("${file_contents}" MATCHES "^gitdir:") + # '.git' is indeed a link to the submodule's Git directory. How

[PATCH] D34955: [Basic] Detect Git submodule version in CMake

jordan_rose added a comment. I'm not sure why we would care if a commit is made on a branch. The important information here is what commit is checked out, and that's what HEAD refers to. https://reviews.llvm.org/D34955 ___ cfe-commits mailing list

[PATCH] D34955: [Basic] Detect Git submodule version in CMake

jordan_rose requested changes to this revision. jordan_rose added a comment. This revision now requires changes to proceed. If I'm remembering correctly from when I set this up, this isn't just about detecting which version control system you're using; it's about finding a file //that will

[PATCH] D26108: Add -Wnullability-completeness-on-arrays.

jordan_rose added a comment. Yep, that's the correct syntax. It's not wonderful, but it's already where the C standard lets you put `const`, so we're just following established practice. (Learn something new and awful about C every day!) Repository: rL LLVM https://reviews.llvm.org/D26108

[PATCH] D26108: Add -Wnullability-completeness-on-arrays.

jordan_rose added a comment. No, the *arrays themselves* need to be marked as nullable or non-nullable. Remember that in C array parameters are passed as pointers. The compiler should give you a fix-it that shows the correct syntax. Repository: rL LLVM https://reviews.llvm.org/D26108

[PATCH] D22391: [Sema] Add warning for implicitly casting a null constant to a non null pointer type

jordan_rose added a comment. This looks like it's only coming up for declarations. What about assignments? int x; int * _Nonnull p = p = NULL; // warn here? Comment at: lib/Sema/SemaExpr.cpp:7117 +if (E && S.checkNonNullExpr(E)) + return

[PATCH] D31005: [Objective-C] Fix "repeated use of weak" warning with -fobjc-weak

jordan_rose added inline comments. Comment at: lib/Sema/SemaDecl.cpp:10184 + (!getLangOpts().ObjCAutoRefCount && getLangOpts().ObjCWeak && + VDecl->getType().getObjCLifetime() != Qualifiers::OCL_Weak)) &&

[PATCH] D31005: [Objective-C] Fix "repeated use of weak" warning with -fobjc-weak

jordan_rose added a reviewer: rjmccall. jordan_rose added a subscriber: rjmccall. jordan_rose added a comment. The warning-related parts look reasonable to me. Comment at: lib/Sema/SemaDecl.cpp:10184 + (!getLangOpts().ObjCAutoRefCount && getLangOpts().ObjCWeak && +

[PATCH] D22391: [Sema] Add warning for implicitly casting a null constant to a non null pointer type

jordan_rose added inline comments. Comment at: include/clang/Basic/DiagnosticGroups.td:290 def NullableToNonNullConversion : DiagGroup<"nullable-to-nonnull-conversion">; +def NullConstToNonnull : DiagGroup<"null-const-to-nonnull">; def NullabilityCompletenessOnArrays :

[PATCH] D28924: [AST Printer] Print attributes on enum constants

jordan_rose closed this revision. jordan_rose added a comment. Landed as-is in https://reviews.llvm.org/rL292571. Repository: rL LLVM https://reviews.llvm.org/D28924 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D28924: [AST Printer] Print attributes on enum constants

jordan_rose added a comment. Interestingly, this case doesn't actually work yet; we unconditionally print enum attributes after the close-brace even though that's not valid for C++ attributes. Do you think I should change that as well, or just land this patch as is? Repository: rL LLVM

[PATCH] D28924: [AST Printer] Print attributes on enum constants

jordan_rose added a comment. Good idea, will do. Repository: rL LLVM https://reviews.llvm.org/D28924 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28924: [AST Printer] Print attributes on enum constants

jordan_rose created this revision. The AST printer was dropping attributes on enumerators (enum constants). Now it's not. Repository: rL LLVM https://reviews.llvm.org/D28924 Files: lib/AST/DeclPrinter.cpp test/Sema/ast-print.c Index: test/Sema/ast-print.c

[PATCH] D27837: Add fix-it notes to the nullability consistency warning

jordan_rose accepted this revision. jordan_rose added a reviewer: jordan_rose. jordan_rose added a comment. This revision is now accepted and ready to land. Doug was okay with the idea and I trust Alex and Akira would have caught any major problems. Committed in r290132. Repository: rL LLVM

[PATCH] D27837: Add fix-it notes to the nullability consistency warning

jordan_rose updated this revision to Diff 81796. jordan_rose added a comment. Updated names to match LLVM style. Repository: rL LLVM https://reviews.llvm.org/D27837 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaType.cpp test/FixIt/Inputs/nullability.h

[PATCH] D27837: Add fix-it notes to the nullability consistency warning

jordan_rose added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8772 +def note_nullability_fix_it : Note< + "insert '%select{_Nonnull|_Nullable|_Null_unspecified}0' if the " + "%select{pointer|block pointer|member pointer|array parameter}1 "

[PATCH] D27837: Add fix-it notes to the nullability consistency warning

jordan_rose created this revision. jordan_rose added a reviewer: doug.gregor. jordan_rose added a subscriber: cfe-commits. jordan_rose set the repository for this revision to rL LLVM. This is especially important for arrays, since no one knows the proper syntax for putting qualifiers in arrays.