MatzeB added inline comments.

================
Comment at: lib/CodeGen/CodeGenModule.cpp:477
       Context.getTypeSizeInChars(Context.getWideCharType()).getQuantity();
-  assert((LangOpts.ShortWChar ||
-          llvm::TargetLibraryInfoImpl::getTargetWCharSize(Target.getTriple()) 
==
----------------
compnerd wrote:
> compnerd wrote:
> > MatzeB wrote:
> > > rnk wrote:
> > > > @MatzeB ptal
> > > Can you find a new place for this assert()? Please do not just remove it!
> > > 
> > > For the backstory: Unfortunately I had to duplicate the wchar decision 
> > > logic inside llvm (TargetLibraryInfoImpl::getTargetWCharSize() for cases 
> > > where we just have the target triple available but need to know the size 
> > > of wchar_t using library function. This means the logic in LLVM needs to 
> > > be updated when support for new platforms is added but for people adding 
> > > platform support it will not be obvious that they have the change 
> > > LLVM/TargetLibraryInfo as well unless an assert() point them to there 
> > > being a mismatch.
> > Sure, I'll try to see if I can find a suitable place or adjustment of it.  
> > However, one thing to note is that the frontend does actually embed that 
> > into the IR metadata ("wchar_size"). 
> I think that if we try to retain this assertion, we need to update all the 
> tests to ensure that they pass the arguments for selecting the `wchar_t` 
> type.  The entire problem is that the backend view of this cannot correlate 
> with what the user specified without passing it back to it.  The "wchar_size" 
> metadata does exactly that.  Using that to perform the validation for the 
> library function call.
This has been resolved with r314187 that made the assertion obsolete.


Repository:
  rL LLVM

https://reviews.llvm.org/D37891



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to