[PATCH] D28911: [LinkerScript] Implement `MEMORY` command

[Meador Inge via Phabricator via llvm-commits]

meadori added inline comments.


================
Comment at: ELF/LinkerScript.cpp:419-422
+      error("section '" + CurOutSec->Name + "' will not fit in region '" +
+            CurMemRegion->Name + "'");
+      error("region '" + CurMemRegion->Name + "' overflowed by " +
+            Twine(OverflowAmt) + " bytes");
----------------
ruiu wrote:
> meadori wrote:
> > ruiu wrote:
> > > You don't want to call `error` twice for one error. You can print out a multi-line error just by adding "\n" in middle of a message.
> > I originally did that, but the formatting looks bad:
> > 
> > ```
> > ./bin/ld.lld: error: section '.data' will not fit in region 'ram'
> > region 'ram' overflowed by 3084 bytes
> > ```
> > 
> > Personally I think the following is easier to understand:
> > 
> > ```
> > ./bin/ld.lld: error: section '.data' will not fit in region 'ram'
> > ./bin/ld.lld: error: region 'ram' overflowed by 3084 bytes
> > ```
> Semantically I want to emit one `error:` for each error. I don't think this needs to be in two lines -- how about concatenating these messages with ": " instead of "\n"?
That works.  It looks like this now:

```
error: section '.data' will not fit in region 'ram': overflowed by 3084 bytes
```


================
Comment at: ELF/LinkerScript.h:197-198
+  MemoryRegion() : Origin(0), Length(0), Index(0), Flags(0), NotFlags(0) {}
+  MemoryRegion(StringRef N, uint64_t O, uint64_t L, uint32_t F, uint32_t NF)
+      : Name(N), Origin(O), Length(L), Index(O), Flags(F), NotFlags(NF) {}
+
----------------
ruiu wrote:
> meadori wrote:
> > ruiu wrote:
> > > I think you can remove this constructor if you initialize a struct using `{}` as I mentioned below.
> > That doesn't work. I get errors like:
> > 
> > ```
> > error: no match for ‘operator=’ (operand types are ‘lld::elf::MemoryRegion’ and ‘<brace-enclosed initializer list>’)
> > ```
> Sorry, the number of initializers did not match with the number of members. Does this work?
> 
>   {Name, Origin, Length, Origin, Flags, NotFlags}
I accounted for that.  It seems that in-class field initialization and braced initializer lists are not compatible:

```
$ c++ --version
c++ (Ubuntu 4.8.4-2ubuntu1~14.04.3) 4.8.4
Copyright (C) 2013 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

meadori at build1-trusty-cs:/scratch/meadori/llvm-mainline/obj/llvm$ cat -b test.cpp
     1  struct T {
     2    unsigned x = 0;
     3  };
     4  int main(void) {
     5    T t = {1};
     6    return 0;
     7  }
meadori at build1-trusty-cs:/scratch/meadori/llvm-mainline/obj/llvm$ c++ -std=c++11 test.cpp
test.cpp: In function ‘int main()’:
test.cpp:5:11: error: could not convert ‘{1}’ from ‘<brace-enclosed initializer list>’ to ‘T’
   T t = {1};
           ^
```

If I remove the in-class initializers, it works.


https://reviews.llvm.org/D28911