erichkeane marked 11 inline comments as done.
erichkeane added inline comments.
================
Comment at: clang/lib/Sema/SemaConcept.cpp:70
+ assert((!LHS.isInvalid() && !RHS.isInvalid()) && "not good expressions?");
+ assert(LHS.isUsable() && RHS.isUsable() && "Side not usable?");
+ // We should just be able to 'normalize' these to the builtin Binary
----------------
ChuanqiXu wrote:
> I feel like the usage of the API could be further simplified.
Heh, the suggestion is inverted slightly (those should be `!isUsable`) which
caught me for a while :) Either way, I think it is a good suggestion.
================
Comment at: clang/lib/Sema/SemaConcept.cpp:432-476
+ if (TemplateArgs) {
+ MultiLevelTemplateArgumentList JustTemplArgs(
+ *FD->getTemplateSpecializationArgs());
+ if (addInstantiatedParametersToScope(
+ FD, PrimaryTemplate->getTemplatedDecl(), Scope, JustTemplArgs))
+ return true;
+ }
----------------
ChuanqiXu wrote:
> The suggested change works. I feel it is not identical with the original. Is
> it correct or do we miss something in the test?
The very top condition is not exactly the same I think, but in a way that I
don't believe matters now that this function isn't recursive.
Otherwise I think this is a vast improvement, so I'll keep it!
================
Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:2173
Function->setInstantiationOfMemberFunction(D, TSK_ImplicitInstantiation);
+ } else if (!isFriend) {
+ // If this is not a function template, and this is not a friend (that is,
----------------
ChuanqiXu wrote:
> The check here is not straightforward. Here we want to check if the Function
> is local declared function. But what we check here is about friendness. I am
> not 100% sure if it is right and other reader might be confusing too. I would
> suggest to check it directly. e.g, check if one of its DeclContext is
> FunctionDecl.
I cannot come up with a better one here. Really the condition here is "this is
not one of the above conditions, and is still not a friend function". A friend
could ALSO have a `DeclContext` of a `FunctionDecl` as well, so I don't have a
good idea of what we could do.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119544/new/
https://reviews.llvm.org/D119544
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
