kadircet added inline comments.
================
Comment at: clang-tools-extra/clangd/unittests/StdLibTests.cpp:37
EXPECT_THAT(CXX, HasSubstr("#include <cstdio>"));
- EXPECT_THAT(CXX, Not(HasSubstr("#include <stdio.h>")));
+ EXPECT_THAT(CXX, HasSubstr("#include <stdio.h>"));
----------------
hokein wrote:
> This is a behavior change, I think it is probably fine. Would be nice to have
> a second look.
let's change it to `stdatomic.h` instead.
================
Comment at: clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp:217
+NSSymbolMap *Recognizer::namespaceSymbols(const NamespaceDecl *D, Lang L) {
+ // D could be nullptr!
auto It = NamespaceCache.find(D);
----------------
nit: i think it's better to early-exit when D is nullptr rather than thinking
about that in the rest of the logic.
================
Comment at: clang/unittests/Tooling/StandardLibraryTest.cpp:68
EXPECT_THAT(stdlib::Symbol::all(), Contains(*Vector));
- EXPECT_FALSE(stdlib::Header::named("<stdint.h>"));
- EXPECT_FALSE(stdlib::Header::named("<stdint.h>", stdlib::Lang::CXX));
+ EXPECT_TRUE(stdlib::Header::named("<stdint.h>", stdlib::Lang::CXX));
EXPECT_TRUE(stdlib::Header::named("<stdint.h>", stdlib::Lang::C));
----------------
as mentioned above, you can use `stdatomic.h` instead
================
Comment at: clang/unittests/Tooling/StandardLibraryTest.cpp:68
EXPECT_THAT(stdlib::Symbol::all(), Contains(*Vector));
- EXPECT_FALSE(stdlib::Header::named("<stdint.h>"));
- EXPECT_FALSE(stdlib::Header::named("<stdint.h>", stdlib::Lang::CXX));
+ EXPECT_TRUE(stdlib::Header::named("<stdint.h>", stdlib::Lang::CXX));
EXPECT_TRUE(stdlib::Header::named("<stdint.h>", stdlib::Lang::C));
----------------
kadircet wrote:
> as mentioned above, you can use `stdatomic.h` instead
maybe put everything below into a separate test for CCompat?
================
Comment at: clang/unittests/Tooling/StandardLibraryTest.cpp:71
- EXPECT_FALSE(stdlib::Symbol::named("", "int16_t"));
- EXPECT_FALSE(stdlib::Symbol::named("", "int16_t", stdlib::Lang::CXX));
+ EXPECT_TRUE(stdlib::Symbol::named("", "int16_t", stdlib::Lang::CXX));
EXPECT_TRUE(stdlib::Symbol::named("", "int16_t", stdlib::Lang::C));
----------------
having a more comprehensive test here might be useful, e.g. check that
`std::int16_t` only exists in C++ and just has <cstdint> as a provider.
`int16_t` exists both in C and C++ with relevant providers
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143214/new/
https://reviews.llvm.org/D143214
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
