[cfe-commits] [Patch] Handle qualifiers for template types in during template type diffing.
Chandler Carruth
chandlerc at google.com
Mon Aug 27 11:04:09 PDT 2012
Cool, a couple of high level comments:
@@ -778,17 +793,19 @@
if (Context.hasSameType(FromType, ToType)) {
Tree.SetSame(true);
} else {
- const TemplateSpecializationType *FromArgTST =
- GetTemplateSpecializationType(Context, FromType);
- const TemplateSpecializationType *ToArgTST =
- GetTemplateSpecializationType(Context, ToType);
-
- if (FromArgTST && ToArgTST) {
- bool SameTemplate = hasSameTemplate(FromArgTST, ToArgTST);
- if (SameTemplate) {
-
Tree.SetNode(FromArgTST->getTemplateName().getAsTemplateDecl(),
-
ToArgTST->getTemplateName().getAsTemplateDecl());
- DiffTemplate(FromArgTST, ToArgTST);
+ Qualifiers FromQual, ToQual;
+ bool SameTemplate = hasSameTemplate(Context, FromType, ToType,
+ FromQual, ToQual);
+ if (SameTemplate) {
+ const TemplateSpecializationType *FromArgTST =
+ GetTemplateSpecializationType(Context, FromType);
+ const TemplateSpecializationType *ToArgTST =
+ GetTemplateSpecializationType(Context, ToType);
+ if (FromArgTST && ToArgTST) {
+
Tree.SetNode(FromArgTST->getTemplateName().getAsTemplateDecl(),
+
ToArgTST->getTemplateName().getAsTemplateDecl());
+ Tree.SetNode(FromQual, ToQual);
+ DiffTemplate(FromArgTST, ToArgTST);
}
}
}
The indent here is now off... and this is crazily nested.
Can we factor this function into smaller pieces with less nesting? I think
that would make it much easier to read.
@@ -824,62 +841,120 @@
}
}
+ static void RemoveQualifiers(Qualifiers &Qual, Qualifiers RemoveQual) {
I feel like this should be a method on Qualifiers?
+ // Removes any common qualifiers from ToQual and FromQual and places them
+ // in CommonQual.
+ void DiffQualifiers(Qualifiers &FromQual, Qualifiers &ToQual,
I feel like this should also be provided by Qualifiers -- probably as a
static factory function?
On Mon, Aug 27, 2012 at 10:51 AM, Richard Trieu <rtrieu at google.com> wrote:
> On Wed, Aug 22, 2012 at 12:59 PM, Chandler Carruth <chandlerc at google.com>wrote:
>
>> On Wed, Aug 22, 2012 at 11:25 AM, Richard Trieu <rtrieu at google.com>wrote:
>>
>>> On Tue, Aug 21, 2012 at 7:17 PM, Chandler Carruth <chandlerc at google.com>wrote:
>>>
>>>> On Tue, Aug 21, 2012 at 6:59 PM, Richard Trieu <rtrieu at google.com>wrote:
>>>>
>>>>> Currently, template diffing does not handle cases where the templates
>>>>> have qualifiers. This can lead to warnings such as:
>>>>>
>>>>> ... 'vector<vector<[...]>>' cannot convert to 'vector<vector<[...]>>'
>>>>> ...
>>>>>
>>>>> when the message means, can't convert from vector<const vector<int>>
>>>>> to vector<vector<int>>. This patch allows the internal diff tree to store
>>>>> the qualifiers for template types and adds new methods for finding the
>>>>> difference between sets of qualifiers and to print out the qualifiers.
>>>>> This does not affect non-template types as they already print qualifiers.
>>>>>
>>>>
>>>> Very cool!
>>>>
>>>>
>>>>> Examples:
>>>>> vector<const vector<int>> and vector<vector<int>>
>>>>> inline: vector<const vector<[...]>> vs. vector<(missing const)
>>>>> vector<[...]>
>>>>>
>>>>
>>>> I think I would prefer: vector<const vector<[...]>> vs.
>>>> vector<vector<[...]>>
>>>>
>>>
>>>> We can use the cyan highlighting to further emphasize the qualifiers on
>>>> both sides (if any) when the qualifiers are different. This would make the
>>>> const bright here, and in the example further down, the const bright on one
>>>> side and the volatile on the other.
>>>>
>>>> One reason why I prefer this format is brevity. The other is that the
>>>> error might be the *presence* of const just as easily as the absence.
>>>>
>>>
>>> The motivation behind the (missing ...) was that there's
>>> no guarantee that the types printed would be close to each other. There
>>> may be cases of : vector<const vector<[...]>> ... long message ...
>>> vector<vector<[...]>>. Going by your suggestion, the second type would be
>>> completely unhighlighted.
>>>
>>
>> Seems fine to me? As long as the const is highlighted in the first type,
>> I think it's likely to draw the attention to the correct place.
>>
>
> New patch. This includes the changes Chandler and Richard Smith
> suggested: when printing types inline, don't print (missing ...) for
> missing qualifiers; and maintain consistence by performing qualifier
> highlighting on non-template types used as template arguments.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120827/041ba171/attachment.html>
More information about the cfe-commits
mailing list