Page MenuHomePhabricator

[clang] Specify type of pthread_create builtin
Needs ReviewPublic

Authored by jrtc27 on Feb 21 2019, 4:13 PM.

Details

Reviewers
rsmith
jdoerfert
Summary

Currently, pthread_create's type string is empty, so GetBuiltinType
fails and any header declaring it gives a -Wbuiltin-requires-header
warning, which gives a false-positive when including pthread.h itself
with -Wsystem-headers. By specifying its type, this false-positive goes
away and it behaves like every other builtin declared in system headers,
only warning when the declaration does not match the expected type.

This requires introducing a way to declare function pointer types in
builtin type strings, which requires refactoring GetBuiltinType to allow
recursive parsing of function types, expressed as a type string within
parentheses.

Event Timeline

jrtc27 created this revision.Feb 21 2019, 4:13 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 21 2019, 4:13 PM

A separate point is whether it makes sense to be emitting this warning in the first place for GE_Missing_type. I'd argue that, if we don't know the type of the builtin, we should never emit the warning, since otherwise we always emit the warning even for legitimate instances, though that's not really important now we're not hitting that case any more.

srhines added inline comments.Mar 20 2019, 8:43 AM
clang/lib/AST/ASTContext.cpp
9574

I would just remove "of function" here, since it isn't correct when you are not decoding a function, and the two words don't make that much more meaningful even in the original case, so I don't think it justifies something like "of intrinsic/function".

probinson added a subscriber: probinson.

We've started running into this too in building the PS4 system. +jdoerfert who added pthread_create to the builtin list.

Looking at the patch, it seems straightforward enough although clearly needs clang-format-diff run over it.
I don't touch Clang that much so I'm reluctant to okay it myself.

We've started running into this too in building the PS4 system. +jdoerfert who added pthread_create to the builtin list.

Looking at the patch, it seems straightforward enough although clearly needs clang-format-diff run over it.
I don't touch Clang that much so I'm reluctant to okay it myself.

@probinson Thanks for making me aware of this patch.

A separate point is whether it makes sense to be emitting this warning in the first place for GE_Missing_type. I'd argue that, if we don't know the type of the builtin, we should never emit the warning

@jrtc27 I hope the immediate need for this is gone after D58091 was finally committed? Given that I caused this mess I can take a look at the patch if you are still interested in it, are you?