chandlerc added inline comments.
================ Comment at: lib/AST/ASTContext.cpp:8786 + if (OverrideNonnull && OverrideNonnullArgs) + *OverrideNonnullArgs |= 1 << ArgTypes.size(); + ---------------- majnemer wrote: > `1U` to avoid overflow UB? I'd mildly prefer an assert (or rely on UBSan) that it *doesn't* overflow. Also note that this is the same pattern we use just a few lines down for the ICE argument bitmask -- we have a pretty baked in assumption that all the builtins here don't have more than 31 arguments. Anyways, happy to make whatever change you would prefer here. I just copied the code below, I don't have any strong opinion here. ================ Comment at: lib/CodeGen/CGCall.cpp:2082 + if (getNonNullAttr(FD, PVD, ParamType, PVD->getFunctionScopeIndex()) && + (OverrideNonnullArgs & (1 << ArgNo)) == 0) + Attrs.addAttribute(llvm::Attribute::NonNull); ---------------- majnemer wrote: > Ditto. See above, I'll keep these in sync. However, this exposes a related bug: we shouldn't be even doing this bit test if the function isn't a builtin. I'll add a check for that. That seems cleaner than relying on the bittest itself failing. https://reviews.llvm.org/D30806 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
