This is an archive of the discontinued LLVM Phabricator instance.

[mlir][ODS] Use StringLiteral instead of StringRef when applicable
ClosedPublic

Authored by vinograd47 on Feb 24 2021, 7:53 AM.

Details

Summary

Use StringLiteral for function return type if it is known to return
constant string literals only.

This will make it visible to API users, that such values can be safely
stored, since they refers to constant data, which will never be deallocated.

StringRef is general is not safe to store for a long term,
since it might refer to temporal data allocated in heap.

Add inline and constexpr methods support to OpMethod.

Diff Detail

Event Timeline

vinograd47 created this revision.Feb 24 2021, 7:53 AM
vinograd47 requested review of this revision.Feb 24 2021, 7:53 AM

This is an updated version of https://reviews.llvm.org/D95945 with attempt to fix build issues caused StringLiteral class usage. @mehdi_amini @antiagainst @bkramer Could you please take a look one more time?

Sorry, I didn't find a way to update existing Differential, Archanist refuses to update Closed change.

Update TypeDef - getMnemonic method.

mehdi_amini accepted this revision.Feb 27 2021, 9:58 AM

LG

Sorry, I didn't find a way to update existing Differential, Archanist refuses to update Closed change.

You need to reopen the revision in the "add action": the same menu available above the comment box where you can approve a revision.

This revision is now accepted and ready to land.Feb 27 2021, 9:58 AM

Use explicit StringLiteral ctor call.

@antiagainst Could you please take a look? Will this version work with current StringLiteral limitations?

Is this something that we want to make a convention, then we'd want to then capture in https://mlir.llvm.org/getting_started/DeveloperGuide/ (I don't believe it is a LLVM convention)

Is this something that we want to make a convention, then we'd want to then capture in https://mlir.llvm.org/getting_started/DeveloperGuide/ (I don't believe it is a LLVM convention)

I don't think it is some kind of convention. I've used StringLiteral in the above places just to make it clear that the return value has infinite life time and it is safe to store it (in contrast to generic StringRef, which might point to temporally allocated data). It is not a requirement for the entire code base, just specific obvious places.