[PATCH] D121328: Disable -Wmissing-prototypes for internal linkage functions that aren't explicitly marked "static"""

This revision was automatically updated to reflect the committed changes. Closed by commit rG34b9b1ea4874: Disable -Wmissing-prototypes for internal linkage functions that arent… (authored by dblaikie). Changed prior to commit: https://reviews.llvm.org/D121328?vs=417989=418360#toc Repository:

[PATCH] D121328: Disable -Wmissing-prototypes for internal linkage functions that aren't explicitly marked "static"""

aaron.ballman accepted this revision. aaron.ballman added a comment. In D121328#3406017 , @dblaikie wrote: > @aaron.ballman wouldn't mind your take on this to see if this seems > sufficiently robust, tested, etc. (should I move the isExternallyVisible

Re: [PATCH] D121328: Disable -Wmissing-prototypes for internal linkage functions that aren't explicitly marked "static"""

I had given this some thought. I have a very limited knowledge of the code in this space. IIUC this only matters if the idea of linkage can change, which would happen only if a tag name needs manufacturing, which can only happen in the case we ran across (someone check my logic here?). The other

[PATCH] D121328: Disable -Wmissing-prototypes for internal linkage functions that aren't explicitly marked "static"""

dblaikie added a comment. @aaron.ballman wouldn't mind your take on this to see if this seems sufficiently robust, tested, etc. (should I move the isExternallyVisible check even further down? So its side effects are even less impactful (maybe there are other warnings that care about this sort

[PATCH] D121328: Disable -Wmissing-prototypes for internal linkage functions that aren't explicitly marked "static"""

deansturtevant accepted this revision. deansturtevant added a comment. This revision is now accepted and ready to land. This looks good from my standpoint. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121328/new/ https://reviews.llvm.org/D121328

[PATCH] D121328: Disable -Wmissing-prototypes for internal linkage functions that aren't explicitly marked "static"""

dblaikie added a comment. In D121328#3405623 , @deansturtevant wrote: > Aha! (I think). > If the code to test "isExternalVisible" is executed *after* the code to test > whether it's a C++ member function (the very next test), then the problem >

[PATCH] D121328: Disable -Wmissing-prototypes for internal linkage functions that aren't explicitly marked "static"""

dblaikie updated this revision to Diff 417989. dblaikie added a comment. Add regression test for the -Wnon-c-typedef-for-linkage issue Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121328/new/ https://reviews.llvm.org/D121328 Files:

[PATCH] D121328: Disable -Wmissing-prototypes for internal linkage functions that aren't explicitly marked "static"""

deansturtevant added a comment. Thanks David. Is it possible to add a regression test for the problem that cropped up with the first try? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121328/new/ https://reviews.llvm.org/D121328

[PATCH] D121328: Disable -Wmissing-prototypes for internal linkage functions that aren't explicitly marked "static"""

dblaikie updated this revision to Diff 417985. dblaikie added a comment. Try Dean's suggestion of moving this check further down Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121328/new/ https://reviews.llvm.org/D121328 Files:

[PATCH] D121328: Disable -Wmissing-prototypes for internal linkage functions that aren't explicitly marked "static"""

deansturtevant requested changes to this revision. deansturtevant added a comment. This revision now requires changes to proceed. Aha! (I think). If the code to test "isExternalVisible" is executed *after* the code to test whether it's a C++ member function (the very next test), then the problem

[PATCH] D121328: Disable -Wmissing-prototypes for internal linkage functions that aren't explicitly marked "static"""

deansturtevant added a comment. Note that some fix is important to make if we think that the -Wmissing-prototypes warning is valuable, because there are cases where it currently would fire where the function cannot explicitly be given internal linkage, e.g. namespace { struct Initialized {}; }

[PATCH] D121328: Disable -Wmissing-prototypes for internal linkage functions that aren't explicitly marked "static"""

jyknight added a comment. Given that this is an error `error: functions cannot be declared in an anonymous struct` (thus you can assume that function declarations inside a struct imply that the struct MUST have a linkage name) struct { static int foo(); }; then I don't think there's

[PATCH] D121328: Disable -Wmissing-prototypes for internal linkage functions that aren't explicitly marked "static"""

dblaikie added a comment. In D121328#3396984 , @aaron.ballman wrote: >> once we figure out what to do about the change in behavior for >> -Wnon-c-typedef-for-linkage > > The devil is in the details; I'm not sure what to do here. I don't think >

[PATCH] D121328: Disable -Wmissing-prototypes for internal linkage functions that aren't explicitly marked "static"""

aaron.ballman added a comment. > once we figure out what to do about the change in behavior for > -Wnon-c-typedef-for-linkage The devil is in the details; I'm not sure what to do here. I don't think there's a way to compute the visibility without also computing the linkage, so I don't think

[PATCH] D121328: Disable -Wmissing-prototypes for internal linkage functions that aren't explicitly marked "static"""

dblaikie added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121328/new/ https://reviews.llvm.org/D121328 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D121328: Disable -Wmissing-prototypes for internal linkage functions that aren't explicitly marked "static"""

manojgupta added a comment. For the background, we had hit this in Chrome OS when building bluetooth code. This is the one of structs hitting the issue where the warning got promoted to an error: typedef struct { private: static std::string AppendCapability(std::string& result, bool

[PATCH] D121328: Disable -Wmissing-prototypes for internal linkage functions that aren't explicitly marked "static"""

dblaikie created this revision. dblaikie added reviewers: aaron.ballman, rsmith, denik, deansturtevant. Herald added a project: All. dblaikie requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Some functions can end up non-externally visible