fix common namespaces for public API and internals #766
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| auto& S = getInterpreter(parent)->getSema(); | ||
| auto* ND = Cpp::Cpp_utils::Lookup::Named(&S, name, Within); | ||
| auto* ND = CppInternal::utils::Lookup::Named(&S, name, Within); |
There was a problem hiding this comment.
warning: no header providing "CppInternal::utils::Lookup::Named" is directly included [misc-include-cleaner]
lib/CppInterOp/CXCppInterOp.cpp:4:
- #include "clang/AST/CXXInheritance.h"
+ #include "CppInterOpInterpreter.h"
+ #include "clang/AST/CXXInheritance.h"|
|
||
| namespace compat { | ||
| using Interpreter = Cpp::Interpreter; | ||
| using Interpreter = CppInternal::Interpreter; |
There was a problem hiding this comment.
warning: no type named 'Interpreter' in namespace 'CppInternal'; did you mean 'clang::Interpreter'? [clang-diagnostic-error]
| using Interpreter = CppInternal::Interpreter; | |
| using Interpreter = clang::Interpreter; |
Additional context
llvm/include/clang/Interpreter/Interpreter.h:85: 'clang::Interpreter' declared here
class Interpreter {
^| } | ||
|
|
||
| auto* ND = Cpp_utils::Lookup::Named(&getSema(), name, Within); | ||
| auto* ND = CppInternal::utils::Lookup::Named(&getSema(), name, Within); |
There was a problem hiding this comment.
warning: no header providing "CppInternal::utils::Lookup::Named" is directly included [misc-include-cleaner]
lib/CppInterOp/CppInterOp.cpp:12:
+ #include "CppInterOpInterpreter.h"| For_Visible_Redeclaration); | ||
|
|
||
| Cpp_utils::Lookup::Named(&S, R, Decl::castToDeclContext(D)); | ||
| CppInternal::utils::Lookup::Named(&S, R, Decl::castToDeclContext(D)); |
There was a problem hiding this comment.
warning: no header providing "CppInternal::utils::Lookup::Named" is directly included [misc-include-cleaner]
CppInternal::utils::Lookup::Named(&S, R, Decl::castToDeclContext(D));
^| namespace Cpp { | ||
| namespace CppInternal { | ||
| namespace utils { | ||
| namespace Lookup { |
There was a problem hiding this comment.
warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
| namespace Lookup { | |
| namespace CppInternal::utils::Lookup { |
lib/CppInterOp/CppInterOpInterpreter.h:141:
- } // namespace Lookup
- } // namespace utils
- } // namespace CppInternal
+ } // namespace CppInternal::utils::Lookup
+ | bool m_UseHashTable = true; | ||
|
|
||
| const Cpp::DynamicLibraryManager& m_DynamicLibraryManager; | ||
| const DynamicLibraryManager& m_DynamicLibraryManager; |
There was a problem hiding this comment.
warning: member 'm_DynamicLibraryManager' of type 'const DynamicLibraryManager &' is a reference [cppcoreguidelines-avoid-const-or-ref-data-members]
const DynamicLibraryManager& m_DynamicLibraryManager;
^| case ELF::DT_RPATH: | ||
| SplitPaths(Data + Dyn.d_un.d_val, RPath, utils::kAllowNonExistent, | ||
| Cpp::utils::platform::kEnvDelim, false); | ||
| SplitPaths(Data + Dyn.d_un.d_val, RPath, |
There was a problem hiding this comment.
warning: do not access members of unions; use (boost::)variant instead [cppcoreguidelines-pro-type-union-access]
SplitPaths(Data + Dyn.d_un.d_val, RPath,
^| case ELF::DT_RUNPATH: | ||
| SplitPaths(Data + Dyn.d_un.d_val, RunPath, utils::kAllowNonExistent, | ||
| Cpp::utils::platform::kEnvDelim, false); | ||
| SplitPaths(Data + Dyn.d_un.d_val, RunPath, |
There was a problem hiding this comment.
warning: do not access members of unions; use (boost::)variant instead [cppcoreguidelines-pro-type-union-access]
SplitPaths(Data + Dyn.d_un.d_val, RunPath,
^|
|
||
| namespace Cpp { | ||
| namespace CppInternal { | ||
| namespace utils { |
There was a problem hiding this comment.
warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
| namespace utils { | |
| namespace CppInternal::utils { |
lib/CppInterOp/Paths.cpp:400:
- } // namespace utils
- } // namespace CppInternal
+ } // namespace CppInternal::utils|
|
||
| namespace Cpp { | ||
| namespace CppInternal { | ||
| namespace utils { |
There was a problem hiding this comment.
warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
| namespace utils { | |
| namespace CppInternal::utils { |
lib/CppInterOp/Paths.h:119:
- } // namespace utils
- } // namespace CppInternal
+ } // namespace CppInternal::utils
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #766 +/- ##
=======================================
Coverage 79.09% 79.09%
=======================================
Files 9 9
Lines 3898 3898
=======================================
Hits 3083 3083
Misses 815 815
🚀 New features to boost your workflow:
|
2 participants
Drops common
Cppnamespace used for both public API, and internal facilities.With this patch, we introduce a
CppInternal(open to better suggestions) namespace that contains private classes that are not meant to be exposedAs a result we no longer perform the following unintuitive remap:
and the distinction between public API (everything in
include/CppInterOp.h) and other implementation facilities are clear