================
@@ -6765,12 +6765,255 @@ llvm::Value 
*CGOpenMPRuntime::emitNumThreadsForTargetDirective(
 namespace {
 LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
 
+/// Utility to compare expression locations.
+/// Returns true if expr-loc of LHS is less-than that of RHS.
+/// This function asserts that both expressions have valid expr-locations.
+static bool compareExprLocs(const Expr *LHS, const Expr *RHS) {
+  // Assert that neither LHS nor RHS can be null
+  assert(LHS && "LHS expression cannot be null");
+  assert(RHS && "RHS expression cannot be null");
+
+  // Get source locations
+  SourceLocation LocLHS = LHS->getExprLoc();
+  SourceLocation LocRHS = RHS->getExprLoc();
+
+  // Assert that we have valid source locations
+  assert(LocLHS.isValid() && "LHS expression must have valid source location");
+  assert(LocRHS.isValid() && "RHS expression must have valid source location");
+
+  // Compare source locations for deterministic ordering
+  return LocLHS < LocRHS;
+}
+
 // Utility to handle information from clauses associated with a given
 // construct that use mappable expressions (e.g. 'map' clause, 'to' clause).
 // It provides a convenient interface to obtain the information and generate
 // code for that information.
 class MappableExprsHandler {
 public:
+  /// Custom comparator for attach-pointer expressions that compares them by
+  /// complexity (i.e. their component-depth) first, then by their expr-locs if
+  /// they are semantically different.
----------------
abhinavgaba wrote:

Can you provide the criteria you'd like me to use instead of this?

>From my perspective, EXPR location comparison has the following pros/cons:
Pros:
* Reliable and reproducible from build to build.
* User-friendly, because in case of pointers with the same complexity, maps 
with the same base variable will be hanlded in the order they appear on a 
directive.
* Light-weight.

Cons:
* Theoretical: May not work when comparing EXPRs across files (which we do not 
need for attach pointer expr comparison)

And for the theoretical con, we have ways to resolve that in the future, if 
needed:

* For now, we can just assert that we are working with the same FileID.
* If we really need to compare Exprs across files in the future, we can use 
`SourceManager::IsBeforeinTranslationUnit`, as suggested in the header comment 
of the `operator <`: 
https://github.com/llvm/llvm-project/blob/87a4e1ce3c0cfbc726a070717d0d5901289ee073/clang/include/clang/Basic/SourceLocation.h#L199-L203

The other option I know of is pointer comparison between two `EXPR *`s, which 
is not reproducible from build to build.

I don't understand what technical reason there is to avoid an EXPR location 
comparison, when we are asserting that the EXPRs being compared have a valid 
location.

@kparzysz, do you know of another way of comparing EXPRs that is reproducible 
and has the benefits that comparing expr locations provides?

https://github.com/llvm/llvm-project/pull/155625
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to