[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 wouldn't arise. 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 https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 {};
} // namespace
void* operator new(size_t size, const Initialized&) {
void* p = malloc(size);
memset(p, 0, size);
return p;
}
operator new is only allowed to be defined in the global namespace.
Currently this will cause the warning to be emitted, even though there is no
way to declare it outside the TU.
jyknight clarified that what he was proposing was that instead of calling
FD->isExternallyVisible() as currently written, call a function which does the
same thing except that it doesn't check whether the struct has a visible name
(because that can be assumed here).
This function *could* be isExternallyVisible with a new boolean argument (WDYT?)
Another approach would be to move the call to
ShouldWarnAboutMissingPrototypes() to a different point in the process, where
calling isExternallyVisible() is *not* problematic. WDYT?
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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
