User Details
- User Since
- Jul 12 2012, 2:19 PM (585 w, 3 d)
Mon, Sep 25
Sun, Sep 24
Fri, Sep 22
[...]
Are these changes in non-C++20 mode intentional?
Thu, Sep 21
Wed, Sep 20
- Merge
Mon, Sep 11
The changes in SemaInit.cpp don't look correct to me. Trying to recurse through the initializer and find every nested temporary is highly error-prone; I can't tell you which expression nodes you forgot to recurse through, but I'm sure there are some.
Wed, Sep 6
Ping. Are there any further concerns here? (This obviously needs to be merged with trunk, and the -fclang-abi-compat= checks and release notes need to be updated to match the latest release version.)
Aug 31 2023
Aug 30 2023
Aug 29 2023
Aug 21 2023
The prior behavior of Clang is correct. A search of a class scope is ill-formed if it finds an ambiguous result, see http://eel.is/c++draft/basic.lookup#class.member.lookup-6.sentence-2
Aug 7 2023
Clang was correct here until fairly recently; P2280 (https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2280r4.html) changed the language rules. It was applied as a DR, so we should make that change retroactively rather than only in C++23 mode. See https://github.com/llvm/llvm-project/issues/63139, which tracks implementation of that language change.
Aug 2 2023
Aug 1 2023
Awesome, thanks! It makes perfect sense that rG61c7a9140b would fix this.
Jul 28 2023
Looks good, thanks!
This looks good to me. I think we can further refine the handling of duplicate diagnostics but I don't think that needs to be done as part of this bugfix.
Jul 27 2023
LGTM, though please wait a day or two for any more comments from @gribozavr2 since he's looked at this more closely than I have.
Jul 26 2023
Jul 25 2023
Jul 24 2023
Jul 21 2023
I've done a pass through this file looking for places where we incorrectly add to the ODR hash a type that was written within some other entity than the one that we're ODR hashing, that could validly be spelled differently in different declarations of that other entity. There are quite a lot of them; please see the comments here for places that need fixing.
OK, I see. The problem is that the canonical version of the type can be spelled in different ways in different translation units, due to us treating some expressions as being equivalent despite them not being the same under the ODR. For example, we consider these function template declarations to be redeclarations:
This looks correct to me, but it's still a little subtle. Perhaps it'd be clearer to map the method to an integer (0 for copy assignment, 1 for move assignment, 2 for destructor, 3 for equality comparison), and then order them by that integer? That'd be more obviously a strict weak order.
Jul 20 2023
- Add option to disable vptr-based dynamic_cast optimization.
Seems reasonable to me. Given that this affects all of LLVM I'd like to wait a day or so to see if anyone has concerns.
This branch doesn't look necessary or correct to me. We should never call this function on a lazy pointer in offset mode unless we have an external source that produced the offset, and it's certainly not appropriate to cast the offset to a pointer and return it.
Jul 19 2023
Ping.
Jul 18 2023
Jul 16 2023
Jul 13 2023
Jul 12 2023
- Avoid emitting the type_info when detecting whether it would be null.
- Bring back class type in GetVTablePtr and instead don't use it here.
Jul 11 2023
Jul 7 2023
- Mark gep as inbounds.
- Address review comments.
Jul 6 2023
Making the return ESR_Failed; unconditional looks to be the correct change here. We can't continue evaluation past that point because we don't know what would be executed next. Unconditionally returning ESR_Failed in that situation is what the other similar paths through EvaluateStmt do.
Jun 30 2023
Wow, what an amazing bug :)
LGTM, thank you for doing this!
Jun 29 2023
Jun 28 2023
I think the behavior change for the testcase here is correct, though I'm not sure that the patch is getting that behaviour change in the right way. Per [temp.type]/1.4 (http://eel.is/c++draft/temp.type#1.4),
Jun 20 2023
A constant expression (including the immediate invocations generated for consteval functions) is a full-expression, so destructors should be run at the end of evaluating it, not as part of the enclosing expression. That's presumably why the code is calling MaybeCreateExprWithCleanups -- to associate cleanups for the immediate invocation with that ConstantExpr rather than with the outer context. I think the problem is that we don't have an ExpressionEvaluationContext wrapping the immediate invocation, so we don't save and restore the enclosing cleanup state -- and we can't really wrap it, because we don't find out that we're building an immediate invocation until after we've already done so. Probably the best way to handle that is to create the inner ExprWithCleanups for the constant expression, but leave the cleanups flags alone so that we also create an outer ExprWithCleanups. That'll mean we sometimes create an ExprWithCleanups that doesn't actually run any cleanups, but that's OK, just redundant. One thing we will need to be careful about is assigning any block / compound literal cleanups to the right ExprWithCleanups node. We can figure out where to put them by doing a traversal of the ConstantExpr's subexpression to see if it contains the BlockDecl / CompoundLiteralExpr being referenced by the cleanup.
Jun 12 2023
Jun 11 2023
I have a concern with the name of this builtin. People are going to assume it produces a nondeterministic value, and use it for seeding random number generators and similar, and will be surprised when the value produced is actually deterministic, and, worse, might leak information on the stack that was left behind by some previous function call. Can we give this a name that doesn't suggest that it produces its value nondeterministically?
Jun 6 2023
Jun 2 2023
Taking a step back for a moment: what is the intended use case for this? My concern is that most of the time you're going to want to make O(N) such queries into a pack of N elements, resulting in O(N^2) compile time, much like we regrettably get from uses of __type_pack_element. If we can avoid the regret in this case by providing a different intrinsic, perhaps we should do so. (For example, we could take two packs of types, and produce a sequence of integers giving the indexes of each of the first types in the second list, in O(N) time. And similarly we could add a __type_pack_elements that takes a pack of types and a pack of indexes and performs N indexing operations at once, in O(N) time, rather than having the program do it in O(N^2) time.)
Jun 1 2023
May 31 2023
May 30 2023
LGTM too.
May 29 2023
May 28 2023
May 25 2023
Sorry for the late comments.
May 23 2023
May 18 2023
May 3 2023
LGTM with the extra test
I think this would be an interesting test:
May 1 2023
There's some corresponding code in lib/CodeGen that can be deleted too:
This looks like a nice improvement to me.
Apr 30 2023
I wonder if there are any cases where the "deduction guide" is so broken that we shouldn't be creating a CXXDeductionGuideDecl at all. For example, if there's no trailing return type, then the declaration is syntactically invalid (and looks more like a constructor than a deduction guide), and it's not clear to me whether creating a CXXDeductionGuideDecl really makes sense.
Apr 26 2023
LGTM, I'll leave approval to @shafik :)
I think that the bugs that this was aiming to fix were fixed by rG1e43349e321694d7fee3d77cb691887ad67fb5d7, which means that we should not ask whether the constraints of an inherited constructor are satisfied any more. Instead, overload resolution will look at whether the constraints of the original constructors are satisfied. (More broadly, I wonder whether DiagnoseUseOfDecl should be considering constraints at all -- the model in the standard is that any time you name a function, you go through overload resolution, which considers constraints, so DiagnoseUseOfDecl should only ever be duplicating work that's already been done by overload resolution, but it's possible we're not entirely following that model. Still, we should be able to avoid redoing the constraint satisfaction checks in the common case where overload resolution already did them.)
Apr 25 2023
Are there any calls to Sema::ActOnParamDefaultArgumentError left? If not, can you also remove the function?