[PATCH] D77133: [mlir] Add an out-of-tree dialect example

Jean-Michel Gorius via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 3 08:02:33 PDT 2020


Kayjukh accepted this revision.
Kayjukh added inline comments.


================
Comment at: mlir/examples/standalone/standalone-opt/standalone-opt.cpp:25
+using namespace llvm;
+using namespace mlir;
+
----------------
marbre wrote:
> Kayjukh wrote:
> > marbre wrote:
> > > If it is an out-of-tree example, I would avoid `using namespace`.
> > Is there any particular reason for this? IMO the two `using namespace` directives make the code easier to read by removing a lot of syntactic overhead. I can understand that in a larger project there might be a potential issue with names colliding but if this is the case, then the maintainer of said project can remove those lines and adapt the code as needed.
> It might not be necessary here, but I thought it might be nice to follow the [[ https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#sf6-use-using-namespace-directives-for-transition-for-foundation-libraries-such-as-std-or-within-a-local-scope-only | C++ Core Guidelines ]]. The [[ https://google.github.io/styleguide/cppguide.html#Namespaces | Google C++ Style Guide ]] gives a similar advice: "Do not use using-directives (e.g. using namespace foo)."
> Further, Intel did avoid copying `using namespace` in their [[ https://github.com/NervanaSystems/ngraph/blob/master/src/contrib/mlir/tools/ngraph-opt/ngraph_opt.cpp | ngraph-opt  ]] and I didin the forked source for `iree-opt`.
Ok, yes, it makes sense to follow the style guides of other out-of-tree projects.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77133/new/

https://reviews.llvm.org/D77133





More information about the llvm-commits mailing list