[Mlir-commits] [mlir] [MLIR][LLVM] Infer export location scope from location, is possible (PR #70465)

[Tobias Gysi]

================
@@ -301,17 +301,11 @@ llvm::DILocation *DebugTranslation::translateLoc(Location loc,
     llvmLoc = translateLoc(callLoc.getCallee(), scope, callerLoc);
 
   } else if (auto fileLoc = dyn_cast<FileLineColLoc>(loc)) {
-    llvm::DILocalScope *locationScope = scope;
-    // Only construct a new DIFile when no local scope is present. This
-    // prioritizes existing DI information when it's present.
-    if (!locationScope) {
-      auto *file = translateFile(fileLoc.getFilename());
-      locationScope = llvm::DILexicalBlockFile::get(llvmCtx, scope, file,
-                                                    /*Discriminator=*/0);
-    }
-    llvmLoc = llvm::DILocation::get(llvmCtx, fileLoc.getLine(),
-                                    fileLoc.getColumn(), locationScope,
-                                    const_cast<llvm::DILocation *>(inlinedAt));
+    if (!scope)
----------------
gysit wrote:

I assume a scope is required to construct a DILocation, or is this a limitation we introduce? A short comment that a location is required may be helpful here for the next time we touch this code. When reviewing I was also looking up how getMergedLocation works (line 321). It seems to correctly handle nullptr. Maybe also add a brief comment there that nullptrs are handled by getMergedLocation? 

https://github.com/llvm/llvm-project/pull/70465