martong marked 2 inline comments as done.
martong added inline comments.
================
Comment at: lib/AST/ASTImporter.cpp:5147
}
+ } else { // ODR violation.
+ // FIXME HandleNameConflict
----------------
shafik wrote:
> ODR violations are ill-formed no diagnostic required. So currently will this
> fail for cases that clang proper would not?
> ODR violations are ill-formed no diagnostic required.
ASTStructuralEquivalenceContext already provides diagnostic in the ODR cases.
E.g.:
```
// Check for equivalent field names.
IdentifierInfo *Name1 = Field1->getIdentifier();
IdentifierInfo *Name2 = Field2->getIdentifier();
if (!::IsStructurallyEquivalent(Name1, Name2)) {
if (Context.Complain) {
Context.Diag2(Owner2->getLocation(),
Context.ErrorOnTagTypeMismatch
? diag::err_odr_tag_type_inconsistent
: diag::warn_odr_tag_type_inconsistent)
<< Context.ToCtx.getTypeDeclType(Owner2);
Context.Diag2(Field2->getLocation(), diag::note_odr_field_name)
<< Field2->getDeclName();
Context.Diag1(Field1->getLocation(), diag::note_odr_field_name)
<< Field1->getDeclName();
}
return false;
}
```
We change this to be always a Warning instead of an Error in this patch:
https://reviews.llvm.org/D58897
> So currently will this fail for cases that clang proper would not?
Well, I think the situation is more subtle than that.
There are cases when we can link two translation units and the linker provides
a valid executable even if there is an ODR violation of a type. There is no
type information used during linkage, except for functions' mangled names and
visibility. That ODR violation is recognized though when we do the merge in the
AST level (and I think it is useful to diagnose them).
So if "clang proper" includes the linker then the answer is yes. If not then we
are talking about only one translation unit and the answer is no.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58673/new/
https://reviews.llvm.org/D58673
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
