[clang] [Clang] Support MSPropertyRefExpr as placement arg to new-expression (PR #75883)

via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 3 16:24:38 PST 2024


================
@@ -2286,6 +2286,9 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal,
   bool PassAlignment = getLangOpts().AlignedAllocation &&
                        Alignment > NewAlignment;
 
+  if (CheckArgsForPlaceholders(PlacementArgs))
----------------
Sirraide wrote:

The only expressions that this seems to affect is expressions for which `isPlaceholderToRemoveAsArg` returns `true` by passing them to `CheckPlaceholderExpr`; this only affects a handful of types:

- `BuiltinType::PseudoObject`: MSProperty (subscript) properties and ObjC (subscript) properties; the latter are only relevant for Objective-C++ in this case. I’ll add tests for the latter as well (to the best of my abilities as I have never written a line of Objective-C or Objective-C++ before in my life). The comment in `BuiltinTypes.def` also mentions ‘VS.NET's `__property` declarations’, but I couldn’t find anything about `__property` in the rest of the codebase, and I’m candidly not entirely sure what this is in reference to, so I’m only adding tests for Objective-C++ for now.
    
- `BuiltinType::UnknownAny`: For this, `CheckPlaceholderExpr` just emits an error, and I personally don’t see a way in which this would ever be valid as a placement-arg to a new expression, but I could be wrong here. Should we add tests for this case?

- `BuiltinType::BoundMember`: `s.foo` and friends, where `foo` is a non-static member function; these are already invalid in placement-args, and `CheckPlaceholderExpr` similarly issues what looks to be the exact same error; we don’t seem to have any tests that check for this error, however, so I’ll add some for this as well.

- `BuiltinType::BuiltinFn`: Only `DeclRefExpr`s are affected here as this simply issues an error otherwise (which should be fine because passing a builtin function as a function argument doesn’t make sense anyway); if it is a builtin such as `__builtin_memset`, then this pr also ends up improving the error message from ‘no known conversion from '<builtin fn type>'’ to ‘builtin functions must be directly called’.

    If it is a `DeclRefExpr`, there are two cases to consider: the first is the MS `__noop` builtin, which w/o this pr simply errors, but now, it is implicitly converted to a `CallExpr` that returns an `int` (which emits 0 in CG); this matches MSVCs behaviour, but I doubt we have a test for that, so I’ll add one as well.

    The only other case that is affected here is if we have a standard-namespace builtin (e.g. `std::move`, `std::forward_like`, which before C++20 resulted in a warning, since C++ in an error; this behaviour is unaffected by this change, but I’ll some tests for this as well because from what I can tell, there currently are no tests for this diagnostic at all.


- `BuiltinType::IncompleteMatrixIdx`: From what I can tell, this is used for [matrix types](https://clang.llvm.org/docs/MatrixTypes.html), specifically, for the type of an expression such as `a[1]` where `a` is a matrix type because matrix types always require two subscripts; since an expression of this type as a placement-arg is thus always invalid, this should error, but previously, we were emitting a less-than-ideal error (e.g. ‘no known conversion from '<incomplete matrix index type>' to 'int'’), whereas with this pr, we now get the error ‘single subscript expressions are not allowed for matrix values’. There does not seem to be a test for this that involves placement-new, so I’ll add a few as well.

- `BuiltinType::OMPArraySection`, `BuiltinType::OMPArrayShaping`, and `BuiltinType::OMPIterator`: To my understanding, these cannot occur outside of OMP `#pragma`s, so they’re not relevant here.



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


More information about the cfe-commits mailing list