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

[Chris Lattner via Phabricator via llvm-commits]

lattner added inline comments.


================
Comment at: mlir/examples/standalone/standalone-opt/standalone-opt.cpp:25
+using namespace llvm;
+using namespace mlir;
+
----------------
Kayjukh wrote:
> 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.
In my work on an out of tree projects using MLIR, I found it to be pretty impractical to avoid using declarations in general.  There is significant syntactic overhead for this, and (IIRC) tblgen output assumes MLIR is in scope in some cases.  Similarly, LLVM datatypes like StringRef are pervasive and used by downstream projects.

I have always ended up using a project specific version of things like this file:
https://github.com/llvm/llvm-project/blob/master/mlir/include/mlir/Support/LLVM.h

but which includes MLIR specific types in addition to LLVM.




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

https://reviews.llvm.org/D77133