r304956 - [ODRHash] Change the fall-back diagnostic error.

[Richard Trieu via cfe-commits]

After r304962, it should now produce messages like:

'foo' defined here has different definitions in different modules; first
difference is this unexpected decl
but in 'Module' found another unexpected decl

'foo' with definition in module 'FirstModule' has different definitions in
different modules; first difference is this field
but in 'Module' found different field

On Wed, Jun 7, 2017 at 6:41 PM, Richard Trieu <rtrieu at google.com> wrote:

> Yes, I will go correct the diagnostic text.
>
> In theory, there is nothing that is supposed to reach this diagnostic or
> the one below it.  Except that the hasher isn't complete yet and some
> things slip through.  Once things are more stable, these should be replaced
> with llvm_unreachable instead.
>
> On Wed, Jun 7, 2017 at 6:05 PM, Richard Smith <richard at metafoo.co.uk>
> wrote:
>
>> On 7 June 2017 at 17:56, Richard Trieu via cfe-commits <
>> cfe-commits at lists.llvm.org> wrote:
>>
>>> Author: rtrieu
>>> Date: Wed Jun  7 19:56:21 2017
>>> New Revision: 304956
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=304956&view=rev
>>> Log:
>>> [ODRHash] Change the fall-back diagnostic error.
>>>
>>> Provide a little more information when a ODR violation is detected, but
>>> the
>>> specific error could not be diagnosed.
>>>
>>> Modified:
>>>     cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
>>>     cfe/trunk/lib/Serialization/ASTReader.cpp
>>>
>>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/
>>> Basic/DiagnosticSerializationKinds.td?rev=304956&r1=304955&r
>>> 2=304956&view=diff
>>> ============================================================
>>> ==================
>>> --- cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
>>> (original)
>>> +++ cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td Wed
>>> Jun  7 19:56:21 2017
>>> @@ -174,6 +174,13 @@ def note_module_odr_violation_mismatch_d
>>>    "method %2 with %ordinal3 parameter of type %4%select{| decayed from
>>> %6}5|"
>>>    "method %2 with %ordinal3 parameter named %4}1">;
>>>
>>> +def err_module_odr_violation_mismatch_decl_unknown : Error<
>>> +  "%q0 has different definitions in different modules; first difference
>>> is "
>>> +  "%select{definition in module '%2'|defined here}1 found different "
>>> +  "%select{||||static assert|field|method|other}3">;
>>> +def note_module_odr_violation_mismatch_decl_unknown : Note<
>>> +  "but in '%0' found different %select{||||static
>>> assert|field|method|other}1">;
>>>
>>
>> This will produce messages like
>>
>> "first difference is defined here found different static assert"
>> "first difference is definition in module 'X' found different other"
>>
>> ... which seem pretty confusing. Can this be rephrased so that it forms
>> easier-to-read sentences? (Also, can we get some test coverage for this
>> diagnostic?)
>>
>>
>>> +
>>>  def warn_duplicate_module_file_extension : Warning<
>>>    "duplicate module file extension block name '%0'">,
>>>    InGroup<ModuleFileExtension>;
>>>
>>> Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serializat
>>> ion/ASTReader.cpp?rev=304956&r1=304955&r2=304956&view=diff
>>> ============================================================
>>> ==================
>>> --- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
>>> +++ cfe/trunk/lib/Serialization/ASTReader.cpp Wed Jun  7 19:56:21 2017
>>> @@ -9753,13 +9753,13 @@ void ASTReader::diagnoseOdrViolations()
>>>        if (Diagnosed == true)
>>>          continue;
>>>
>>> -      Diag(FirstRecord->getLocation(),
>>> -           diag::err_module_odr_violation_different_definitions)
>>> -          << FirstRecord << FirstModule.empty() << FirstModule;
>>> -
>>> -      Diag(SecondRecord->getLocation(),
>>> -           diag::note_module_odr_violation_different_definitions)
>>> -          << SecondModule;
>>> +      Diag(FirstDecl->getLocation(),
>>> +           diag::err_module_odr_violation_mismatch_decl_unknown)
>>> +          << FirstRecord << FirstModule.empty() << FirstModule <<
>>> FirstDiffType
>>> +          << FirstDecl->getSourceRange();
>>> +      Diag(SecondDecl->getLocation(),
>>> +           diag::note_module_odr_violation_mismatch_decl_unknown)
>>> +          << SecondModule << FirstDiffType <<
>>> SecondDecl->getSourceRange();
>>>        Diagnosed = true;
>>>      }
>>>
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170607/5d6ea630/attachment-0001.html>