================
@@ -24,9 +27,140 @@ CIRGenModule::CIRGenModule(mlir::MLIRContext &context,
                            clang::ASTContext &astctx,
                            const clang::CodeGenOptions &cgo,
                            DiagnosticsEngine &diags)
-    : astCtx(astctx), langOpts(astctx.getLangOpts()),
-      theModule{mlir::ModuleOp::create(mlir::UnknownLoc())},
-      target(astCtx.getTargetInfo()) {}
+    : builder(&context), astCtx(astctx), langOpts(astctx.getLangOpts()),
+      theModule{mlir::ModuleOp::create(mlir::UnknownLoc::get(&context))},
+      diags(diags), target(astCtx.getTargetInfo()) {}
+
+mlir::Location CIRGenModule::getLoc(SourceLocation cLoc) {
+  assert(cLoc.isValid() && "expected valid source location");
+  const SourceManager &sm = astCtx.getSourceManager();
+  PresumedLoc pLoc = sm.getPresumedLoc(cLoc);
+  StringRef filename = pLoc.getFilename();
+  return mlir::FileLineColLoc::get(builder.getStringAttr(filename),
+                                   pLoc.getLine(), pLoc.getColumn());
+}
----------------
dkolsen-pgi wrote:

> I don't buy the reasoning of "mlir::Location exists, therefore it's the right 
> representation".

`mlir::Location` is not the right representation because it exists.  It's 
because it is used by all the other MLIR dialects that have nothing to do with 
C++ and cannot have a dependency on Clang.  It is wanting ClangIR to play 
nicely with the rest of the MLIR ecosystem, when many dialects are mixed 
together in a single tree later in the compilation chain, that makes using 
`SourceLocation` problematic.

> Not saying this is something we need to solve right now

Definitely.  Using `SourceLocation` instead of `mlir::Location` is not 
something that can be done in this PR.  We would need evidence that the lack of 
`SourceLocation` and the richness of what it can represent is a serious problem 
before attempting such a radical change.


https://github.com/llvm/llvm-project/pull/113483
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to