RFC: Make ExtInfo param to FunctionType construction always explicit?

Development Compiler
1

Hello,

I'd like to get some feedback about making various function type APIs not default the ExtInfo parameter. For example:

@@ -3231,7 +3231,7 @@ class FunctionType final
 public:
   /// 'Constructor' Factory Function
   static FunctionType *get(ArrayRef<Param> params, Type result,
-                           ExtInfo info = ExtInfo());
+                           ExtInfo info);

   // Retrieve the input parameters of this function type.
   ArrayRef<Param> getParams() const {

In my experience with a downstream branch which has additional ExtInfo state, it's hard not to notice how often the compiler unpacks and rebuilds function types. It's also hard not to notice how haphazard ExtInfo propagation/preservation is as function types are unpacked and rebuilt.

Ideally, I'd like the propagation/preservation logic to be more explicit or at least have better comments. I.e. "Why are some ExtInfo bits dropped and others preserved?" But for for now, I'd just settle for ExtInfo not being defaulted so at least people get a reminder to think about the ExtInfo bits as they unpack and rebuild a function type.

What do people think?

Dave

4 Likes
2

I tried making this change at one point earlier when I was adding the ClangTypeInfo field and had the same issue that you mentioned in understanding why certain aspects were getting preserved and others weren't. However, given the number of places that this is being used, I stopped mid-way. Maybe it's better to first add comments in specific places where it is particularly confusing and then re-consider if it's worth removing the default?

3

I have a fast machine. Grinding through the fall out isn’t hard.

4

Sure, if that works for you. then by all means. :+1:

5