steakhal marked 2 inline comments as done.
steakhal added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp:70-73
+ auto ExternCBlockBegin =
+ SM.getDecomposedExpansionLoc(LinkSpecDecl->getBeginLoc());
+ auto ExternCBlockEnd =
+ SM.getDecomposedExpansionLoc(LinkSpecDecl->getEndLoc());
----------------
whisperity wrote:
> This unconditionally assumes that an encountered LSD will be `extern "C"`.
> However, `extern "C++"` is also possible (and supported by Clang). (And
> things like `extern "Java"` are also possible according to the standard,
> albeit not supported by Clang.)
O.O I did not know its a thing :D
It seems like clang only supports `C` or `C++`.
================
Comment at:
clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp:87-88
const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP)
{
- PP->addPPCallbacks(
- ::std::make_unique<IncludeModernizePPCallbacks>(*this, getLangOpts()));
+ PP->addPPCallbacks(::std::make_unique<detail::IncludeModernizePPCallbacks>(
+ *this, getLangOpts(), PP->getSourceManager()));
+}
----------------
whisperity wrote:
> (🔮: Same here. Is this state that might keep a dangling reference if multiple
> TUs are consumed in sequence?)
How can I acquire the right `SourceManager` if not like this?
I haven't found any alternative.
What do you suggest?
================
Comment at:
clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp:135
{"float.h", "cfloat"},
{"limits.h", "climits"},
{"locale.h", "clocale"},
----------------
whisperity wrote:
> POSIX defines a "global" include `limits.h` which gives you the definition of
> macros like `PATH_MAX`. Will such code keep on working if the include is
> turned into `climits`?
IDK.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125209/new/
https://reviews.llvm.org/D125209
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
